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

A pull the access wall holds says so: it stops reading "approved and green" and "moving" #7899

Merged opened this in answer to

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

Merged

A pull the access wall holds now says a person must decide, in plain words, everywhere that used to say "approved and green" or `moving: true`. `tsc` is clean, and `gate.ts`, `design-lint.ts` and `mcp-size.test.ts` pass. I ran the tests that cover the change, not the full suite. `scripts/fast.ts` named 1201 files, which is too broad to run here, so the forge's check covers the rest.

shipped production is serving it deploy 2e641a3

opened this

A pull the access wall holds now says a person must decide, in plain words, everywhere that used to say "approved and green" or moving: true. tsc is clean, and gate.ts, design-lint.ts and mcp-size.test.ts pass. I ran the tests that cover the change, not the full suite. scripts/fast.ts named 1201 files, which is too broad to run here, so the forge's check covers the rest.

What changed

  • Plain words: when the land sweep stops at the wall, it now records a plain sentence alongside the raw cells. For example, "anyone signed out could read packs" (wallPlain in src/contract/policy.ts). Wall decisions recorded before this change fall back to the old cell text.
  • next on the pull: a new module, src/flow/wall_wait.ts, reads the latest wall decision for the pull's live head. nextActor returns person, with the wall sentence, ahead of any "queue" answer. A conflict, a red check or requested changes still read as the builder's, so sending the pull back hands it to the builder. A push makes a new head, so the old wall word stops applying until the sweep re-records it. The pull page, the API next, the terminal and MCP pull all answer from this.
  • MCP flow: there is a new first cause, wall, so moving is false and says and clears carry the sentence. This is a cause on the existing tool, not a new tool. mcp-size passes without raising any ceiling.
  • Stuck alarm: it now says "is approved and waits for a person: …" and carries a wall field.
  • One button: the wall card on the pull and Files tabs now has "Allow it and merge", which posts to the existing merge route, and "Send it back", which posts a changes-requested review to the existing review route. Both show only to someone who can write.
  • Where the owner looks: the audit page's "pulls wait at the wall" rows now show the plain sentence instead of "N doors open wider". The terminal dashboard shows the stuck alarm's new wording.

Tests

  • next_actor: a walled pull that is approved and green reads as person, and one with changes requested reads as builder.
  • mcp-flow-why: a walled, approved, green pull via MCP reads moving: false, cause: wall, next.who: person, and stops reading as walled after a push.
  • Alarm: the stuck alarm names the wall for the matching head only.
  • policy.test.ts: the plain sentences.
  • test/wall-card.test.ts: the wall card's buttons, shown to a writer and absent for a reader.

Left out

  • No new feed or notification surface. The pull, the audit page, the stuck alarm and MCP carry it.
  • "Allow" is the ordinary merge, so it still honours the review and check gates and flashes a refusal if one applies. I didn't add a bypass.
  • Only approved pulls trigger the alarm, as before.

To try it: open a pull that widens contract.json for the floor role, then check its pull page, GET …/pulls/N (next) and MCP flow {repo, pull} once the sweep has recorded the wall.

Closes #7847

commented

A pull the access wall holds now says a person must decide, in plain words, everywhere that used to say "approved and green" or moving: true. tsc is clean, and gate.ts, design-lint.ts and mcp-size.test.ts pass. I ran the tests that cover the change, not the full suite. scripts/fast.ts named 1201 files, which is too broad to run here, so the forge's check covers the rest.

What changed

  • Plain words: when the land sweep stops at the wall, it now records a plain sentence alongside the raw cells. For example, "anyone signed out could read packs" (wallPlain in src/contract/policy.ts). Wall decisions recorded before this change fall back to the old cell text.
  • next on the pull: a new module, src/flow/wall_wait.ts, reads the latest wall decision for the pull's live head. nextActor returns person, with the wall sentence, ahead of any "queue" answer. A conflict, a red check or requested changes still read as the builder's, so sending the pull back hands it to the builder. A push makes a new head, so the old wall word stops applying until the sweep re-records it. The pull page, the API next, the terminal and MCP pull all answer from this.
  • MCP flow: there is a new first cause, wall, so moving is false and says and clears carry the sentence. This is a cause on the existing tool, not a new tool. mcp-size passes without raising any ceiling.
  • Stuck alarm: it now says "is approved and waits for a person: …" and carries a wall field.
  • One button: the wall card on the pull and Files tabs now has "Allow it and merge", which posts to the existing merge route, and "Send it back", which posts a changes-requested review to the existing review route. Both show only to someone who can write.
  • Where the owner looks: the audit page's "pulls wait at the wall" rows now show the plain sentence instead of "N doors open wider". The terminal dashboard shows the stuck alarm's new wording.

Tests

  • next_actor: a walled pull that is approved and green reads as person, and one with changes requested reads as builder.
  • mcp-flow-why: a walled, approved, green pull via MCP reads moving: false, cause: wall, next.who: person, and stops reading as walled after a push.
  • Alarm: the stuck alarm names the wall for the matching head only.
  • policy.test.ts: the plain sentences.
  • test/wall-card.test.ts: the wall card's buttons, shown to a writer and absent for a reader.

Left out

  • No new feed or notification surface. The pull, the audit page, the stuck alarm and MCP carry it.
  • "Allow" is the ordinary merge, so it still honours the review and check gates and flashes a refusal if one applies. I didn't add a bypass.
  • Only approved pulls trigger the alarm, as before.

To try it: open a pull that widens contract.json for the floor role, then check its pull page, GET …/pulls/N (next) and MCP flow {repo, pull} once the sweep has recorded the wall.

commented

I fixed the MCP docs failure and pushed. I didn't change anything for the terminal-notes failure, because I couldn't reproduce it.

  • docs/MCP.md tool table: my change to the flow tool's doc text left the generated table out of date. I ran bun scripts/mcp-tools.ts and committed the regenerated file. test/mcp-size.test.ts and test/mcp-tools.test.ts pass (175 tests).
  • "notes from the app, in the terminal > only an app's repository has the rows": test/tui-notes.test.ts passed on four runs here, three of them back to back. The test drives the terminal palette and doesn't touch the wall, nextActor or flow, so I think it's a timing flake under the load of 54 shards. If it fails again on this push, send me the log and I'll look at it properly.
Changes requested — before the latest push

The wall now reads as 'a person decides' in next, flow, the alarm and the audit page, and the pure pieces are tested. Two things need fixing before it lands.

  1. Both doors (fix first). 'Allow it and merge' and 'Send it back' exist only on the page. src/tty.ts is untouched, so a person in the terminal is told a person must decide but cannot do it there. Add the same two actions to the terminal's pull view, going through the same merge and review paths with the same write check. Say in the README how to use them.
  2. Wiring is untested. test/wall-card.test.ts renders wallCard directly. Nothing proves the pull page and the Files tab pass the thread only to a writer, or that a reader's page has no form. Nothing covers crossingsWaiting().plain or the audit row either. Add page-level tests that fail without the change.

Smaller points are in the line comments: the two pages use different write checks, the send-back reason is hard-coded, and the plural rule is naive. I did not exercise the preview.

commented

A pull the access wall holds now reads "a person must decide" in plain words. A writer can allow it or send it back on the pull page, on the Files tab and in the terminal.

What changed since the review

  • Terminal door: on a pull the wall holds, a writer presses w in the pull screen. A dialog lists the crossings in plain words with Cancel, Send it back and Allow it and merge. Allow goes through mergePull with the squash strategy, as the page's button does. Send-back goes through submitReview as a request for changes, with no body. Both check the write. Readers get no w key and no dialog. The README says how to use it. The terminal file is src/tui/screens/pull.ts, because src/tty.ts doesn't exist here.
  • Send-back reason: the hard-coded hidden input is now a visible optional "Why (optional)" field, and an empty reason is posted as an empty body. I didn't change the review-state route. It already goes through readForm(ctx, "rw"), which the merge route also uses, and I took that to be the same-origin check without reading readForm.
  • Plural: ${resource}s became ${resource} records ("their own address records"). I updated the existing test strings to match.
  • Write predicate: I made no change here. Both pages already gate on canWrite (mayMerge in src/web/pages/pull.ts is canWrite(...)), and the merge route enforces canWrite. I think the two pages never differed, but say so if you saw something I missed.

Tests that fail without the change

  • test/wall-doors.test.ts:

- The pull page and the Files tab show the forms and the reason field to a writer. A signed-in reader and a visitor get the card with no forms. - A contract that doesn't read gets the "does not boot" wording and no "If you allow it" line. - In the terminal, a reader gets no dialog, w sends back (a changes_requested review is recorded), and allow merges the pull.

  • test/audit.test.ts covers crossingsWaiting().plain and the audit row's plain words.
  • test/policy.test.ts covers the own wording and the address plural.

Checks run: the gate and the design lint pass. tsc is clean, and the wall, audit, policy and terminal-pull tests pass (93 tests across 10 files). scripts/fast.ts named 1206 of 1726 files, which is too broad, so I picked those 10 files myself. I did not exercise the preview.

Left out: the terminal's send-back takes no reason, so use R for a worded review. The wall dialog doesn't bypass a pull's other merge gates; mergePull still refuses those in its own words.

Approved

Earlier asks are met. The terminal door is there: w on the pull screen, gated by mayWrite, going through mergePull and submitReview, with a test and a README line. The send-back reason is now a visible optional field. The plural is fixed (records). The write predicate is the same on both pages. The send-back route reads its form with readForm, which does the same-origin check, caps the body by the bytes actually read, and checks token scope. Forms are POST buttons and work with JavaScript off. Readers get no forms and no w key. Non-blocking: the terminal send-back carries no reason (R does); the README says so.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs