stack/agents/filbert/work/queue-e-review-r1-2026-09-27.md

# Queue Piece E review, round 1 (#1508, row 13)

Filbert, 2026-09-27. Plan: `queue-as-data-plan-2026-09-26.md` §8.10,
amended for lead decision 40 (plan sha256 68a25ffe…, uncommitted).

## Verdict

**Changes requested, one item (C1).** The review covers the amended
round-1 candidate: `build.patch` sha256
02a01c291890626f58ba103d1c65e853b042b609887868bf59dea7f9b50b227d at
2333d837, with `build-manifest.sha256` cad51929… and `build.md` d3bdb826….
I had also reviewed the first version (patch 78445322…, manifest
8f0e4fea…) in full. The amendment changes only the full-page rule, its
tests and the README, so everything I checked on the first version still
holds.

C1 is a real bug in the age check, and it's small. Everything else is
ready, including the decision-40 change.

## C1. The age check never flags a row made required after genesis

**Where:** `packages/ledger/src/queue-checks.mjs`, line 200 in the amended
file:

```js
const days = ageDays(Date.parse(`${r.requiredSince}T00:00:00Z`), now);
if (days > AGE_LIMIT_DAYS) add(violations, 'age', ...
```

**What happens.** The code assumes `requiredSince` is a date. Genesis
rows carry dates (rows 9, 10, 11 and 13 have `2026-09-13`). But the queue
writes a full ISO time for any row made required later:
`set <id> required true` sets `requiredSince = entry.at`
(`packages/queue/src/queue.mjs:860`), and `add --required` does the same
(line 904). The validator accepts both forms (line 322, `checkTime(…,
{ date: true })`). For an ISO time the template gives
`2026-09-01T12:00:00.000ZT00:00:00Z`, `Date.parse` returns NaN, `days`
is NaN, and `NaN > 14` is false. The row is never an age violation, and
nothing is reported as undecided either.

**Reproduced** in-process at 2026-09-27T12:00Z on one required,
in-progress row:
- `requiredSince: "2026-09-01"` → violations `owner-invalid, age`, result
  `fail`;
- `requiredSince: "2026-09-01T12:00:00.000Z"` → violations `owner-invalid`
  only. The age violation is missing.

(`owner-invalid` comes from passing no seats; it doesn't matter here.)

**Why it matters.** Today's rows all have dates, so Monday's run is right.
The first row made required through the queue CLI would slip past the
14-day gate without a word. A check that goes quiet is the failure the
ledger's result levels exist to prevent.

**Fix.**
- A `DATE_RE` value parses as `T00:00:00Z`; anything else goes to
  `Date.parse` as it stands.
- If the result is NaN, fail closed: an `age` undecided item naming the
  row, or a violation. Never silence. (The validator should make this
  unreachable, but the check shouldn't depend on that.)
- Tests: an ISO `requiredSince` 15 days old fails and one 14 days old
  doesn't, next to the existing date cases. Your fixtures build
  `requiredSince` with `since(n)`, so an ISO variant fits in the same test.
- Mutant: restore the old template. The new test should kill it.
- The README's age paragraph can say that `requiredSince` is a date for
  genesis rows and an ISO time for later ones.

## The full page (lead decision 40)

I agree with the ruling, and the amendment implements it correctly. An
issue is never treated as closed because it's missing from the open list.
It's looked up or left `unknown (budget)`, and the unknown state already
makes the run incomplete. A server that caps pages below 50 would make
`full` meaningless anyway, so dropping it as a standalone signal costs
nothing.

What I checked on the delta:
- `open-list-full` is added only when the page is full and some issue in
  `wanted` is `unknown`. `wanted` is the union of every row's `closes`,
  including rows that aren't done. An unknown issue on a pending row can
  only affect a disposition, but it still keeps the page undecided. That
  errs toward `incomplete`, and it matches the decision's wording ("some
  wanted issue"). Keep it.
- The new end-to-end test drives the fake helper through `issueStates` and
  `queueChecks` for all three cases: resolved, open off the page, and past
  the budget.
- The README's result list and call table match the code.
- My plan now says the same thing: 8.10's budget table and result list,
  a pointer from section 2's E text, and Q9 and 2.A for row 7.

## What I checked

All of this ran in scratch clones with push disabled: `/tmp/fqe` at
f304eaa5 for the first version, and `/tmp/fqe2` at 2333d837 for the
amendment. The inputs were frozen as 0444 copies.

- **Manifest and suites.** The amended manifest checks 5/5. `node --test
  packages/ledger/tests/` passes 76/76, and `packages/queue/tests` with
  `packages/seat/tests` passes 161/161. The first version had passed
  75/75 and 161/161.
- **Source.** I read `queue-checks.mjs` in full, and the diffs to
  `cli.mjs`, the README and both test files. I compared the amendment with
  the first version file by file. Only `queue-checks.mjs` (the rule and its
  comment), the README (two passages) and `queue-checks.test.mjs` changed.
- **Local run** on the first version, for 2026-09-27 with `--no-issues
  --no-t3` and three seats declared: 0 violations, result `incomplete`
  (issues not run), 18 protected changes.
- **Mutations.** I wrote 21 mutants of my own against the first version
  (E1–E21), apart from Darkwing's 37. The suite kills 19. The two
  survivors are equivalent:
  - E2 checks the metric page before the open list. Both are current
    sources, so they can't disagree about an issue.
  - E8 applies the age check to rows that aren't required. The validator
    refuses `requiredSince` on such a row, so the mutant changes nothing.

  I wrote five more against the amendment (F1–F5), and the suite kills
  all five:
  - F1: a full page is always undecided;
  - F2: an unknown issue makes `open-list-full` without a full page;
  - F3: any state other than open counts as unresolved;
  - F4: only `unknown (budget)` counts, so a failed lookup doesn't;
  - F5: it takes two unknown issues.

  None of my 26 mutants touches the age parsing. The C1 bug is in an input
  shape the fixtures don't use.

## Darkwing's choices

I agree with choices 1 to 11 in `build.md`. Two of them depart from the
plan's table, and in both cases the change fails closed:
- 4: a malformed pid is `invalid`, not `pid-unknown`, so it's a violation
  rather than undecided;
- 5: a registration for another checkout is `missing`, because it matches
  the seat name but not the root.

Choice 11 (protected changes listed, never checked) is what R4, R10 and J2
asked for.

## Non-blocking

- **n1. The call deadline kills the helper, not curl.** `execFileSync`
  with a 60-second `timeout` sends SIGTERM to `gitea-api.sh` only. I
  checked the behaviour with a script that runs a child `sleep`:
  `execFileSync` throws `ETIMEDOUT` at the deadline, so the ledger's wall
  time stays bounded, but the child keeps running as an orphan. For the
  ledger that means a stuck curl can outlive the run, and the helper's
  response file may be left in `/tmp`. It's a GET, so nothing is written
  to Gitea. D wrapped its helper call in `timeout -s KILL` for this.
  Doing the same here is a small change. It can go in this round or
  later.
- **n2. `closedInMetric` could also check `state === 'closed'`.** It uses
  `closed_at` alone. I haven't checked whether Gitea clears `closed_at`
  when an issue is reopened. If it doesn't, a reopened issue that is off
  the open page but on the metric page would count as closed. The open
  page is always full now, so that case can happen. Checking `state` as
  well settles it either way.
- **n3. The unknown-budget message doubles a word.** The test pins
  `unknown (unknown (budget))`. It would read better as `unknown (budget)`
  or `unknown (lookup budget)`. Cosmetic.
- **n4. `--unsupported-runtime` is self-declared.** The flag is a
  statement from whoever runs the ledger, not evidence. It's printed on
  every run, which is what 8.10 asks for. Nothing to change.

## For Darkwing and Sage

Fix C1 with its test and mutant, then send the patch and manifest again.
I'll review round 2 against C1 only, plus n1 if it's in. My plan
amendment (68a25ffe…) goes into E's commit with this review.