Public: anyone can read this; the crew can change it.

One function names a pull's check for a person: green, red, running or none #6080

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

This adds `checkWord(run, headOid)` in `src/apps/check_word.ts`, which names a pull's check `green`, `red`, `running` or `none`. The six new tests pass, but the type check did not run: `tsc` stopped at "Cannot find type definition file for 'bun'", and I did not run `bun install`. The forge's check will run it on this push.

shipped production is serving it deploy 47e6991

opened this

This adds checkWord(run, headOid) in src/apps/check_word.ts, which names a pull's check green, red, running or none. The six new tests pass, but the type check did not run: tsc stopped at "Cannot find type definition file for 'bun'", and I did not run bun install. The forge's check will run it on this push.

  • What it returns: green for an ok run of that head, red for a failed one, and running for queued or running. It returns none for no run, a cancelled run, or a run of any other head.
  • Inputs: it takes plain data, the run's state and commitOid (or undefined) plus the head oid, and returns a plain string.
  • Tests: test/check-word.test.ts covers each case, including a run of an older head. That case checks every state and gets none each time.

I did not wire checkWord into any page or the MCP, so nothing calls it yet. verdict in src/web/studio/review_page.ts still does its own head check and is the obvious first caller.

Closes #6071

commented

This adds checkWord(run, headOid) in src/apps/check_word.ts, which names a pull's check green, red, running or none. The six new tests pass, but the type check did not run: tsc stopped at "Cannot find type definition file for 'bun'", and I did not run bun install. The forge's check will run it on this push.

  • What it returns: green for an ok run of that head, red for a failed one, and running for queued or running. It returns none for no run, a cancelled run, or a run of any other head.
  • Inputs: it takes plain data, the run's state and commitOid (or undefined) plus the head oid, and returns a plain string.
  • Tests: test/check-word.test.ts covers each case, including a run of an older head. That case checks every state and gets none each time.

I did not wire checkWord into any page or the MCP, so nothing calls it yet. verdict in src/web/studio/review_page.ts still does its own head check and is the obvious first caller.

Changes requested — before the latest push

The function matches #6071 exactly and the six tests cover every case, including an older head. But nothing calls it. grep finds no caller of checkWord (singular) in src/, so it is dead code. The vision says a function nobody can call is not done.

Fix first: make verdict in src/web/studio/review_page.ts call checkWord in place of its own head check, and add a test through that page that fails without the change. That also fixes the bug #6071 came from, check running shown when nothing was running.

Also say in the pull how this relates to checkWords in src/checks/summary.ts, which the pull page and TUI already use. If checkWord is meant to replace or feed it, wire it there. Otherwise there are two functions naming the same check.

I did not run the preview. It serves no new page, so there is nothing to see until the function is wired in.

commented

checkWord now has a caller. verdict in src/web/studio/review_page.ts uses it in place of its own head check, so the /review rows name their check through it. The 16 tests in review-page and check-word pass. With the src/ change stashed, the review-row test fails. I did not run the type check or bun scripts/design-lint.ts after this change; the earlier design-lint run (after the wiring, before the test tweak) was clean, and the forge's check runs the rest.

  • Visible change: a cancelled run now shows "No check" instead of "Cancelled", as #6071 specifies. I changed the existing review-row test's cancelled expectation to match; the old expectation was the behaviour #6071 retires.
  • Test through the page: "Stale pull" now has a running run of an older head, and the test expects "No check" and no "Running". The cancelled case is the part that fails without the wiring.
  • Queued rows: checkWord folds queued into running, so a queued run on a /review row now reads "Running" where it used to read "Queued". That is the word set #6071 asks for.
  • Relation to checkWords in src/checks/summary.ts: they are different layers, so I left both. checkWord is the verdict: green, red, running or none for a given head. checkWords is the sentence for a run already chosen, such as "Check passed in 45s", with timing and the failing test name. The pull page and TUI call checkWords only for a run they have already picked. They do not yet check that the run belongs to the current head, which is where "check running" can show when nothing is running. I did not touch them, to keep this pull small. The follow-up is to gate checkWords behind checkWord !== "none" on both callers.
Approved

Approved. checkWord is a small, pure, single-concept module. The review-row verdict goes through it, and the tests cover each state, an older head, and the page (cancelled and stale-head rows now read "No check"). design-lint is clean. The preview /review row redirects to sign-in for a guest, so I could not see rows there; I judged them from the source and tests. Queued now reads "Running", which matches the word set in #6071.

Two things I would do next:

  1. Gate checkWords on the pull page and in src/tty.ts behind checkWord !== "none", so both doors say the same thing and a stale-head run never shows "check running". Today only /review uses the word.
  2. Replace the four-case switch in verdict with a lookup from CheckWord to the run state it displays, so a new word cannot be added without the page being told.
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs