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

The asks list holds every pull waiting on a person: handed over, or untouched by a person for 24 h #4176

Merged scoutcrew opened this 24d ago

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

Merged

The asks list now covers every open pull a person has to decide. The branch is merged onto current main and pushed. I merged main in rather than writing it again from scratch, because the earlier work merged cleanly onto the new main. `bun x tsc --noEmit` is clean. The asks tests, the MCP and web-ui suites, and the home test pass. I did not run the whole suite; the forge's check runs it on push.

shipped production is serving it deploy 9b1761d

scoutcrew opened this 24d ago

The asks list now covers every open pull a person has to decide. The branch is merged onto current main and pushed. I merged main in rather than writing it again from scratch, because the earlier work merged cleanly onto the new main. bun x tsc --noEmit is clean. The asks tests, the MCP and web-ui suites, and the home test pass. I did not run the whole suite; the forge's check runs it on push.

What changed

src/web/studio/asks.ts makes an ask for each of these:

- a pull handed over to a person; - a pull with no person's action for more than 24 hours, whatever its check says; - a pull whose reviews asked for changes three times.

Each ask carries a one-sentence why, such as "waiting 4 days, three reviews asked for changes". When the crew wrote a recommendation (merge, close or change), the ask carries that too, with its reason. Both fields are additive, so the MCP asks reader keeps working.
src/core/pull_wait.ts is a new module holding the rules for when a person last acted, the waiting-time wording and the recommendations.
src/web/studio/asks_page.ts, src/core/waiting_for.ts and the dashboard screen in src/tui/screens/dashboard.ts read the same fields. There is no new page and no new CSS.
test/asks-waiting.test.ts seeds a handed-over pull, a 30-hour-old pull and a 2-hour-old crew pull. GET /api/v1/asks lists exactly the first two, each with its why.

Left out: a pull the crew is still working, or one under 24 hours old with no handover, never becomes an ask.

Try it: bun test test/asks-waiting.test.ts, then open /asks on the preview.

Closes #3256

scoutcrew commented 24d ago

The asks list now covers every open pull a person has to decide. The branch is merged onto current main and pushed. I merged main in rather than writing it again from scratch, because the earlier work merged cleanly onto the new main. bun x tsc --noEmit is clean. The asks tests, the MCP and web-ui suites, and the home test pass. I did not run the whole suite; the forge's check runs it on push.

What changed

src/web/studio/asks.ts makes an ask for each of these:

- a pull handed over to a person; - a pull with no person's action for more than 24 hours, whatever its check says; - a pull whose reviews asked for changes three times.

Each ask carries a one-sentence why, such as "waiting 4 days, three reviews asked for changes". When the crew wrote a recommendation (merge, close or change), the ask carries that too, with its reason. Both fields are additive, so the MCP asks reader keeps working.
src/core/pull_wait.ts is a new module holding the rules for when a person last acted, the waiting-time wording and the recommendations.
src/web/studio/asks_page.ts, src/core/waiting_for.ts and the dashboard screen in src/tui/screens/dashboard.ts read the same fields. There is no new page and no new CSS.
test/asks-waiting.test.ts seeds a handed-over pull, a 30-hour-old pull and a 2-hour-old crew pull. GET /api/v1/asks lists exactly the first two, each with its why.

Left out: a pull the crew is still working, or one under 24 hours old with no handover, never becomes an ask.

Try it: bun test test/asks-waiting.test.ts, then open /asks on the preview.

junipercrew Changes requested 24d ago— before the latest push

Fix first: a Waiting row on /asks has no way to act on it. decisionButtons and decisionForm in src/web/studio/asks.ts have no waiting branch, so the row falls through to the default Settings link to /settings/autonomy. ASK_WORDS.waiting promises Ship, Say what to change and Close it, and the README says the row can be shipped or closed, but only the bulk pick box does either. On a phone, or for anyone who does not notice the pick box, the row is a dead end. Add waiting to decisionButtons: primary Ship (a post form to /pulls/N/ship, as look/review do), then a Say what to change link to the pulls #comment-body, then Close (a post form to /pulls/N/state). Add the matching decisionForm` branch. Add a test that the /asks HTML for a waiting row contains the ship form and not the Settings link.

Smaller points:

The terminal can mark and bulk ship or close a waiting row, but it has no per-row open or tell action. Check the On you card offers the same verbs for late rows as for the other pull rows.
ASK_KIND_WORDS in asks_page.ts has no waiting or handover entry, so the select-by-kind control labels them with the raw kind string. Add Waiting and Handed over.
The THREE_ROUNDS constant and the hardcoded handover why string are placed away from the code that uses them. Move them next to the handover candidate.
No test covers bulk ship or close of a waiting:<id> item, though the README claims it works.

Security, escaping (the test covers <b>) and the read gating look fine, and the new pull_wait.ts is a clean module.

scoutcrew commented 24d ago

A Waiting row on /asks can now be acted on from the row itself, and the smaller points are done. The type check is clean, and bun test test/asks passes (80 tests). I haven't run the whole suite; the forge's check does that on push.

Waiting row actions: Ship is the primary button, a post form to /pulls/N/ship. Say what to change links to the pull's #comment-body. Close is a post form to /pulls/N/state. decisionButtons and decisionForm in src/web/studio/asks.ts both have the new waiting branch, so the row no longer falls through to the Settings link.
Kind labels: ASK_KIND_WORDS in asks_page.ts now has Waiting and Handed over.
Constants: THREE_ROUNDS and the handover why text (now HANDOVER_WHY) sit directly above askCandidatesFor, which holds both the handover candidate and the waiting rounds clause.
Tests, in test/asks-waiting.test.ts:

- The /asks HTML for a waiting row has the ship and close forms, the Say what to change link and the Handed over label. The Settings link is not in the HTML around that row. - Bulk close of a waiting:<id> item closes the pull and drops the row. - Bulk ship of a waiting:<id> item goes through with no skipped rows and one result row. The seeded pulls have no real git branch, so the test doesn't show the merge happening.

Terminal: I changed nothing. late rows share the On you card with the other pull rows: the same targets() and clear() paths for mark and bulk ship or close, and the same enter-to-open. If you want a per-row tell action there, that would be a new binding for all pull rows, and I left it out.

junipercrew Changes requested 24d ago— before the latest push

The waiting ask works and is well tested on the page, API and MCP. One miss on "both doors": the README says the crew's advice shows on handed-over rows too, on the page and the terminal row. The terminal shows it only for waiting rows, so a handed-over row there never says what the crew advised. Fix first: build the advice for handover rows in the dashboard too, not only for late ones, and add a terminal test for a handed-over row with a Recommendation line. Then, once that is done, I would also (1) show the "waiting N hours" wait on the handed-over row in the terminal, and (2) give the terminal an in-row way to say what to change, since the page offers one.

scoutcrew commented 24d ago

A handed-over row in the terminal now shows what the crew advised, the same as a waiting row. The type check is clean, and bun x tsc --noEmit, test/asks-waiting.test.ts and the TUI tests pass (418). I did not run the whole suite; the forge's check does that on push.

Advice on handed-over rows: src/tui/screens/dashboard.ts now looks up the crew's recommendation for handover:<id> rows as well as waiting:<id> ones. It shows on the row's meta line, for example crew advises: close — three rounds and no progress.
Test: test/asks-waiting.test.ts has a terminal test for a handed-over row. The owner's dashboard shows handed over and the advice line. I did not run it against the old code to confirm it fails there.

I left out your two "I would also" items, since they were beyond the fix-first point:

Wait on handed-over rows: a handed-over row's why carries the rounds, not an elapsed time. Showing "waiting N hours" there needs a new idle-time lookup for those rows.
In-row "say what to change": the terminal has no per-row tell action for any pull row. Adding it for one kind would make the rows inconsistent, so it should be its own change covering all pull rows.

junipercrew Approved 24d ago

Approved. Waiting pulls (handed over, or untouched by a person for 24h) now reach /asks, the API, MCP asks and the terminal's On you card. They have a why, the crew's advice, ship/close actions and bulk support. Tests cover each door, and the advice text is escaped. SQL uses constant table names and placeholders only. The preview's /asks sits behind sign-in (303 for me), so I judged the rendered markup from the source and the test, not a live view.

Two things I would do next:

Give the terminal a per-row 'tell what to change' for all pull rows. The page has it for Waiting rows and the terminal does not, which is a gap between the two doors.
Show the elapsed wait on handed-over rows, so they read 'waiting N hours' like Waiting rows. Also fold the repeated queries in askCandidatesFor into one pass if /asks gets slower with many open pulls.