Skip to content

First run: a card follows the first task from File one to its PR; the Start card counts only steps before Start (#741) [PROTECTED: owner merges] - #816

Merged
jayden1711 merged 12 commits into
mainfrom
issue-741-first-task-card
Oct 20, 2026
Merged

jayden1711 merged 12 commits into
mainfrom
issue-741-first-task-card

Conversation

@jayden1711

@jayden1711 jayden1711 commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Intent: After "File one", the first-run page follows that issue to its PR. A First task card shows claimed, the worker, the evaluator's verdict, the PR, its checks, then merged or held. A held PR says why in fixed plain words and what lets it through. Every stage comes from events the engine already records, and the card shows no free text. Every estimate names its basis. The Start card also stops warning about Your first task, a step that comes after Start.

Risk class: protected. It touches src/dashboard.ts (one field added to the existing GET /api/first-run, no new route) and adds src/first-task-progress.ts and src/hold-words.ts. Any src/** change is engine. Commit 1 (the Start count) is web/ only and could ship alone.

Closes #741

What changed

  • 11c9f66, web/ only: startBlockers in web/src/pages/FirstRun.tsx. Start's warning counts only the required steps not done that come before Start, in screen order. With no Start step in the mode, every required step not done counts, as before.

    • Correction (measured): this commit's message says its "still warns ... as before" test "passes before and after". That is false. The test fails on main, which counts 2 steps (Your first task too) where the test expects 1. Commit 9d22da1's message also says so. No history was rewritten.
  • f739109, c7bae71, 9d22da1, 0f5ccf2, the card:

    • src/first-task-progress.ts: a pure fold over the event log.

      • It starts from the latest first_run.first_task_filed audit entry and reads only that issue's events recorded after it.
      • Status, merge and close events count only for the PR the harness opened for the issue.
      • A merge.decided or merge.failed counts only at that PR's latest known head, taken from its latest pr.opened, pr.status, pr.ready or pr.unready. Otherwise the stage waits: "a new head: it waits for its checks and a fresh decision".
      • A verdict counts only for the worker's latest attempt.
      • Estimates:
        • Claimed: "Estimate", from the coordinator's tick. A test keeps it equal to src/service.ts.
        • Worker, evaluator and PR: "Guess: 15-60 minutes from the claim to a PR".
        • Checks and merge: "no estimate".
    • src/hold-words.ts: fixed words (why it waits, and what lets it through) for every kind merge.decided carries today:

      • mergeDecision's kinds;
      • from merge-flow: GitHub can't merge it cleanly, merging failed, GitHub still computing;
      • the combined check: a conflict, no branch point, or the fast tests failing;
      • the docs-only ruleset hold;
      • conflict fix:.

      An unknown prefix reads "Another reason (see the Board)", never its text.

    • src/dashboard.ts: GET /api/first-run adds firstTask. It is memoized per event-log snapshot, and null in setup mode. The route is writers only, as before.

    • web/src/components/FirstTaskProgress.tsx: the read-only card under Start. The block line is fixed words pointing to the Board. No issue title is shown.

    • docs/feature-map.md: a row for the card, and one sentence on Start's row.

  • Change 3 (a new hold prefix must fail a test): I built the source-scan test, not the shared kind list.

  • Round 3, 27902ca:

    • Forged rows. The event log is never re-validated on read. So each field the card renders from an event is checked against its schema type, or dropped:
      • the run's end reason and the verdict's confidence must be members of the schema's own enums (EventSchemas['run.finished'].shape.reason.options, EventSchemas['eval.verdict'].shape.confidence.options), else "unknown";
      • the attempt, the PR number and the filed issue must be positive safe integers;
      • every time must be a parseable date (normalized to ISO), else dropped.
    • True sentences:
      • design names "a dependency manifest changes";
      • doubt says "for example" and adds no verdict, a doubted test and no design answer;
      • merge-failed says "at least 3 times".
    • The scan fails closed (test code only, no package cost):
      • Every emitter of merge.decided and conflict_fix.finished in src/ must sit in a scanned region. The one exception is a conflict_fix.finished whose call has reasons: [] itself (recovery's "interrupted"); the scan found that emitter.
      • New regions: mergeDecisionFor, fix-runs conflictRun and giveUpConflict, and merge-policy's reason functions one by one.
      • Traced values match whole expressions (a traced call matches with any arguments). Each traced value names the region that builds what it carries in, and that region must be scanned.
      • A non-literal why/hold value or reasons initializer fails unless traced. So does any unshift, splice or concat.
      • The tokenizer skips strings, templates and comments.
    • Known gaps (named in the test): a traced helper or table lookup that changes what it returns, and reasons passed through a new MergeInput field whose builder isn't traced. These cost only the plain words: the card then reads "Another reason (see the Board)", and nothing leaks.
  • Round 4, f93efce:

    • The card never takes the page down. The route builds it through firstTaskCard. If the fold throws, it returns null (the card is left out) and logs once; Start, the steps and the rest still answer.
    • The fold no longer throws on a forged row:
      • no String() is applied to an event value;
      • a non-string hold reason reads as an unknown one ("Another reason (see the Board)");
      • a null payload reads as empty;
      • a pr.opened whose number is not a positive safe integer is not an opened PR. It is never a done stage reading "PR #null".
    • The scan reads mergeDecisionFor's definition.
      • region() now requires its signature exactly once.
      • mergeDecisionFor's signature is now unique ( mergeDecisionFor(n: number).
      • Correction, false through 27902ca: "each missed row is now reported". The first match was the call at merge-flow.ts:163, so the definition was never scanned, and E1-E3 were missed on the real file.
      • Now E1-E3 are proved on merge-flow.ts's real text changed in memory, and a fixture puts a call before the definition.
    • Recovery's exception (as of 7b4f5e6) applies only when the argument right after the event type is itself an object literal and is the call's last argument. That literal must hold exactly reasons: [] among its top-level fields, with no spread. The review's N7 and N11a-c are fixtures.
      • 53e2dc5 (round 6): every top-level field must also start with a plain identifier key, or be shorthand, and never __proto__. These all lose the exemption and are fixtures:

        • a computed key (['reasons'], [k]);
        • a quoted key ('reasons', "reasons");
        • a getter or a method named reasons;
        • a comment before a key;
        • __proto__.

        recovery.ts:46 stays exempt (measured: first-task-progress 18 of 18).

      • Correction, false through e0f3f2c's parent 0c1a7ff: the body said "exactly reasons: [] among the call's top-level fields". It read the first { anywhere in the call, so a wrapped payload was exempt.

    • Named gaps, not chased. The scan is a backstop, and "every kind gets its own words" is lint-only until Hold reasons: a typed hold kind from one shared list, so a new merge.decided reason can't be emitted without its plain words (by design; #816 follow-up) #824. Each gap below costs only the plain words: the card reads "Another reason (see the Board)", nothing leaks, and the page stays up.
      • Round-3 review (N1-N4, N8-N10):
        • an index write, push.apply, a shorthand { hold } or a hold variable, ??=;
        • an emitter by a constant type, by another method name, or by emit?.(.
      • Round-6 review (measured by its probe10):
        • emitters written emit (, emit\n(, emit<'merge.decided'>( or emit(/* c */ '…';
        • writes by push?.(, push ( or Array.prototype.push.call;
        • paired regex literals that hold quotes, which can hide a spread or a computed key from the recovery exemption. This needs deliberately crafted code.

Package size

  • No budget raise: this PR now fits main's budget.

    • Under the owner's practice as changed late on 2026-10-19, a PR raises PACKAGE_BUDGET only if it would exceed it. test: raise the package budget to main's size + 2% after #805 and #787 [PROTECTED: owner merges] #823 has merged, setting main's budget to 1,950,000 / 6,060,000.
    • Merge 640b6cd brings origin/main d5674d0 into this branch. With the dashboard UI built, npm pack --dry-run --json run inside the worktree measures 1,906,348 tarball, 5,950,518 unpacked (measured). That is under the budget by 43,652 / 109,482.
    • So I dropped this PR's earlier raises (0c1a7ff, e0f3f2c). test/package-files.test.ts is now main's exactly (measured: git diff origin/main -- test/package-files.test.ts is empty).
    • After the merge (measured): package-files ran 7 of 7, and the five affected test files ran 181 of 181.
  • This PR's own delta: +4,190 tarball, +16,107 unpacked, over main's 1,902,158 / 5,934,411.

  • What grew, file by file against main (measured, a per-file diff of the two npm pack --dry-run --json listings):

    File Unpacked bytes
    dist/src/first-task-progress.js (new) +8,846
    dist/src/hold-words.js (new) +3,758
    dist/web/assets/FirstRun-*.js +3,024 (92,928 → 95,952)
    dist/src/dashboard.js +406
    dist/web/assets/index-*.css (the card's classes) +73
    Total +16,107 (+4,190 tarball)

    Every other web asset only changed its hash name, at the same size. No sourcemap, .d.ts, test, fixture or stray file is packed, and the file count goes from 512 to 514: the two new modules.

  • Why it grew: the card's fold, the hold-words table, the card's component and the route's one field. All of it is code this PR adds on purpose.

How it was verified

  • Fail-without, round 4 (measured): I ran f93efce's tests on 27902ca. Result: 174 tests, 172 pass, 2 fail:

    • forged rows of every JS type: "5.number = {}", so the PR stage was done with no PR;
    • a card that cannot be built drops the card only: "500 !== 200".
  • Round 4 after (measured): first-task-progress, viewer-events, web-render and feature-map ran 174 of 174. The same tests on the tree merged with origin/main d4b4503 also ran 174 of 174, so the scan holds on Approvals: one role rule on every surface, decide only as root or the coordinator user, money paths read from main (#715) [PROTECTED: owner merges] #805's and Project commands run as the agent user only in a sandbox that hides its credentials; their output is redacted (#713) [PROTECTED: owner merges] #787's merge-flow.

  • Package, round 4 (measured, npm pack --dry-run; budget 1,900,000 tarball / 5,930,000 unpacked):

    Tree Tarball Over by Unpacked Over by
    Main d4b4503 alone 1,902,158 2,158 5,934,411 4,411
    Merged with f93efce 1,906,348 6,348 5,950,518 20,518
  • Fail-without, round 3 (measured): I ran the final tests at 27902ca on 0f5ccf2, with the test files checked out from 27902ca. Result: 167 tests, 166 pass, 1 fail. The failure is forged rows: "no attempt that is not a positive safe integer", actual 'attempt 1e+21 started; no finish recorded yet'.

    • The scan's tests carry their own scanner (test code), so its fail-without is the round-2 review's probe3 (measured by the reviewer). Each of probe3's missed rows is now reported by the scan on fixture text (measured: test the scan fails closed).
    • The sentence fixes are text, with no test (inferred: read against merge-policy.ts and merge-flow.ts).
  • Round 3 after (measured): at 27902ca, first-task-progress, viewer-events, web-render and feature-map ran 172 of 172. The tree merged with origin/main 128453a (merge-tree c9d81501, checked out in a scratch worktree) ran those four plus package-files: 179 of 179.

  • Package, round 3 (measured, merged with origin/main 128453a): tarball 1,899,687 (313 under the 1,900,000 budget), unpacked 5,927,388 (2,612 under 5,930,000). The budget is unchanged.

  • Fail-without, round 2 (measured): I ran the final tests on c7bae71 in a scratch worktree. Only the three test files came from 0f5ccf2. The command was the named-file node --import ./dist/test/isolate.js --test, with a scratch HOME, TMPDIR and state dirs and a stand-in claude. Result: 165 tests, 157 pass, 8 fail:

    • older head: "'held' !== 'waiting'";
    • no raw text: holds deep-equal fails (raw kinds were 'other' with their text);
    • source scan: src/coordinator/merge-flow.ts has unworded prefixes;
    • unknown prefix: "fixed words, never the text";
    • PR link without repo: "no repo given: no link (fail closed)";
    • no false progress: the block's text and the confidence wording;
    • followed: "no free text: not even the title";
    • web-render card: the card held "Round totals" (its title).
  • Fail-without, round 1 (measured, on 178c44e): 162 tests, 157 pass, 5 fail. These were the Start-card test, the "still warns" test, the card render, the /api/first-run route and the viewer test, as reported before.

  • After (measured):

    • At 0f5ccf2, first-task-progress: 14 of 14 pass.
    • At 9d22da1, first-task-progress, viewer-events, web-render and feature-map: 170 of 170 pass.
    • The tree merged with origin/main 128453a (git merge-tree of 9d22da1, checked out in a scratch worktree): first-task-progress, viewer-events, web-render, feature-map and package-files ran 177 of 177 pass. So the scan passes against main's current sources.
  • Package (measured, npm pack --dry-run --json, merged with origin/main 128453a):

    Tree Tarball (budget 1,900,000) Unpacked (budget 5,930,000)
    main 128453a alone 1,894,384 5,911,738
    merged with 9d22da1 1,899,892 5,927,848
    merged with 0f5ccf2 (comments shortened) 1,899,506 (494 under) 5,926,818 (3,182 under)
    • Correction: in round 1 I read the 15,738 / 41,743-byte gap between my local and CI numbers as a CI offset. It was main's growth since my base: main 128453a measures the same locally.
  • Checks (measured): typecheck clean; check:denylist "1312 files clean" (the round-6 reviewer's measurement on 640b6cd; it was 1298 on earlier heads); DCO "all commits signed off"; push gate clean.

  • CI: see the latest run on this head.

  • Browser check owed (still not done): run the verify-dashboard skill against a stand-in dashboard: scratch state, no real events DB, local mode. Cover the cases the reviews list:

    • the Start card;
    • the card just filed, held, after a new head, and blocked;
    • the PR link;
    • an unparseable filing time, and an unknown run reason;
    • a raw pr.opened with number -41;
    • a card that can't be built;
    • phone width in light and dark themes;
    • a viewer.

    Neither I nor any reviewer has run a browser.

Acceptance, each with a fail-without test

Acceptance line Test Fails without (measured)
Every step before Start done and first-task not: no Start warning web-render #741 the Start card counts only ... on 178c44e
A step before Start not done: still warns web-render #741 the Start card still warns ... on 178c44e (main counts 2)
From fixture events: claimed, verdict, PR opened, merged or held, for the filed issue only (including a decision from an older head) first-task-progress route test and older head test; web-render card route and render on 178c44e; older head on c7bae71
A held PR shows #706's plain words for its hold kind (every kind merge.decided carries) first-task-progress held, no evaluator text, source scan on c7bae71. Lint-only: "every kind gets its own words" rests on the source scan alone, a backstop with named gaps. The by-design fix is #824. A missed kind reads "Another reason (see the Board)", and nothing leaks.
A viewer's page carries no hold reason viewer-events #741 a held first task on 178c44e

By-design protection

Under the owner's 2026-10-19 rule, these are the protections. Each is a runtime check or a permission:

  • The page never goes down. The route builds the card only through firstTaskCard, whose own try returns null (the card left out) and logs once if the fold throws. Start, the steps and the policy form still answer.
    • Measured: a card that cannot be built drops the card only, with a fault injected into the fold.
    • Measured by the round-6 reviewer: 4,000 fuzzed forged logs gave 0 throws.
  • No forged or free text on the card. The fold checks each field at runtime. It renders only fixed words, members of the schema's own enums, positive safe integers, normalized times, and a link validated to this repo's PR page. holdWords maps reasons only through its fixed table, and any reason it doesn't know reads the fixed "Another reason (see the Board)".
    • Measured: forged rows, forged rows of every JS type, no evaluator text.
    • Measured by the round-6 reviewer: 0 forged marker words in 4,000 fuzzed logs.
  • A viewer sees nothing new. /api/first-run is writers only, so a viewer gets 403 (measured: viewer-events #741).

Backstops only, not protections:

What this can never do

  • Change a setting, merge, or hold anything (inferred from the diff; measured: a render test finds no button, form, input, select or textarea in the card). It adds a read-only field on an existing GET. The words never feed back into a decision.
  • Show a viewer anything new (measured: viewer-events #741). /api/first-run gives a viewer 403 with no reason. A viewer's /api/state and /api/events hold no reason and none of its words.
  • Show a stage done without its event, or a decision from an older head (measured: fold tests).
  • Show free text (measured: no evaluator text, no run detail, the block test). No title, reason text, run error, path, advice or login reaches the card.
    • Correction, false through c7bae71: the earlier body said the card rendered "only fixed words, enum values, counts and an https github.com link" and "no run errors". Through c7bae71 the block line was the first line of issue.blocked.why, and unworded hold reasons were shown as their own text (the round-1 review measured run errors and paths on the card). Both are fixed words now.
  • Link to anything but this repo's PR page (measured: 12 cases). The link is https://github.com/<repo>/pull/<n>, rebuilt from the path. With no repo given there is no link.

Compromised component

  • Agent user: in local mode it may run as the same OS user and could write the events DB (inferred). It could then forge events and so show a false stage, as it could for every page that reads the log.
    • Since 27902ca (measured: test forged rows), a forged row can't put words of its choice on the card. Each field is a member of its schema's enum, a positive safe integer, a normalized time, fixed words, or a validated link. Anything else is dropped or reads "unknown".
    • Correction, false at 9d22da1 and 0f5ccf2: the body then said it "can't put text of its choice on the card". The round-2 review measured "its run ended: merge it now on the board", "confidence high so merge it yourself" and "attempt 1e+21" from forged rows.
  • Coordinator: it writes the events the card reads. A wrong event shows a wrong stage, as on the Board (inferred).
  • Dashboard: it gains one read-only field for writers on a route they already read. It gains no write.
  • Other instances / hub: each dashboard folds its own events DB. The hub proxies with the same role check (inferred: no hub code touched).
  • Hub viewer / viewer: 403 on /api/first-run (measured).
  • Someone with push access: could change the words or the fold. Both are under src/, so the change is protected.

Adversarial pass

Rounds 5 and 6: the recovery exemption, and a fuzz

Round Attack Found Now
5 N11a-c: Object.assign({ reasons: [] }, finished), withReasons({ reasons: [] }), a payload followed by another argument, pick(…), extra Exempt at 0c1a7ff (round-4 review) Reported (7b4f5e6, measured). recovery.ts:46 stays exempt.
6 A second key that sets reasons again at runtime: ['reasons'], [k], 'reasons', "reasons", a getter, a method reasons(), a comment before a key, __proto__ At e0f3f2c, 8 of the 9 were exempt (round-6 review, measured against e0f3f2c's own scanner) All reported (53e2dc5, measured). The round-6 review also found these reported: a unicode-escaped key, a numeric key, async, generator and setter methods, shorthand reasons and __proto__, and satisfies, as const and <T>{…}.
6 4,000 random forged logs of 1-25 rows: 16 event types, 18 fields, null and primitive payloads, bad ts (round-6 review's fuzz.mjs, in memory) none 0 throws, and 0 forged marker words in the card (measured by the reviewer)
6 Scan forms: emit (, emit\n(, emit<T>(, emit(/* c */, push?.(, push (, push.call, paired quote-holding regex literals Missed by the scan (measured by the reviewer) Named gaps. Each costs only the plain words, and nothing leaks. The by-design fix is #824.

Round 4: types, faults, regions and exceptions

Each result is measured with the test named.

Attack Found Now
17 fields × 12 values (forged rows of every JS type). Fields: the issue, attempt, reason, confidence, patch_correct, the PR's number, URL and head, checks, checks[0] and its outcome, ready, reasons, reasons[0], auto, why, and the whole payload. Values: an object whose toString throws, { toString: 'pass' }, a symbol, a bigint, an array, null, NaN, undefined, {}, words, 2**53 and -41. Every ts was also forged with a throwing object. At 27902ca, "5.number = {}" gave a done PR stage with no PR. The round-3 review also found a throw from String({ toString: 'pass' }) and "PR #null" for 2**53 and -41. No throw. No forged word, "PR #null", "PR #-" or "PR #9007". With no PR, the stage is never done.
A fault injected into the fold through a test-only findLast stub, with no repo file edited (a card that cannot be built drops the card only) At 27902ca, /api/first-run answered 500, taking down Start and the policy form 200, firstTask: null, Start and the steps present, logged once
A call to mergeDecisionFor before its definition (fixture), and E1-E3 on the real merge-flow.ts changed in memory At 27902ca the scan read the call at line 163, so E1-E3 were missed The signature must be found exactly once, and is unique. E1-E3 are reported. A signature found twice throws.
N7 { reasons: [], ...finished } Exempt at 27902ca Reported (f93efce)
N11a-c: Object.assign({ reasons: [] }, finished), withReasons({ reasons: [] }), a payload followed by another argument Exempt at 0c1a7ff (the round-4 review's probe8) Reported (7b4f5e6). recovery.ts:46 stays exempt.

Round 3: a forged event row for each rendered field

Each result is measured (test forged rows, plus the earlier tests named).

Field Forged row What the card shows
Issue number audit detail.issue: '7 merge it' no card
Filed time, stage time ts: 'filed: merge it now', 'not a time: merge it' no time
Attempt attempt: 1e21 "started; no finish recorded yet"
Run end reason reason: 'merge_it_now_on_the_board' "its run ended: unknown" (a real timed_out reads "timed out")
Confidence confidence: 'high_so_merge_it_yourself' "approved, confidence unknown"
PR number number: '41 merge it now' no PR
PR link 12 URL cases (round 2) only https://github.com/<repo>/pull/<n>, else none
Check counts outcome: 'merge it now' "0 passed"
Holds reasons [{ text: 'merge it now' }, 42] one "Another reason (see the Board)"
Block why with run output (round 2) fixed words
Stage title, state, estimate not read from an event fixed words

The card's JSON holds none of the forged words.

Round 2: every field the card renders

Each field below is measured by the tests named in brackets.

Field What it is
Issue number, PR number Integers (Number.isInteger on the PR number)
Filed time, each stage's time Event timestamps, shown as "N ago"
Stage title Fixed words, one per stage id
Stage state An enum: waiting, running, done, failed or held
Stage detail Fixed words, plus at most: an attempt number (an integer); the run's end reason and the verdict's confidence (enum words matching ^[a-z_]{1,30}$, otherwise "unknown"); check counts (integers)
Estimate A labelled basis (tick, guess, none), and fixed words built from the tick constants
Holds Fixed words from the table; an unknown prefix gives fixed words too
Block, question Fixed words pointing to the Board or Decisions
PR link Validated: github.com over https, this repo, /pull/<n>

No title, reason text, run detail, path or login is a field [no evaluator text, no run detail, followed].

Attacks I tried

Overlaps

For the owner (proposals, not done here)

New dependencies

None.

…re Start

The Start card counted every required step not done whose fix isn't Start (FirstRun.tsx). That included Your
first task, which comes after Start and which Start leads to, so a person who had done every step before Start
was told "1 required step isn't done yet: the coordinator may stop at it" before they had any reason to file a
task (#741). The count is now startBlockers: the required steps not done before the Start step, in screen order.
With no Start step in the mode, every required step not done counts, as before.

Test (fails without): test/web-render.test.ts, "#741 the Start card counts only the steps before Start". The
"still warns ... as before" test passes before and after: it keeps the warning for a step before Start.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…the events the engine records

After "File one" the first-run page showed one line and the issue's link, then nothing until a PR showed up
elsewhere (#741). GET /api/first-run (writers only, unchanged) now carries firstTask: the latest issue the form
filed (its first_run.first_task_filed audit entry), followed through that issue's own events recorded after it:
claimed, the worker's run, the evaluator's verdict, the PR the harness opened for it, that PR's checks, then
merged, closed, or held. Nothing is guessed: a stage with no event is waiting, a verdict from an earlier
attempt is stale once the worker runs again, and only events of that PR number count for its merge.

Each stage not done carries an estimate that says its basis: the coordinator's tick (60 s plus up to 5 s; a
test keeps it equal to src/service.ts), the median of earlier tasks here, a labelled guess (the review's
15-60 minutes to a PR, only when nothing here has timed one), or none yet.

A held PR's reasons are said in plain words, one line per kind, with what lets it through
(src/hold-words.ts: the table #706 slice 2 asks for, one entry per reason prefix mergeDecision emits; an
unknown reason shows as its own text). In local mode the kill switch reads as #729 words it: every PR waits
for you; no policy.yaml is named. The PR is linked only by GitHub's https page of that PR in this repo. The
card shows no run path, run error text, advice or login. It is read-only, and viewers still get 403.

Test (fails without): test/first-task-progress.test.ts (the /api/first-run test fails on main: firstTask is
undefined), test/web-render.test.ts ("#741 the First task card"), test/viewer-events.test.ts ("#741 a held
first task").

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…medians, shorter modules

CI's package-size test failed on f739109: the tarball 1900939 bytes (budget 1900000), unpacked 5933368
(budget 5930000). Raising PACKAGE_BUDGET is the owner's call, so the card is made smaller instead.

The per-stage medians of earlier tasks are gone: on a first run there is no earlier task, so they never showed
where the card matters. The worker, evaluator and PR stages now carry the review's guess, labelled as one
(15-60 minutes from the claim to a PR, for these stages together); checks say there is no estimate; the claim
keeps the coordinator's tick. first-task-progress and hold-words are written more compactly, with the same
behaviour otherwise. Locally, npm pack --dry-run: main 1879443 / 5869995, this branch 1883548 / 5885690.

Test (fails without): test/package-files.test.ts (the size budget) failed in CI on f739109;
test/first-task-progress.test.ts now asserts the guess and the absence of a made-up CI estimate.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (round 1)

Reviewed at head c7bae71, judged by the DECISION GUIDE. The Start count is correct and causes no regression. The fold handles another issue, an older PR, events before the filing, a refiled task, duplicates and a retried worker. Two things are wrong. (1) The merge stage can show a false state after the PR's head moves. (2) The card renders raw free text, run errors, paths and evaluator text, through the block line and through hold reasons that mergeDecision doesn't emit but merge.decided carries. There is also no test that fails when a new hold prefix is added. Every fix below follows principles 1, 4 and 5, so none needs an owner decision.

Scratch probes, fixtures only, no repo file edited: work/eval-816-r1/probe.mjs and probe2.mjs in the session scratchpad, run against the built head with a scratch HOME and state dirs.

Acceptance criteria

Criterion State Evidence
Every step before Start done and first-task not: no Start warning met measured: web-render #741 the Start card counts only the steps before Start fails on main 128453a and passes on head
A step before Start not done: still warns met measured: web-render #741 ... still warns fails on main ("Claude login, before Start, counts; Your first task, after it, does not") and passes on head
From fixture events: claimed, verdict, PR opened, merged or held, for the filed issue only partially measured: the PR's fixtures pass. Probe 1 shows a false merge stage after a new head (required change 1)
A held PR shows #706's plain words for its hold kind partially measured: every mergeDecision prefix has words. The kinds merge-flow and the conflict fix add to merge.decided show their raw text (probe 4, required change 2)
A viewer's page carries no hold reason met measured: viewer-events #741 a held first task passes on head. On main it fails ("Cannot read properties of undefined (reading 'holds')")

Fail-without (measured)

  • Setup: I copied the PR's three test files and its two new pure modules onto origin/main 128453a in a scratch worktree, then built with npx tsc -p tsconfig.json.
  • Command: node --import ./dist/test/isolate.js --test dist/test/first-task-progress.test.js dist/test/viewer-events.test.js dist/test/web-render.test.js, with a scratch HOME, TMPDIR and state dirs and a stand-in claude first on PATH.
  • Result: 162 tests, 157 pass, 5 fail. These are the builder's five: the route test, viewer-events #741, and web-render tests 60, 61 and 62. The failing lines match the PR body.
  • On head c7bae71 (measured): first-task-progress, viewer-events, web-render, feature-map, merge-policy, brand and package-files ran 198 tests, 198 pass.
  • Other checks on head (measured):
    • npm run typecheck is clean.
    • npm run check:denylist reports "1298 files clean".
    • npm run build:web builds.
    • node scripts/check-dco.mjs 178c44e6 c7bae719 reports "all commits signed off".
    • No Co-Authored-By or "Generated with" in the commits.

Can the card show a false state? (check 1)

Attack Result
Another issue's events; an older PR of the same issue; events before the filing No effect (measured: the PR's test, re-run)
A refiled first task The latest filing is followed (measured)
Duplicated or reversed event arrays Same stages (measured: probes 2a and 2b)
A worker retry The earlier verdict goes stale (measured)
A closed PR that is then reopened It stays "closed". watchedPrs drops a PR at pr.closed and the engine records nothing after, so the card matches the engine's own view (inferred: src/coordinator/merge-flow.ts:237-245)
A human merges outside the harness watchPr records pr.closed merged:true, and the card says "merged on GitHub" (inferred: merge-flow.ts:299-300 and the fold)
A restart mid-run Shows "attempt N started; no finish recorded yet" until the reclaim. That is literally true (inferred)
A new head after a decision False state (measured, probe 1). See required change 1
A claim recorded just before the filing's audit entry "Claimed: waiting" while "Worker: running" (measured, probe 3). The window between creating the issue and the audit append is milliseconds (inferred). Not required
  • Estimates: labelled correctly (measured: the PR's tests).
    • Claimed: "Estimate", tied to src/service.ts by a test.
    • Worker, evaluator and PR: "Guess".
    • Checks and merge: "no estimate".

Leaks (check 2)

  • Viewer: gets 403 on /api/first-run. Its /api/state and /api/events carry no reason (measured).
  • PR link: github.com over https, this repo, /pull/<n> only. It is rebuilt from the path, so a query, fragment, port or userinfo never survives (measured: 11 cases; inferred: prLink code).
  • Raw text on the card (measured): the card does render raw text, so the PR body's claim that it renders "only fixed words, enum values, counts and an https github.com link" is false.
    • The block line is the first line of issue.blocked.why, up to 200 characters. Probe 2 put worker run ended: failed (ENOENT /home/agent/wl/work/issue-7/.git) on the card. Many block() callers embed run detail: src/coordinator.ts:715,797,1240,1373 and src/coordinator/landing.ts:184.
    • Hold reasons with no table entry are echoed, up to 300 characters. Today these are not hypothetical. merge-flow.ts adds them to the same merge.decided reasons:
      • "GitHub can't merge it cleanly (…)" (merge-flow.ts:589)
      • "merging failed N times: <GitHub's refusal>" (:591)
      • "GitHub was still computing…" (:586)
      • the light-check conflict (:603)
      • the combined gate's holds, including "the fast tests fail: <the last 3 lines of test output>" (src/coordinator.ts:1821,1842)
      • the docs-only ruleset hold (src/pr-rules.ts:162)
      • conflictWaitReasons, including "the evaluator's confidence is medium, not high: " (src/conflicts.ts:172-176, via merge-flow.ts:542)
    • Probe 4 shows evaluator text and home paths reaching holds[].why.
    • Writers can already read this text on /api/events and the Board, so this adds no new reach (inferred). It does break the card's stated bar of no run errors, paths or evaluator text.

Principle 6 (check 3)

Read-only. The card has one <a> and no button, form or write path (inferred: web/src/components/FirstTaskProgress.tsx). No test asserts that it has no control. That is optional.

No regression (check 4)

  • The Start warning renders only when start.kind === 'local' (web/src/pages/FirstRun.tsx:458,473). A server instance gets owner-command, and setup mode gets unsupported (src/dashboard.ts:313-324). So the change is visible only in local mode (inferred).
  • In local mode, after Start come only linger (optional) and first-task (src/setup-steps.ts:199-201,1068-1114). first-task gates nothing: its own detect says the coordinator "has nothing to work on" (inferred). startBlockers hides no step that really blocks the coordinator.
  • Server mode has its start-kind step (coordinator) before protection, policy and updates, so startBlockers would drop those. Its warning is not rendered before or after this change (inferred), so nothing changes there.
  • Existing first-run tests pass on head (measured: web-render).

Hold-words coverage (check 5)

  • Today every mergeDecision prefix has words. Measured: the PR's test, plus my scan of src/merge-policy.ts's push( literals, which found no prefix without words.
  • No test enforces it for a prefix added later. The PR's test is input-driven: it holds only on the inputs it lists. A new reasons.push('stale: …') behind a new MergeInput field passes it untouched (inferred from the test's code).
  • A source scan catches this. Measured: my scan of a fixture copy of the source with that line added reports ["stale:"] without words.
  • Open Merge: a PR that removes or weakens a test, lint or guard waits for the owner (#783) [PROTECTED: owner merges] #820 shows the gap is real. It adds design: may weaken a check: …, which maps to the design sentence. That sentence lists its causes as "the evaluator flagged one, a dependency manifest changes, or it adds a top-level module", so it would be incomplete for a weakening hold (measured: git grep at a3fc766 src/merge-policy-weakening.ts:719). This is a merge-order follow-up for whichever lands second, not a change for this PR alone.

Protection, scope, overlaps

CI (measured)

  • All checks green on c7bae71. The PR's run tested merge commit e4fa253 ("Merge c7bae71 into 128453a"), which is current main.
  • Package budget: test shard 2/4 reports "ok 708 - the package stays within its size budget".
  • Headroom is small: about 700 tarball bytes, the builder's inferred figure. The fixes below add words, so they may push it over. See the owner note.

Required changes

  1. The merge stage must not use a decision or merge failure from an older head.
    • Today (measured, probe 1):
      • A merge.decided auto:false at head A, then a review fix pushes head B whose checks are pending. The card says merge "held" with A's design reason.
      • Variant 1b: an auto:true decision at A, then head B's checks fail. The card says "the merge policy will merge it".
    • Fix: count merge.decided and merge.failed only when their head equals the PR's latest known head (the latest pr.status head, or pr.opened's). Otherwise the stage waits ("a new head: it waits for its checks and a fresh decision").
    • Test: add both fixtures as a test that fails without the fix.
  2. The card must carry no raw run or evaluator text.
  3. A new hold prefix must fail a test.
    • Scan: extract the reason literals from the sources that feed merge.decided (src/merge-policy.ts, the d.reasons.push sites in src/coordinator/merge-flow.ts, src/conflicts.ts conflictWaitReasons, src/pr-rules.ts rulesetHold and src/coordinator.ts combinedGate), and assert each one has a table entry.
    • Proof: feed the scan a fixture source with an added prefix, not a planted repo file (rule 8), to show it catches a new one.
  4. Correct the PR body (principle 5).
    • Remove "renders only fixed words, enum values, counts and an https github.com link" and "no run errors", or make them true through change 2.
    • Commit 1's message says the "still warns ... as before" test "passes before and after", but it fails on main (measured). The PR body has it right. Say so in the body. No history rewrite is needed.

Not required (suggestions)

  • Fail closed in prLink: it accepts any repo when repo is null. The route always passes one (the config schema requires owner/repo, src/config/schema.ts:93), so this is unreachable today (inferred). Making repo required would close it for good.
  • Read-only test: a render assertion that the card holds no <button> or <form> would pin principle 6.

For the owner

  • Package budget (a new cost): CI measured this PR within budget on today's main, with about 700 bytes of headroom (the builder's inferred figure). The words in change 2 may push it over. If they do, the builder should not loosen the check (rule 10). The choice is an owner raise of PACKAGE_BUDGET or a smaller card. That is your call.

Browser check owed (human)

Use the verify-dashboard skill against a stand-in dashboard (scratch state, no real events DB). Before and after screenshots of First run in local mode, with zero new console errors, showing:

  • Start card: with every step before Start done and Your first task not, there is no orange "required step isn't done" line. With Claude login not done, it reads "1 required step isn't done yet".
  • First task card, just filed: it sits under Start. All six stages are "waiting". Claimed reads "Estimate: within about 65 s …". Worker, Evaluator and PR read "Guess: 15-60 minutes …". Checks reads "Estimate: no estimate …".
  • Held: Merged is "held" with an orange dot. Each hold shows its sentence and "What lets it through: …". In local mode the kill switch reads "Local mode: every PR waits for you". No policy.yaml and no raw reason appears.
  • PR link: "PR #n" links to https://github.com/<repo>/pull/<n> and opens in a new tab.
  • Layout: a long title and a long block line wrap at phone width (375 px) without horizontal scroll, in light and dark themes.
  • Viewer: a viewer login gets the first-run refusal and no card.

…n from an older head

Review round 1 on #816 (issuecomment-6272706512):

- A merge.decided or merge.failed counts only at the PR's latest known head (its latest pr.opened, pr.status,
  pr.ready or pr.unready). A held decision at head A followed by a push of B no longer shows A's hold; an auto
  decision at A followed by failing checks on B no longer says "will merge it". The stage waits: "a new head:
  it waits for its checks and a fresh decision".
- The card carries no free text. The block line is fixed words pointing to the Board (never issue.blocked.why);
  the issue's title is gone; a run's end reason and a verdict's confidence pass only as enum words.
- src/hold-words.ts words every kind merge.decided carries today, not only mergeDecision's: GitHub can't merge
  cleanly, merging failed, GitHub still computing, the combined check (conflict, no branch point, fast tests
  failing), the docs-only ruleset hold, and conflict fix. An unknown prefix reads "Another reason (see the
  Board)", never its text. The design sentence now also names a possible weakened check (#820), worded as
  examples so it holds whether or not #820 lands first.
- A new hold prefix fails a test by design: a source scan over every source of merge.decided reasons
  (merge-policy.ts, merge-flow.ts mergeOrWait, coordinator.ts combinedGate, pr-rules.ts rulesetHold, conflicts.ts
  conflictWaitReasons) checks each literal reason's prefix has words, and that every reason pushed from a
  variable is a traced one. Fixture text proves it catches a new prefix and a new untraced source.
- prLink fails closed: no repo given, no link.

Correction to 11c9f66's message: its "still warns ... as before" test does not pass before; it fails on main,
which counts 2 steps (Your first task too) where the test expects 1.

Test (fails without): test/first-task-progress.test.ts, "#741 a decision or a failed merge from an older head",
"#741 the card shows no evaluator text, path or run detail", and the source-scan test (its fixture lines).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…kage headroom

Merged with origin/main 128453a, npm pack --dry-run measured 1899892 tarball bytes of a 1900000 budget at
9d22da1. Only comments change.

Test (fails without): none; behaviour unchanged (test/first-task-progress.test.ts passes as before).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

Copy link
Copy Markdown
Owner Author

Round 2: what changed (head 0f5ccf2)

  1. Decisions from an older head (measured, test older head): merge.decided and merge.failed now count only at the PR's latest known head. In both of the review's cases the merge stage now waits: "a new head: it waits for its checks and a fresh decision". A decision at the new head counts again.
  2. Fixed words only (measured, test no evaluator text):
    • The block line is fixed words pointing to the Board.
    • The issue title is gone.
    • A run's end reason and a verdict's confidence pass only as enum words.
    • src/hold-words.ts now words every kind merge.decided carries: mergeDecision's kinds, can't merge cleanly, merging failed, still computing, the combined check (three forms), the docs-only ruleset hold, and conflict fix.
    • An unknown prefix reads "Another reason (see the Board)".
    • A fixture full of evaluator text, home paths and ENOENT lines leaves none of them in the card's JSON.
  3. A new prefix fails a test (measured): I built the source scan, not the shared kind list. The shared list would touch five files held by open PRs.
  4. Claims corrected:
    • The PR body's "only fixed words … no run errors" now carries a "false through c7bae71" note.
    • Commit 1's "passes before and after" is corrected in the body and in 9d22da1's message, with no history rewrite.
    • My round-1 "CI offset" inference was wrong: the gap was main's growth.

Also:

  • prLink fails closed when no repo is given (measured).
  • A render test finds no button, form, input, select or textarea in the card (measured).

Fail-without on c7bae71 (measured): 165 tests, 157 pass, 8 fail. These are the 7 new or changed fold tests and the web-render card test.

Package (measured, npm pack --dry-run of the tree merged with origin/main 128453a): tarball 1,899,506 (494 under the 1,900,000 budget), unpacked 5,926,818 (3,182 under 5,930,000). The budget is unchanged.

Merged with main (measured): first-task-progress, viewer-events, web-render, feature-map and package-files ran 177 of 177 pass.

@jayden1711

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (round 2)

Reviewed at head 0f5ccf2, judged by the DECISION GUIDE. All four round-1 changes are in. Old-head decisions now wait, the block line and hold kinds are fixed words, and the claims are corrected. Three gaps remain, all cheap:

  1. The source scan misses several ordinary ways a new hold reason can reach merge.decided. One of them is a function that feeds merge.decided today and is not scanned.
  2. A forged event row can still put words of its choice on the card. That makes the Compromised component claim false.
  3. Some of the hold sentences are not quite true.

Every fix follows principles 1, 4 and 5, so none needs an owner decision.

Probes (fixtures only, no repo file edited) are in the session scratchpad under work/eval-816-r2/:

  • probe3.mjs: a verbatim copy of the test's scan, because unworded isn't exported. It is fed repo sources that were changed in memory only.
  • probe4.mjs: forged rows.
  • The round-1 probe.mjs and probe2.mjs, re-run.

Everything ran with a scratch HOME, TMPDIR and state dirs, and a stand-in claude first on PATH.

Round-1 changes

# Required State Evidence
1 No decision or failed merge from an older head met measured: probe 1 (a held decision at A, head B pending) gives merge "waiting: a new head: it waits for its checks and a fresh decision", outcome open, no holds. Probe 1b (an auto decision at A, head B failing) gives the same wait. Test older head fails on c7bae71 ("'held' !== 'waiting'")
2 Fixed words only on the card partially measured: probe 2's block gives {"at":…} only. Probe 4's five raw reasons map to worded kinds (conflict-fix, combined, merge-failed, cant-merge, ruleset), and an unknown one reads "Another reason (see the Board)." The word() filter still lets a forged row choose words (required change 2)
3 A new hold prefix fails a test partially measured: the scan catches a new pushed literal or template, an untraced pushed variable and a new const why literal. It misses the cases in required change 1
4 Claims corrected met, except one new claim the body carries both corrections. The #820 follow-up and Compromised component claims are new and wrong (required changes 2 and 4)

Check 2: attacking the source scan (measured, probe3.mjs)

Emitters. grep -rn "merge\.decided" src finds exactly one emitter, this.emit('merge.decided', …) at src/coordinator/merge-flow.ts:613 in mergeOrWait. That emitter is scanned. Every other hit is a reader. No dynamic or other-quoted form exists.

What feeds that emitter's reasons:

  • mergeDecisionFor (merge-flow.ts:495-544): mergeDecision, plus [...d.reasons, ...fix.reasons] from conflict_fix.finished. Not scanned.
  • The conflict-fix reasons are built at src/coordinator/fix-runs.ts:594-601 from conflictWaitReasons (scanned). fix-runs.ts itself is not scanned.
  • mergeOrWait's pushes, combinedGate's { hold }, and rulesetHold's why: all scanned.

Each attack below adds one line to the real source text in memory and runs the scan on it:

Attack Result
Helper pushed directly: d.reasons.push(this.staleWhy(polls)) caught (untraced push)
Helper through a variable: const why = this.staleWhy(polls); d.reasons.push(why) missed
Lookup pushed directly: d.reasons.push(STALE_WORDS[kind]) caught
Lookup through why: const why = STALE_WORDS[kind]; d.reasons.push(why) missed
combinedGate returns { hold: HOLDS.stale } / { hold: staleHold(mainSha) } missed (a non-literal hold: is never checked)
Untraced variable d.reasons.push(whyStale) / d.reasons.push(gate.holdText) missed (traced names match by prefix: 'why', 'gate.hold')
d.reasons.unshift('stale: x') / d.reasons = [...d.reasons, 'stale: x'] / d.reasons.splice(0, 0, 'stale: x') missed (only .push( is read)
mergeDecisionFor returns reasons: [...d.reasons, ...fix.reasons, 'stale: …'], or pushes 'stale: …' missed (not a scanned region)
merge-policy: reasons.push(...(i.instructionEvalsExtra ?? [])) missed (prefix of a traced spread)
merge-policy: const reasons: string[] = ['stale: x', ...neverAutoMerge(…)] missed
A push after conflictWaitReasons in fix-runs.ts missed (inferred: the file is not in REASON_SOURCES)
A new file that emits merge.decided missed (inferred: the test reads only the five REASON_SOURCES files and never lists emit sites)

The card stays safe in every missed case: the reason reads "Another reason (see the Board)", never its text (measured). So a gap costs the owner the plain words, not a leak. Round 1 asked that a new prefix fail a test, and these cases don't.

Check 3: #820

Check 4: every rendered field (component and API read; probe4.mjs)

Field Verdict
Issue #, PR # integers (audit detail.issue must be an integer > 0; Number.isInteger on the PR number)
filed / stage times event ts, shown through ago()
Stage title, state badge, block, question, estimate labels fixed words / enum
Estimate text fixed words built from the tick constants (test keeps them equal to src/service.ts)
Check counts integers
Holds fixed words from the table; unknown gives fixed words
PR link validated: https, github.com, no userinfo, /<repo>/pull/<n>, rebuilt from the path; no repo gives no link
Run end reason, verdict confidence not an enum. word() accepts any ^[a-z_]{1,30}$. EventLog.read does not re-validate (src/events/log.ts:204-216). Measured with forged rows: reason merge_it_now_on_the_board renders "its run ended: merge it now on the board"; confidence high_so_merge_it_yourself renders "approved, confidence high so merge it yourself".
Attempt Math.trunc(Number(x)) || 1: a forged 1e21 renders "attempt 1e+21" (measured). This is minor, but it is not an integer check

The PR body's Compromised component says that since 9d22da1 the agent user "can't put text of its choice on the card". That is false: the agent user writes the DB directly, as the body itself assumes.

Check 5: no regression

  • Start card and route: web/src/pages/FirstRun.tsx and src/dashboard.ts are unchanged since c7bae71 (measured: git diff --stat c7bae719 0f5ccf24). Round 1's analysis stands.
  • Viewer: viewer-events #741 passes on head, with 403 on /api/first-run and no reason or its words on /api/state and /api/events (measured).
  • Existing first-run flow: web-render passes (measured).

Check 6: the usual checks (measured)

Required changes

  1. Close the cheap scan gaps (test-only, no package cost).
    • a. Assert the set of merge.decided emitters: scan every src/**/*.ts for an emit or append of 'merge.decided', and require each one to sit in a scanned region. Today there is one, in mergeOrWait.
    • b. Add the two regions that feed that emitter today:
      • mergeDecisionFor in merge-flow.ts;
      • the conflict-fix reasons site in src/coordinator/fix-runs.ts, or an assertion that it is conflictWaitReasons(…) and nothing else.
    • c. Make the scan fail closed.
      • Traced entries match whole expressions, not prefixes, so whyStale and instructionEvalsExtra fail.
      • A const why =, why: or hold: with a non-literal value must be traced.
      • Any other write to the reasons array (unshift, splice, a reassignment or spread, an array-literal initializer) fails unless it is traced.
    • Prove each with fixture text, as now: every missed row in the table above.
  2. Pass the run's end reason and the verdict's confidence only as members of the schema's own enums, else "unknown".
    • The enums are EventSchemas['run.finished'].shape.reason.options and EventSchemas['eval.verdict'].shape.confidence.options.
    • Show the attempt only when it is a positive safe integer.
    • Add a test with probe 4's forged rows.
    • Correct the Compromised component sentence (principles 1 and 5).
  3. Make the hold sentences true (principle 5, src/hold-words.ts).
    • design says "a dependency manifest or a top-level module is added". The reason fires when a dependency manifest changes (merge-policy.ts:219-220).
    • doubt's parenthesised list reads as complete but leaves out three causes: no verdict, the evaluator doubts the test, and no design-change answer (merge-policy.ts:253-260). Either list them or say "for example".
    • merge-failed says "3 times", but the reason fires at 3 or more (merge-flow.ts:591, inferred). Say "at least 3 times".
  4. Correct the Merge: a PR that removes or weakens a test, lint or guard waits for the owner (#783) [PROTECTED: owner merges] #820 follow-up in the body. The traced entry alone passes the scan but leaves src/merge-policy-weakening.ts unscanned (measured above). The follow-up is: the traced entry, a region for weakeningReasons and weakeningOf, and a ${WEAKENING_REASON} substitution. Change 1c's design could make this impossible to miss: each traced spread names the region that builds it.

Not required

  • Probe 3 from round 1 (a claim recorded before the filing's audit entry) still shows "Claimed: waiting" while the worker runs, for milliseconds (inferred).
  • Changes 2 and 3 add a few bytes to dist/src. Change 1 is in test/, which isn't packed (measured: package.json files). If the package goes over the budget, raising it is the owner's call (rule 10). The builder must not loosen the check.

Browser check still owed (human)

Use the verify-dashboard skill against a stand-in dashboard (scratch state, no real events DB). Take before and after screenshots of First run in local mode, with zero new console errors:

  • Start card: with every step before Start done and Your first task not, there is no orange "required step isn't done" line. With Claude login not done, it reads "1 required step isn't done yet".
  • First task card, just filed:
    • The header reads "First task #n filed … ago", with no title. The card sits under Start.
    • All six stages are "waiting".
    • Claimed reads "Estimate: within about 65 s …".
    • Worker, Evaluator and PR opened read "Guess: 15-60 minutes …".
    • Checks reads "Estimate: no estimate …".
  • Held:
    • Merged is "held" with an orange dot.
    • Each hold shows its sentence and "What lets it through: …".
    • In local mode the kill switch reads "Local mode: every PR waits for you."
    • An unknown reason reads "Another reason (see the Board)."
    • No policy.yaml and no raw reason appears.
  • A new head after a held decision: Merged is "waiting" with "a new head: it waits for its checks and a fresh decision", and there are no holds.
  • Blocked: the line reads "Blocked: it waits for you on the Board, which says why.", with no run text.
  • PR link: "PR #n" links to https://github.com/<repo>/pull/<n> and opens in a new tab.
  • Layout: the hold sentences wrap at phone width (375 px) without horizontal scroll, in light and dark themes.
  • Viewer: a viewer login gets the first-run refusal and no card.

…ema type; the hold scan fails closed

Review round 2 on #816 (issuecomment-6273455831):

- Forged rows (the event log is never re-validated on read): a run's end reason and a verdict's confidence show
  only as members of the schema's own enums (EventSchemas run.finished reason, eval.verdict confidence), else
  "unknown"; the attempt, the PR number and the filed issue only as positive safe integers; every time only as a
  parseable date (normalized), else dropped. A forged "merge_it_now_on_the_board", "high_so_merge_it_yourself" or
  1e21 no longer reaches the card.
- Hold sentences made true: design names a dependency manifest that changes; doubt's list says "for example" and
  names no verdict, a doubted test and no design answer; merge-failed says "at least 3 times".
- The source scan fails closed:
  - every emitter of merge.decided and conflict_fix.finished in src/ must sit in a scanned region (a
    conflict_fix.finished with reasons: [] in the call itself is allowed: recovery's "interrupted");
  - new regions: mergeDecisionFor, fix-runs.ts conflictRun and giveUpConflict, and merge-policy's reason
    functions one by one;
  - traced values match whole expressions (a call: any arguments), each naming the region that builds what it
    carries in, and that region must be scanned;
  - a non-literal why/hold value or reasons initializer, and any unshift, splice or concat, fail unless traced;
  - the tokenizer skips strings, templates and comments.
  Each way round it the review found is proved on fixture text. Known gaps, named in the test: a traced helper
  or table lookup that changes what it returns, and reasons passed in through a new MergeInput field (the push in
  mergeDecision is scanned, the field's builder is not unless traced).

Test (fails without): test/first-task-progress.test.ts, "forged rows" and "the scan fails closed".

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

Copy link
Copy Markdown
Owner Author

Round 3: what changed (head 27902ca)

  1. Forged rows (measured, test forged rows).
    • The run's end reason and the verdict's confidence pass only as members of EventSchemas['run.finished'].shape.reason.options and EventSchemas['eval.verdict'].shape.confidence.options, else "unknown".
    • The attempt, the PR number and the filed issue pass only as positive safe integers.
    • Every time passes only as a parseable date, normalized to ISO; anything else is dropped.
    • Your probe-4 rows now render "its run ended: unknown", "approved, confidence unknown" and "started; no finish recorded yet".
    • The Compromised component sentence is corrected, with a "false at 9d22da1 and 0f5ccf2" note.
  2. True sentences:
    • design: "a dependency manifest changes";
    • doubt: "for example ...", adding no verdict, a doubted test and no design answer;
    • merge-failed: "at least 3 times".
  3. The scan, the cheap parts (test-only, measured):
    • Emitters: every emitter of merge.decided and conflict_fix.finished in src/ must sit in a scanned region. This found recovery.ts's conflict_fix.finished, which is allowed because its call has reasons: [].
    • New regions: mergeDecisionFor, fix-runs conflictRun and giveUpConflict, and merge-policy's reason functions one by one.
    • Fails closed:
      • traced values match whole expressions, and each names its building region, which must be scanned;
      • a non-literal why:, hold: or const why fails unless traced;
      • any unshift, splice or concat fails;
      • a reasons initializer, reassignment or array literal is checked element by element.
    • Proof: each missed row in your probe-3 table is now reported, on fixture text.
    • Known gaps (named in the test and the body): a traced helper or table lookup that changes what it returns, and a new MergeInput field. Both cost only the plain words, not a leak.
  4. Merge: a PR that removes or weakens a test, lint or guard waits for the owner (#783) [PROTECTED: owner merges] #820 follow-up corrected in the body (measured on the tree merged with Merge: a PR that removes or weakens a test, lint or guard waits for the owner (#783) [PROTECTED: owner merges] #820): the scan reports untraced ...(i.weakening ?? []). The follow-up is the traced entry, a region for the weakening builder, and a ${WEAKENING_REASON} substitution. The test enforces the first two together.

Fail-without on 0f5ccf2 (measured): 167 tests, 166 pass, 1 fail (forged rows). The scan's tests carry their own scanner, so their fail-without is your probe 3.

Package (measured, npm pack --dry-run of the tree merged with origin/main 128453a): 1,899,687 tarball (313 under), 5,927,388 unpacked (2,612 under). The budget is unchanged.

Merged-tree tests (measured): first-task-progress, viewer-events, web-render, feature-map and package-files ran 179 of 179.

@jayden1711

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (round 3)

Reviewed at head 27902ca and judged by the DECISION GUIDE. Most of round 2 is in:

  • probe 4's forged words are gone;
  • the three hold sentences are true;
  • the emitter set is complete;
  • every probe-3 row is now caught, except one.

Two defects remain, both cheap:

  1. The new mergeDecisionFor region scans the wrong four lines. So the real mergeDecisionFor, which feeds merge.decided today, is still unscanned, and the body's "each missed row is now reported" is false for it.
  2. A forged row of the wrong type can make the card's fold throw. That would take down /api/first-run, the page that also carries Start and the policy form.

Both fixes follow principles 1, 4 and 5, so neither needs an owner decision.

Probes (fixture text and in-memory rows only, no repo file edited) are in the session scratchpad under work/eval-816-r3/:

  • probe5.mjs runs probe 3's attacks and new ones against the round-3 scan. It loads the scan verbatim from the compiled test (dist/test/first-task-progress.test.js, from const REGIONS up to const srcFiles), so it can't drift from the test.
  • probe6.mjs re-runs probe 4 and forges every field the builder didn't list.

Everything ran with a scratch HOME, TMPDIR and state dirs, and a stand-in claude first on PATH.

Check 1: forged rows (measured, probe6.mjs)

  • Probe 4's rows: "its run ended: unknown", "approved, confidence unknown", and "started; no finish recorded yet". No forged word appears in the JSON.
  • Other fields forged, no forged word on the card ("clean"):
    • times: 'zork 2026-10-19', '… zork', '+275760-…', '0', '1e21', ' ', and a number. Each one is either normalized to ISO or dropped.
    • attempt: '2', 2**53, 1.5;
    • reason and confidence: ['succeeded'], '__proto__', 'toString', {};
    • reasons: nested arrays, a string, null entries;
    • checks: a string;
    • block, question, merge.failed, pr.closed, land.result, an opened.url to another PR, and repo: null.
  • Two defects:
    • A throw. pr.status with checks: [{ outcome: { toString: 'pass' } }] throws "Cannot convert object to primitive value" from String(c.outcome) at src/first-task-progress.ts:115. .map(String) on merge.decided reasons at :127 throws the same way on such an entry (inferred: same call). That object is valid JSON, so the events DB can hold it.
      • The fold runs inside the /api/first-run handler (src/dashboard.ts, eventCache.memo('firstTaskProgress', …)). The top-level catch at src/dashboard.ts:2822 answers 500 (inferred from the code; I made no DB row).
      • Every writer then loses the whole first-run page. That includes Start and the PolicyForm (web/src/pages/FirstRun.tsx:618). On main the route has no such fold.
      • Principles 1 and 2. It also contradicts the body's "Anything else is dropped or reads unknown".
    • "PR #null". pr.opened with number: 2**53 or -41 gives p.pr === null, but the PR stage still reads done, "PR #null", and Merged reads "no merge decision yet…". The existing test asserts only p.pr === null. No words leak, but the stage is shown done from an invalid row.

Check 2: the scan (measured, probe5.mjs)

Emitter set: complete (measured).

  • grep -rnE "merge\.decided|conflict_fix\.finished" src finds exactly one merge.decided emitter: merge-flow.ts:613, in mergeOrWait.
  • It finds four conflict_fix.finished emitters:
    • fix-runs.ts:467 (giveUpConflict);
    • :518 and :614 (conflictRun);
    • recovery.ts:46.
  • No emit or append call passes a non-literal type that resolves to either event. The generic emit: (type, payload) => this.emit(type, payload) pass-throughs in coordinator.ts and the TaskGraph never receive these literals.
  • emittersOutside passes on the real tree.

recovery.ts's exception: sound for recovery.ts, but the rule can be fooled.

  • For recovery.ts itself it holds. Its call has a literal reasons: [] and outcome: 'interrupted'. mergeDecisionFor reads only fixes with outcome === 'pushed' (merge-flow.ts:541), so this row never reaches merge.decided either way.
  • The rule as written is looser than that. emittersOutside({ 'src/x.ts': "this.emit('conflict_fix.finished', { reasons: [], ...finished });" }) returns [] (N7). A spread after reasons: [] overrides it.

Probe 3's rows against the round-3 scan:

Row Result
A1, A2 (helper, direct or through why) caught
B1-B4 (lookup; { hold: HOLDS.stale }, { hold: staleHold(…) }) caught
C1, C2 (whyStale, gate.holdText) caught
D1-D3 (unshift, reassignment with spread, splice) caught
E1 real mergeDecisionFor returns reasons: [...d.reasons, ...fix.reasons, 'stale: the fix moved'] missed
E2 real mergeDecisionFor: d.reasons.push('stale: the base moved') missed
E3 real mergeDecisionFor: return { ...d, auto: false, reasons: ['stale: …'] } missed
F1, F2 (instructionEvalsExtra, array-literal initializer) caught
G (push after conflictWaitReasons in the real conflictRun) caught
H (a new emitter file) caught
I1 (#820's ...(i.weakening ?? [])) caught

Why E is missed.

  • region() takes the first src.indexOf(sig). In merge-flow.ts the first mergeDecisionFor( is the call at line 163, const d = this.mergeDecisionFor(e.issue, e.number, pr);, inside waitsForOwner's try {.
  • So the region scanned is lines 162-165, the 4-line try block. The definition at line 495 is never read (probe5 prints REGION … mergeDecisionFor( starts at line 162, 4 lines).
  • The test's fixture puts the definition first, so its E cases pass on fixture text and fail on the real file.
  • TRACED's ['d.reasons', 'mergeDecisionFor('] passes "its region is scanned" by name only.
  • Every other signature starts with export function, private async or protected async, and lands on its definition (measured line numbers).

New forms the scan misses (measured):

  • N1: d.reasons[0] = 'stale: x'.
  • N2: d.reasons.push.apply(d.reasons, ['stale: x']).
  • N3: const hold = HOLDS.stale; return { hold } in combinedGate (shorthand hold, and a hold initializer).
  • N4: d.reasons ??= [...].
  • N8: an emitter by a constant type, const T = 'merge.decided'; this.emit(T, …).
  • N9: an emitter through another method name, this.emitDecided('merge.decided', …).
  • N10: an emitter through an optional call, this.emit?.('merge.decided', …).

As you allowed, these may be named rather than chased: each costs only the plain words ("Another reason (see the Board)"), and nothing leaks (measured in round 2).

Check 3: the hold sentences (read against src/merge-policy.ts:210-269 and merge-flow.ts:591)

Check 4: no regression (measured)

Named tests at the head ran 196 of 196: first-task-progress, viewer-events, web-render, feature-map, merge-policy and brand. Among them:

  • Viewer: #741 GET /api/first-run … a viewer is refused, and viewer-events #741 a held first task (403, and no reason on state or events).
  • Start card: both web-render Start-card tests.
  • Old-head wait: #741 a decision or a failed merge from an older head never stands.
  • PR link: #741 the PR link is GitHub's page ….

Since 0f5ccf2, FirstRun.tsx and dashboard.ts are unchanged. The card changes only to hide "filed" when filedAt is null.

Check 5: the usual checks

Required changes

  1. Make the mergeDecisionFor region read the definition, and make a wrong region impossible (principle 4).
    • region() must find its signature exactly once in the file, e.g. assert.equal(src.split(sig).length, 2, …), and the mergeDecisionFor signature must be unique, e.g. ' mergeDecisionFor(n: number'.
    • Add a fixture where a call to the function comes before its definition.
    • Prove E1-E3 on the real merge-flow.ts text changed in memory, as probe5 does.
    • Correct the body's "each missed row in your probe-3 table is now reported" (principle 5).
  2. The fold must never throw on a forged row, and an invalid PR number means no PR stage (principles 1 and 2).
    • Count check outcomes without String() (e.g. k.includes(c.outcome as string)).
    • Keep only string reasons (.filter((r): r is string => typeof r === 'string')) instead of .map(String).
    • Treat a pr.opened whose number is not a positive safe integer as not opened (e.g. of('pr.opened', (p) => pos(p.number) !== null)).
    • Add these to forged rows: a check outcome { toString: 'pass' }, a reason { toString: 'x' }, and a PR number of 2**53. Assert no throw and no "PR #null".
    • These should cost only a few bytes (inferred), which fits the 313 left. If they don't fit, raising the budget is the owner's call (rule 10): don't loosen it.
  3. Tighten recovery.ts's exception (principle 9: an exception must be exactly as wide as its reason). Allow it only when the call has a literal reasons: [] and no top-level spread (N7). Add N7 as a fixture.
  4. Name the remaining scan gaps in the test's Known gaps comment and the body: N1-N4 and N8-N10 above. Or close any that are cheap.

Browser check still owed (human)

Use the verify-dashboard skill against a stand-in dashboard: scratch state, no real events DB, local mode. Take before and after screenshots of First run, with zero new console errors.

  • Start card:
    • With every step before Start done and Your first task not, there is no orange "required step isn't done" line.
    • With Claude login not done, it reads "1 required step isn't done yet".
  • First task card, just filed:
    • The header reads "First task #n filed … ago", with no title, and the card sits under Start.
    • All six stages are "waiting".
    • Claimed reads "Estimate: within about 65 s …".
    • Worker, Evaluator and PR opened read "Guess: 15-60 minutes …".
    • Checks reads "Estimate: no estimate …".
  • Held:
    • Merged is "held" with an orange dot.
    • Each hold shows its sentence and "What lets it through: …".
    • In local mode the kill switch reads "Local mode: every PR waits for you."
    • An unknown reason reads "Another reason (see the Board)."
    • No policy.yaml and no raw reason appears.
  • A new head after a held decision: Merged is "waiting" with "a new head: it waits for its checks and a fresh decision", and there are no holds.
  • Blocked: "Blocked: it waits for you on the Board, which says why.", with no run text.
  • PR link: "PR #n" links to https://github.com/<repo>/pull/<n> and opens in a new tab.
  • Round 3:
    • With a filing whose time doesn't parse, the header shows no "filed …".
    • A failed run with an unknown reason reads "its run ended: unknown".
  • Layout: the hold sentences wrap at phone width (375 px) without horizontal scroll, in light and dark themes.
  • Viewer: a viewer login gets the first-run refusal and no card.

… reads mergeDecisionFor's definition

Review round 3 on #816 (issuecomment-6274088682):

- The card never takes the first-run page down: the route builds it through firstTaskCard, which returns null
  (the card left out) when the fold throws, and logs that once; Start, the steps and the rest still answer.
- The fold itself no longer throws on a forged row: no String() on a value from an event (a check outcome is
  compared as is, a non-string hold reason reads as an unknown one), a null payload reads as empty, and a
  pr.opened whose number is not a positive safe integer is not an opened PR (never a done stage "PR #null").
- The scan: region() requires its signature exactly once, and mergeDecisionFor's is now unique
  ("  mergeDecisionFor(n: number"); before, the first match was a call at line 163, so the definition was never
  scanned. A fixture puts a call before the definition, and the review's E1-E3 are proved on merge-flow.ts's real
  text changed in memory.
- recovery's exception is exactly `reasons: []` among the call's top-level fields with no spread (the review's
  N7 is a fixture).
- The remaining scan gaps are named in the test: index writes, push.apply, a shorthand { hold }, ??=, and an
  emitter by a constant type, another method name or emit?.(.

Test (fails without): test/first-task-progress.test.ts, "forged rows of every JS type" and "a card that cannot
be built drops the card only"; the scan's E1-E3 and call-before-definition cases.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

Copy link
Copy Markdown
Owner Author

Round 4: what changed (head f93efce)

  1. The card can't take the page down (measured). The route builds the card through firstTaskCard. If the fold throws, it returns null (the card is left out) and logs once.
    • Route test: a fault injected into the fold only. GET /api/first-run answers 200 with firstTask: null, and Start and the steps still come back. On 27902ca the same test fails with "500 !== 200".
    • The fold no longer throws on forged rows:
      • no String() is applied to an event value;
      • a non-string reason reads "Another reason";
      • a null payload is read as empty;
      • an invalid PR number (2**53, -41, {}) means the PR was not opened. It is never a done stage reading "PR #null".
    • New test: every one of 17 fields, forged with each of { toString() { throw } }, { toString: 'pass' }, a symbol, a bigint, an array, null, NaN, undefined, {}, words, 2**53, -41 and a bad ts.
      • The fold never throws.
      • No forged word, "PR #null", "PR #-" or "PR #9007" appears.
      • A missing PR never shows a done PR stage.
      • On 27902ca this test fails at "5.number = {}".
  2. region() requires its signature exactly once, and mergeDecisionFor's is now mergeDecisionFor(n: number.
    • A fixture puts the call before the definition.
    • E1-E3 are proved on merge-flow.ts's real text, changed in memory.
    • The "each missed row is now reported" claim is corrected (false through 27902ca).
  3. Recovery's exception requires exactly reasons: [] with no spread. N7 is a fixture.
  4. N1-N4 and N8-N10 are named as known gaps in the test comment and the body.

Fail-without on 27902ca (measured): 174 tests, 172 pass, 2 fail.

After (measured): 174 of 174, also on the tree merged with origin/main d4b4503.

Package: over budget, and I've stopped. Measured with npm pack --dry-run:

Tree Tarball Unpacked
Main d4b4503 alone 1,902,158 5,934,411
Merged with this PR 1,906,348 5,950,518

Main alone is already over the 1,900,000 / 5,930,000 budget, after #805 and #787. This PR adds about 4.2 KB to the tarball, more than any compaction can recover. The budget is unchanged: whether to raise it, or have the card wait, is the owner's call.

… task card (#741)

The owner's standing practice (2026-10-19): the new size plus 2%, rounded up to 1,000. Measured with npm pack
--dry-run --json on #816 (f93efce) merged with origin/main d4b4503, the dashboard UI built: tarball 1,906,348,
unpacked 5,950,518 (main alone: 1,902,158 / 5,934,411, already over the old budget). 1,906,348 x 1.02 =
1,944,475 -> 1,945,000; 5,950,518 x 1.02 = 6,069,529 -> 6,070,000.

What grew, file by file against main (only intended code; no sourcemap, test, fixture or stray file; the other
web assets only changed their hash names, at the same sizes): dist/src/first-task-progress.js +8,846 (new),
dist/src/hold-words.js +3,758 (new), dist/web FirstRun chunk +3,024, dist/src/dashboard.js +406, the CSS +73
(the card's classes). Total +16,107 unpacked, +4,190 tarball.

Test (fails without): test/package-files.test.ts, "the package stays within its size budget", failed in CI on
f93efce (run 39302117550) with those numbers.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (round 4)

Reviewed at head 0c1a7ff (round-4 fixes f93efce, then the package-budget raise). Judged by the DECISION GUIDE.

All four round-3 changes are in, and the budget raise checks out file by file. Two cheap items remain:

  1. The tightened recovery exception can still be bypassed by wrapping the payload in a call.
  2. The PR body still holds stale package claims, and its Adversarial pass has no round-4 entry.

Neither needs an owner decision: principles 4, 5 and 9 settle both.

How I ran things: fixture text and in-memory rows only, with no repo file edited. Every run used a scratch HOME, TMPDIR and state dirs, and a stand-in claude first on PATH. The probes are in the session scratchpad under work/eval-816-r4/ (probe7.mjs, probe8.mjs, packdiff.mjs). Round 3's probe5.mjs and probe6.mjs are under work/eval-816-r3/.

Round-3 required changes

# Required in round 3 Result
1 The scan must read mergeDecisionFor's definition, with a unique signature Met (measured). probe5 shows the region now starts at merge-flow.ts line 495 and runs 50 lines; in round 3 it was 4 lines at line 162. E1, E2 and E3 are CAUGHT on the real file, each reported as "stale: …" under mergeDecisionFor(n: number. The remaining probe-3 rows (A1-A2, B1-B4, C1-C2, D1-D3, F1-F2, G, H, I1) are still caught. probe8 shows a signature found twice (a comment plus the definition) and a missing signature both throw "is there exactly once".
2 The fold must never throw, and an invalid PR number must never read "PR #null" Met (measured). Details below the table.
3 Tighten recovery's exception (N7) Met for N7, but a wider bypass remains (measured). See required change 1.
4 Name N1-N4 and N8-N10 Met (measured). The test comment (test/first-task-progress.test.ts:361-365) and the body name them all. probe5 still finds N1-N4 and N8-N10 missed, as named. N5 and N6 are caught.

Item 2 in detail:

  • probe6: every one of its rows is "clean", with no THROW. PR number 2**53 and -41 now give pr: null and "pr:waiting: no PR yet". checks: [{ outcome: { toString: 'pass' } }, { outcome: 'pass' }] reads "1 passed".

  • probe7 (a), on the real route: forged rows went straight into a scratch events DB as JSON, skipping append's schema check, as a process that can write the DB could. Among them:

    • pr.opened with numbers 2**53, -41 and {};
    • a check outcome { "toString": "pass" };
    • reasons [{ "toString": "zork" }];
    • a null payload;
    • a { "toString": … } attempt, run reason and block why.

    GET /api/first-run answered 200, with Start present and the card built. pr was null, the PR stage was "waiting", and the card held no "zork" and no "PR #null", "PR #-" or "PR #9007".

  • probe7 (b), a throw injected at a different point than the builder's test: Number.isSafeInteger throws for one marker issue number, so the fold throws inside firstTaskFiled, its first line. The result:

    • GET /api/first-run answered 200 both times, with firstTask: null, and start, setup and policy all present;
    • the "First task card is left out" line was logged once;
    • no other errors.

The N7 fixture and the exception (measured, probe8)

These forms are CAUGHT:

  • N7 { reasons: [], ...finished };
  • { ...finished, reasons: [] };
  • reasons: [] as string[];
  • a nested { meta: { reasons: [] }, reasons: why };
  • a duplicate { reasons: [], reasons: why };
  • merge.decided with reasons: [].

Recovery's real call at recovery.ts:46 is exempt, as intended.

These forms are EXEMPT, but they shouldn't be:

  • N11a: this.emit('conflict_fix.finished', Object.assign({ reasons: [] }, finished));
  • N11b: this.emit('conflict_fix.finished', withReasons({ reasons: [] }));
  • N11c: this.emit('conflict_fix.finished', pick({ reasons: [] }, finished), extra).

Cause: emittersOutside reads the fields of the first { anywhere in the call (call.slice(call.indexOf('{'))), not the payload argument itself. So the body's "exactly reasons: [] among the call's top-level fields" is false for a wrapped payload.

Cost: only the plain words. A pushed fix's reasons with an unworded prefix read "Another reason (see the Board)", and nothing leaks (inferred from mergeDecisionFor and holdsInWords). But this is an exception to a guard, so principle 9 applies, and it is cheap to make exact.

The package budget (measured, packdiff.mjs)

Method:

  • I packed main d4b4503, the PR's base 178c44e and the head 0c1a7ff, each in its own worktree with npm run build && npm run build:web and npm pack --dry-run --json through npmw.sh.
  • I compared the file lists in memory, with hashed web-asset names matched without their hash.
  • Per rule 12 I built no merged tree. I attributed the difference instead, and took the merged number from CI.

This PR alone (base to head): unpacked +16,107, tarball +4,433, and the file count goes from 512 to 514:

File Unpacked
dist/src/first-task-progress.js (new) +8,846
dist/src/hold-words.js (new) +3,758
dist/web/assets/FirstRun-*.js +3,024
dist/src/dashboard.js +406
dist/web/assets/index-*.css +73

No other file changed size. No new .map, .d.ts, test, fixture, .tgz or stray path. This matches the builder's breakdown exactly.

Merged numbers:

  • Main d4b4503 alone packs to 1,902,158 / 5,934,411, the builder's numbers.
  • Main plus this PR's growth gives unpacked 5,934,411 + 16,107 = 5,950,518. That is exactly the merged number CI printed on f93efce: run 39302117550, "the package's unpacked size is 5950518 bytes", and "the package's tarball is 1906348 bytes". Tarball sizes don't add up exactly under compression; my additive estimate is 1,906,591.

Arithmetic:

  • Tarball: 1,906,348 × 1.02 = 1,944,474.96, rounded up to 1,945,000.
  • Unpacked: 5,950,518 × 1.02 = 6,069,528.36, rounded up to 6,070,000.

Both match the PR.

Rule 10: across the PR, test/package-files.test.ts changes by +1/-1, the PACKAGE_BUDGET line only. The under-budget self-check (overBudget(p, { size: p.size - 1, … })) is untouched.

For #823, not a change here: the comment above PACKAGE_BUDGET (package-files.test.ts:108) still says "each rounded up to the next 10,000".

Merge order:

Usual checks

Required changes

  1. Make recovery's exception exact (principles 4 and 9).
    • Exempt a conflict_fix.finished call only when the argument after the event type is itself the object literal and is the call's last argument. Then read that literal's top-level fields, never the first { anywhere in the call.
    • Add N11a (Object.assign({ reasons: [] }, finished)) and N11b (withReasons({ reasons: [] })) as fixtures that must be reported.
    • recovery.ts:46 stays exempt.
    • Correct the body's "exactly reasons: [] among the call's top-level fields" if the wording changes.
  2. Correct the body (principle 5) and add round 4 to the Adversarial pass.
    • Stale and now false:

      • "Attacks I tried": "Package: fits, 494 bytes under in tarball against main 128453a. No budget change."
      • "Package, round 4": "per the owner's pending decision on the budget I stopped. The budget is unchanged."
      • "For the owner": "Package budget (a decision for you) … Either the budget is raised, or the card waits."

      Mark each as superseded by 0c1a7ff, under the standing practice.

    • Add a "Round 4" entry under Adversarial pass:

      • the 17-field × 12-value forge;
      • the injected fold fault;
      • the call-before-definition fixture;
      • N7;
      • and what each found.

Browser check still owed (human)

Use the verify-dashboard skill against a stand-in dashboard: scratch state, no real events DB, local mode. Take before and after screenshots of First run, with zero new console errors.

  • Start card:
    • With every step before Start done and Your first task not, there is no orange "required step isn't done" line.
    • With Claude login not done, it reads "1 required step isn't done yet".
  • First task card, just filed:
    • The header reads "First task #n filed … ago", with no title, and the card sits under Start.
    • All six stages are "waiting".
    • Claimed reads "Estimate: within about 65 s …".
    • Worker, Evaluator and PR opened read "Guess: 15-60 minutes …".
    • Checks reads "Estimate: no estimate …".
  • Held:
    • Merged is "held" with an orange dot.
    • Each hold shows its sentence and "What lets it through: …".
    • In local mode the kill switch reads "Local mode: every PR waits for you."
    • An unknown reason reads "Another reason (see the Board)."
    • No policy.yaml and no raw reason appears.
  • A new head after a held decision: Merged is "waiting" with "a new head: it waits for its checks and a fresh decision", and there are no holds.
  • Blocked: "Blocked: it waits for you on the Board, which says why.", with no run text.
  • PR link: "PR #n" links to https://github.com/<repo>/pull/<n> and opens in a new tab.
  • Round 3:
    • With a filing whose time doesn't parse, the header shows no "filed …".
    • A failed run with an unknown reason reads "its run ended: unknown".
  • Round 4:
    • A pr.opened row with number -41, inserted raw in the stand-in DB, shows PR opened as "waiting" ("no PR yet"), never "PR #null".
    • A card that can't be built leaves the card out, while Start, the steps and the policy form still render, with no error banner.
  • Layout: the hold sentences wrap at phone width (375 px) without horizontal scroll, in light and dark themes.
  • Viewer: a viewer login gets the first-run refusal and no card.

… the call's last argument

Review round 4 on #816 (issuecomment-6275370342): the exception for a conflict_fix.finished with no reasons read
the first "{" anywhere in the call, so a wrapped payload (Object.assign({ reasons: [] }, finished),
withReasons({ reasons: [] })) or one followed by another argument was exempt. It is now exempt only when the
argument right after the event type is itself the object literal and the call's last argument, with exactly
`reasons: []` among its top-level fields and no spread. recovery.ts's call stays exempt.

Test (fails without): test/first-task-progress.test.ts, the N11a-c fixtures in "the scan fails closed" (exempt
under the round-3 rule, as the review measured).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…he test's comment says

The budget practice as corrected (2026-10-19): the PR merged with the current main, plus 2%, rounded up to the
next 10,000 (the comment above PACKAGE_BUDGET, as #823 does). Main is still d4b4503; merged with it this PR
packs to 1,906,348 / 5,950,518 (CI run 39302117550, measured again for 0c1a7ff by the round-4 review):
x 1.02 = 1,944,475 -> 1,950,000; 6,069,529 -> 6,070,000 (unchanged).

Test (fails without): test/package-files.test.ts, "the package stays within its size budget" (it passes with
either value; this follows the stated rounding).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

Copy link
Copy Markdown
Owner Author

Round 5: what changed (head e0f3f2c)

  1. 7b4f5e6: recovery's exception is exact (measured, first-task-progress 25 of 25 with package-files). A conflict_fix.finished call is exempt only when the argument right after the event type is itself an object literal and is the call's last argument. That literal must hold exactly reasons: [] among its top-level fields, with no spread. These are now fixtures that must be reported:

    • N11a: Object.assign({ reasons: [] }, finished);
    • N11b: withReasons({ reasons: [] });
    • a payload followed by extra;
    • pick({ reasons: [] }, finished), extra.

    recovery.ts:46 stays exempt, and so does a trailing comma after the one payload.

  2. e0f3f2c: the budget follows the corrected practice.

  3. The PR body:

    • The three package claims are marked as superseded by 0c1a7ff and e0f3f2c.
    • The recovery wording is corrected, with "false through 0c1a7ff".
    • The Adversarial pass has a round-4 entry: the 17 × 12 forge, the injected fold fault, the call-before-definition fixture with E1-E3, N7, and N11a-c, each with what it found.

…y (fail closed)

Round-5 probes on #816 (e0f3f2c): a payload literal { reasons: [], <second key> } was still exempt when its
second key sets reasons again at runtime: ['reasons']: why, [k]: why, 'reasons': why, "reasons": why,
get reasons() { return why }, and either of those behind a comment. The exemption now requires every
top-level field to start with a plain identifier key (or be shorthand), and never __proto__; a computed or quoted
key, a getter, a method or a comment before a key loses it. recovery.ts's call stays exempt.

Test (fails without): test/first-task-progress.test.ts, the round-5 fixtures in "the scan fails closed" (exempt
under e0f3f2c's rule, as the round-5 probes measured).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

Copy link
Copy Markdown
Owner Author

Round 6 (head 53e2dc5): recovery's exemption now requires every top-level field of the payload literal to start with a plain identifier key, or be shorthand, and never __proto__. These now lose the exemption, and each is a fixture that must be reported: a computed or quoted key, a getter or method named reasons, a comment before a key, and __proto__. recovery.ts:46 stays exempt. Measured: first-task-progress 18 of 18. Main is still d4b4503, so the budget stands.

… package budget

The owner's practice as changed (2026-10-19, late): a PR raises PACKAGE_BUDGET only when it would exceed it.
#823 set main's budget to 1,950,000 / 6,060,000. Merged with d5674d0, this PR packs to 1,906,348 /
5,950,518 (npm pack --dry-run --json, the dashboard UI built), which fits, so the raise is dropped and
test/package-files.test.ts is main's exactly.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

Copy link
Copy Markdown
Owner Author

Merged origin/main d5674d0 (#823) as 640b6cd. Measured with npm pack --dry-run --json inside the worktree, UI built: the merged tree packs to 1,906,348 tarball / 5,950,518 unpacked. That fits main's budget of 1,950,000 / 6,060,000, so this PR's raise is dropped and test/package-files.test.ts is main's exactly (measured: the diff against origin/main is empty). This PR's own delta is +4,190 tarball / +16,107 unpacked; the body's "Package size" section breaks it down file by file. After the merge (measured): first-task-progress, viewer-events, web-render, feature-map and package-files ran 181 of 181, and typecheck is clean.

@jayden1711

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (round 6, PR body only)

Round 6, a fresh final check on head 640b6cd (53e2dc5 plus a merge of main d5674d0). I'm an independent reviewer, not the builder. I did not merge, commit, push, rebase, cherry-pick or reset anything.

The code passes every check below. The only required changes are to the PR body. They come from the owner rule of 2026-10-19: every new protection must be by design, a lint that reads source is only a backstop, and the body must name the by-design protection.

By-design protection: what it is, and what rests on the scan alone

Guarantee By-design protection Named in the body?
The card never takes the first-run page down A runtime behaviour: firstTaskCard's try returns null and logs once (src/first-task-progress.ts). Measured: the test a card that cannot be built drops the card only, and 4,000 fuzzed forged logs gave 0 throws. Yes: "The card never takes the page down. The route builds it through firstTaskCard" (round 4). It isn't called the by-design protection, though.
No free text or forged words on the card, whatever any writer emits A runtime check of each row. The fold renders only fixed words, schema-enum members, positive safe integers, normalized times and a validated link. holdWords maps any reason it doesn't know to the fixed "Another reason (see the Board)". Measured: the tests forged rows and no evaluator text, and the fuzz found 0 marker leaks. Partly: in "Compromised component" and in "What this can never do". The body never says the source scan is only a backstop to this.
A viewer sees nothing new A permission: /api/first-run is writers only, and gives a viewer a 403 (measured: viewer-events #741). Yes.
Every kind merge.decided carries gets its own plain words (acceptance line 4) None. This rests on the source-scan test alone. If it misses something, nothing leaks, but the hold reads "Another reason (see the Board)" (inferred from holdWords). Partly: specific scan gaps are listed. The body doesn't say that the guarantee as a whole is lint-only. The acceptance table cites the "source scan" as its proof.

Earlier findings: each one is fixed

Round Finding Now
3 Region scan (the call to mergeDecisionFor was read instead of its definition) Fixed (measured). region() requires the signature exactly once. The repo-wide scan test and the E1-E3 in-memory test pass on 640b6cd.
3 A forged row took the page down Fixed (measured). The a card that cannot be built drops the card only test passes. I also fuzzed the fold with 4,000 random logs of 1-25 forged rows each. The rows covered 16 event types and 18 fields, with JSON-typed values that included null payloads, primitive payloads and bad ts. Result: 0 throws, and 0 forged marker words in the card's JSON (fuzz.mjs, in memory).
3 Recovery spread loophole (N7) Fixed (measured). { reasons: [], ...finished } is reported.
4 Recovery exemption applies to the payload argument only (N11a-c) Fixed (measured). Object.assign(...), withReasons(...), a literal followed by another argument, and pick(...), extra are all reported.
4 Package claims Fixed (measured). See "Package size": the builder's numbers match mine to the byte.
5 Computed and quoted keys fail open in the exemption Fixed (measured). See the probe below.

Probe (measured; probe10.mjs lifts the scanner verbatim out of the compiled test; fixture text only; no repo file edited):

  • Still exempt: src/coordinator/recovery.ts:46 (the real call), and reasons : [] written with spaces.
  • Reported: every round-5 shape: ['reasons'], [k], 'reasons', "reasons", a getter, the method reasons(), /* c */ and // c before a key, and __proto__.
  • Also reported: these new shapes I tried:
    • a key spelled reasons, with its a written as a unicode escape;
    • a numeric key;
    • an async method, a generator method or a setter named reasons;
    • shorthand reasons;
    • shorthand __proto__ and quoted '__proto__';
    • reasons: [ ], with a space inside;
    • a trailing comment that hides ...finished;
    • a line comment holding a quote, followed by a spread;
    • satisfies T, as const, <T>{...}, and a comment before );
    • a template literal value that contains braces, followed by a spread.

Fail-without for round 6 (measured)

The scanner lives in the test file, so checking out 53e2dc5's test onto e0f3f2c brings the fix with it: 18/18 pass, which shows nothing. So I ran it the other way:

  1. Scratch worktree at e0f3f2c, with e0f3f2c's own test (git checkout e0f3f2c2 -- test/first-task-progress.test.ts), then npm run build.
  2. The same probe against that compiled scanner.

Result: 8 of the 9 round-5 fixtures are EXEMPT: ['reasons'], [k], 'reasons', "reasons", the getter, the two comment-led keys, and __proto__. Only reasons() {} was already reported. Unicode-escaped, numeric, async, generator and setter keys were also exempt.

At 640b6cd, all of them are reported. The fixtures prove the fix.

Named gaps (not blockers)

None of these lets a writer bypass the card, and none can take the page down (inferred from src/first-task-progress.ts: holdsInWords maps any unknown prefix to "Another reason (see the Board)", the fold never renders reason text, and the fuzz above found no leak). Each only loses plain words. Each was measured with probe10.mjs:

  1. Emitter shapes the regex doesn't see. These are new variants of the class the test already names ("by another method name or by emit?.("):

    • this.emit ('…' with a space;
    • emit\n(;
    • emit<'merge.decided'>(, with an explicit type argument (emit is declared generic in 7 places in src/);
    • emit(/* c */ '…'.

    No formatter or linter is configured that would normalize these (inferred: no prettier, eslint or biome config, and no lint script).

  2. Scan-side writes it doesn't see (siblings of the named push.apply): d.reasons.push?.(…), push (…) with a space, and Array.prototype.push.call(d.reasons, …).

  3. The tokenizer doesn't skip regex literals. With two regex literals that each hold one quote, { reasons: [], x: /'/, ...finished, y: /'/ } reads as exempt, and so does the same shape around ['reasons']: why. This needs deliberately crafted code.

Optional follow-up, if wanted: widen the emitter regex to \b(?:emit|append)\s*(?:<[^>]*>)?\s*\(\s*(?:\/\*[^]*?\*\/\s*)? and \.push\s*(?:\?\.)?\(. This is cheap and fails closed. It isn't required for this PR.

Package size (measured)

npm pack --dry-run --json inside each worktree, with npm run build and npm run build:web run in both. Per-file diff by packdiff.mjs.

Tree Tarball Unpacked Files
main d5674d0 1,902,158 5,934,411 512
head 640b6cd 1,906,348 5,950,518 514
Delta +4,190 +16,107 +2
File Unpacked bytes
dist/src/first-task-progress.js (new) +8,846
dist/src/hold-words.js (new) +3,758
dist/web/assets/FirstRun-*.js +3,024 (92,928 → 95,952)
dist/src/dashboard.js +406
dist/web/assets/index-*.css +73
  • Contents: every other web asset changed only its hash. The only paths new to the package are the two modules. No sourcemap, .d.ts, test, fixture or stray file was added. The one test-like path, examples/basic/test/price.test.js, is in main's package too.
  • Budget: under main's budget of 1,950,000 / 6,060,000, by 43,652 / 109,482. Every number matches the body.
  • Package files: git diff d5674d04 640b6cdc -- test/package-files.test.ts package.json package-lock.json is empty. origin/main is still d5674d0.

Tests, typecheck and denylist on 640b6cd (measured)

Every run used a scratch HOME, TMPDIR and state dirs, and a stand-in claude.

  • Named tests: first-task-progress, viewer-events, web-render, feature-map and package-files. node --import ./dist/test/isolate.js --test …: 181/181 pass.
  • npm run typecheck: clean, with no errors printed.
  • npm run check:denylist: "denylist: 1312 files clean".
  • fullsuite.sh: 419 files, 3,837 tests: 3,770 pass, 10 fail, 57 skipped.

CI, commits and body

  • CI (measured): run 39315919292 is on head 640b6cd and concluded success. All test shards on Node 22 and 24 pass, as do dco, denylist, push-gate, scope, new-deps, desktop and wizard end to end. Only the optional real-systemd jobs were skipped.
  • Commits (measured):
    • All 11 non-merge commits and the merge 640b6cd carry Signed-off-by.
    • No commit message has a Co-Authored-By trailer or a "Generated with" line.
    • The body has no attribution line.
  • Protection (measured):
    • The title carries [PROTECTED: owner merges], **Risk class:** protected is set, and the needs-owner label is on.
    • The body has real What this can never do and Compromised component sections.
    • Closes #741 is present.
  • Body claims: they are labelled throughout. Two nits, neither a required change:
    • "1298 files clean" is from an older head; it's now 1312 (measured).
    • The "Adversarial pass" tables stop at round 4. Round 6's fixtures are described under "What changed", not as their own adversarial row.

Merge order with #820 (measured)

Owed

  • A browser check of the card, with the verify-dashboard skill against a stand-in dashboard (inferred: no screenshot is in the PR). Neither the builder nor I ran a browser.

Required changes (PR body only; no code change is required)

  1. Name the by-design protections, and call the scan a backstop. Add a short section, for example "By-design protection", that names:

    • firstTaskCard's try (the page can never go down);
    • the fold's runtime check of each field, and holdWords's fixed table with its "Another reason" fallback (no reason text can reach the card);
    • the route's writers-only check.

    Say that the source-scan test and the recovery exemption are cheap backstops next to these, not protections themselves.

  2. Name the lint-only guarantee as a gap, or make it by design.

    Making it by design now would touch merge-policy.ts, merge-flow.ts and other files that open PRs hold, so naming it as a gap is the expected route (inferred).

@jayden1711
jayden1711 merged commit f091cbf into main Oct 20, 2026
28 checks passed
@jayden1711
jayden1711 deleted the issue-741-first-task-card branch October 20, 2026 01:02
jayden1711 added a commit that referenced this pull request Oct 20, 2026
…-783-weakening-hold

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>

# Conflicts:
#	CHANGELOG.md
jayden1711 added a commit that referenced this pull request Oct 20, 2026
Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>

#816 landed first: its scan of merge.decided's reason sources needs the entry for reasons.push(...(i.weakening ?? [])) in mergeDecision. Test data only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

First run: a card follows the first task from "File one" to its PR, and the Start card stops counting steps that come after Start

1 participant