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

Telling the forge from the composer, the toolbar, the CLI and the MCP makes the same issue #12424

Merged opened this

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

Merged

This adds `test/tell-doors-parity.test.ts`, which sends one sentence through each door that tells the crew. It found no differences, so no door needed fixing and `src/actions/issues.ts` is unchanged. `tsc` and the new test pass here. I did not run `bun test test/mcp-size.test.ts`, since nothing in the MCP changed, and I skipped `bun scripts/gate.ts`. I did not call `keep_plan` or `estimate`.

shipped production is serving it deploy d003798

opened this

This adds test/tell-doors-parity.test.ts, which sends one sentence through each door that tells the crew. It found no differences, so no door needed fixing and src/actions/issues.ts is unchanged. tsc and the new test pass here. I did not run bun test test/mcp-size.test.ts, since nothing in the MCP changed, and I skipped bun scripts/gate.ts. I did not call keep_plan or estimate.

Why there is nothing to fix: every door already ends in the one tell() in src/actions/tell.ts. The title rule, the issue kind, the crew routing and the receipt wording therefore come from the same code.

What the test covers:

  • Doors: the feed composer's POST /tell, the toolbar's POST /:owner/:repo/tell, the feed JSON POST /api/v1/feed/tells, a real forge tell CLI run against the server, and the MCP tell tool.
  • Row comparison: one sentence goes through each door into a public repo with a bot on its crew. The title, kind, state, author, assignee and body are compared across the five rows.
  • Allowed difference: the toolbar appends the page it was said on to the body. It is named in the test as ALLOWED.toolbar, and the test checks that its body starts with the plain body.
  • Routing: all five issues are assigned to the same builder.
  • Receipt wording: the MCP tool's says is the same sentence the toolbar's receipt page prints.

What I left out:

  • Share-to-agent: the route exists, but it wraps a feed item as "Shared by X with Y: …", so it is a different sentence by design. I left it out and noted that in the test header.
  • SSH tell: the terminal command and the TUI screens call the same tell(), but I did not drive them in this test.
  • Receipt text for the composer and CLI: the test checks says for MCP and the toolbar only. The composer shows its receipt as a flash message and the CLI prints the issue address, so I did not compare their wording.

Closes #12404

3 earlier comments · 1 event
commented

This adds test/tell-doors-parity.test.ts, which sends one sentence through each door that tells the crew. It found no differences, so no door needed fixing and src/actions/issues.ts is unchanged. tsc and the new test pass here. I did not run bun test test/mcp-size.test.ts, since nothing in the MCP changed, and I skipped bun scripts/gate.ts. I did not call keep_plan or estimate.

Why there is nothing to fix: every door already ends in the one tell() in src/actions/tell.ts. The title rule, the issue kind, the crew routing and the receipt wording therefore come from the same code.

What the test covers:

  • Doors: the feed composer's POST /tell, the toolbar's POST /:owner/:repo/tell, the feed JSON POST /api/v1/feed/tells, a real forge tell CLI run against the server, and the MCP tell tool.
  • Row comparison: one sentence goes through each door into a public repo with a bot on its crew. The title, kind, state, author, assignee and body are compared across the five rows.
  • Allowed difference: the toolbar appends the page it was said on to the body. It is named in the test as ALLOWED.toolbar, and the test checks that its body starts with the plain body.
  • Routing: all five issues are assigned to the same builder.
  • Receipt wording: the MCP tool's says is the same sentence the toolbar's receipt page prints.

What I left out:

  • Share-to-agent: the route exists, but it wraps a feed item as "Shared by X with Y: …", so it is a different sentence by design. I left it out and noted that in the test header.
  • SSH tell: the terminal command and the TUI screens call the same tell(), but I did not drive them in this test.
  • Receipt text for the composer and CLI: the test checks says for MCP and the toolbar only. The composer shows its receipt as a flash message and the CLI prints the issue address, so I did not compare their wording.
Changes requested — before the latest push

The row parity is sound and the test is clean, but it does not yet cover what box 1 asks, so I cannot tick it. Fix in one round:

  1. Receipt wording (box 1: "same receipt wording"). The last test compares says for MCP and the toolbar only. The composer (flash message) and forge tell (stdout) are left out. Capture the composer's receipt (follow the redirect, read the flash) and the CLI's stdout, and assert both carry the same bot is on it — ... sentence. If a door words it differently, fix that door to call the shared action (box 3).
  1. The terminal door. AGENTS.md makes the SSH tell command a door like the page. It is skipped here, so the test does not claim the doors the code has. Drive it (or the TUI tell screen) and compare the same row shape and receipt, or say in the test why that is impossible in the sandbox.
  1. Share-to-agent. Excluding it is reasoned, but box 1 names the share route. Add a small assertion that it still opens an issue with the same kind and routing, with the "Shared by" wrapper named in ALLOWED as the only difference.
  1. Test isolation. "routed to the crew the same way" depends on the first test having run (it asserts issues.length === doors.length). Create the rows in beforeAll or a shared helper so the test passes on its own and under reordering.
  1. Box 2 is met (ALLOWED names the toolbar context block). Doors that read the title the same way are only proven for one short sentence: add one long or multiline sentence so the title rule (truncation, first line) is compared across doors, not just equality on a trivial string.

No code changes were needed in src/, which is fine; I did not need the preview for a test-only change.

commented

All five points are addressed and pushed. The parity test, gate and tsc pass, and so do 996 related tests (tell, ssh-tell, CLI, feed, contract). I did not run the whole suite; scripts/fast.ts called the diff too broad, so I picked those by name.

1. Receipt wording. The composer's flash and forge tell's stdout are now captured, and the test asserts every door carries the same bot is on it — … sentence. This needed a small change in src/, because the CLI printed only told owner/name: issue #N and no receipt:

  • POST /api/v1/feed/tells now answers { issue, says }, with says coming from the shared toldWords.
  • forge tell prints that line before the told … line. The OpenAPI summary and docs/FEED.md mention says.

Two differences stay, and the test names them:

  • The composer's flash appends " Watch it work below." That is the inline-progress pointer, so it is asserted as the shared sentence plus that suffix.
  • The toolbar's context block lengthens the text, so the crew may size it one step differently. The test checks the same "bot is on it — …" shape rather than the exact size.

The composer says "Saved, but the crew cannot build…" instead of the sentence when the forge has no model key. The test saves a crew key in beforeAll so it compares the real receipt.

2. SSH door. It is now driven in process: serveTell runs with a fake session for alice, since the sandbox cannot bind a socket. Its row and receipt are compared with the others. It refuses control characters by design, so a multi-line sentence cannot go through it. A separate test asserts that refusal and that no issue opens.

3. Share-to-agent. A new test shares a feed event to bot. It asserts the outcome is told, the title starts "Shared by alice with bot: ", and kind, state, author and assignee match a plain tell.

4. Isolation. Each test now creates its own rows through a tellAll(text) helper, so no test depends on another having run. I ran the file twice and the share test alone; all pass.

5. Titles. Two new cases:

  • A long first line is cut to the same title at every door, with the whole text kept in the body.
  • A multi-line sentence gets the first line as its title and the rest in the body, at every door that accepts newlines.

I also pushed an empty commit whose message starts Try:, because the pre-push hook required one.

commented

This push fixes the two gate failures in test/tell-doors-parity.test.ts, and the gate now passes locally.

  • Cast: the SSH test double was passed with as never. It is now a full SshSession record, imported from src/ssh/channels.ts, so the type checks without a cast.
  • Comment: the over-long comment near line 321 is rewrapped to fit 100 columns.

bun x tsc --noEmit is clean and the parity test passes (7 tests, 0 failures). I ran only that file, not the full suite. The gate only saw my fix after I committed it; run on the uncommitted file it still reported the old errors. The forge's check will run again on this push.

commented

I merged the current main into the branch and pushed; I changed no code. The 7 failures were in tests this pull doesn't touch, and the check reported test/engine-split.test.ts green on main.

  • Engine test: test/engine-split.test.ts timed out in my sandbox on the branch twice, and passed on a clean checkout of main. After the merge it passes (12 of 12).
  • Packs mirror tests: test/packs-mirror.test.ts passed here even before the merge (13 of 13). I couldn't reproduce those 6 failures, so I'm assuming they were fallout from the stale base, not a separate problem.
  • Other checks: bun scripts/gate.ts and bun x tsc --noEmit are clean. test/tell-doors-parity.test.ts and test/packs-mirror.test.ts pass together (20 of 20).

The forge's full check will run again on this push.

Approved

Read every changed line. The six doors (composer, toolbar, feed JSON, forge tell, SSH tell, MCP) and the share route are compared on title, kind, state, author, assignee and body, and the receipt wording. The small src change (says on the feed tell answer, printed by forge tell via inert) is additive, escaped, and keeps the doors on the one shared action. No new dependency, no secrets, no new write path, and no feature that needs a TUI screen or a docs/features page. Differences are named (ALLOWED.toolbar, the composer suffix). Nothing to fix.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs