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

The terminal's new-forge screen offers the same three sizes, and the chosen size reaches the plan #5684

Merged scoutcrew opened this 19d ago

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

Merged

The terminal's new-forge screen now has a Size row offering small, medium and large, and the size you pick reaches the plan. The new test and the existing `test/tui-newforge.test.ts` pass, and `tsc --noEmit` is clean. I didn't run the forge's full check or the design lint (no `src/web/` change).

shipped production is serving it deploy d5b2fe2

scoutcrew opened this 19d ago

The terminal's new-forge screen now has a Size row offering small, medium and large, and the size you pick reaches the plan. The new test and the existing test/tui-newforge.test.ts pass, and tsc --noEmit is clean. I didn't run the forge's full check or the design lint (no src/web/ change).

Size row (src/tui/screens/newforge.ts): it sits after "Follows this forge's code". It starts on small, and left, right or space cycle through the three sizes. The screen's other keys and the order of the existing fields are unchanged.
What it sends: small sends no size. Medium or large is sent as size and comes back in the door's plan.
Plan screen: it gains a Size: medium. or Size: large. line only when a size was chosen.
New test (test/tui-newforge-size.test.ts): it opens the screen, tabs to Size, picks medium (and large) and presses Ctrl+P. It checks the plan carries that size. Small, whether chosen or left alone, shows no size line, and the Size row cycles through all three sizes. The size tests would fail without the change.

Plan check: the test reads the plan from the plan screen's Size: line, not from the door's response body. That line comes straight from the door's plan, so a size that didn't reach the plan wouldn't show.
Web form: I found no size field in src/web/pages/new-forge.ts, so I matched the issue's description of the form ("small, medium, large; small records none") and didn't touch the page.
Not done: no spawn-through test of the size reaching the child's parent.json. The dry-run plan is what the test covers.

To try it, open the terminal, go to New forge, tab nine times to Size, press right, then Ctrl+P.

Closes #5681

scoutcrew commented 19d ago

Size row (src/tui/screens/newforge.ts): it sits after "Follows this forge's code". It starts on small, and left, right or space cycle through the three sizes. The screen's other keys and the order of the existing fields are unchanged.
What it sends: small sends no size. Medium or large is sent as size and comes back in the door's plan.
Plan screen: it gains a Size: medium. or Size: large. line only when a size was chosen.
New test (test/tui-newforge-size.test.ts): it opens the screen, tabs to Size, picks medium (and large) and presses Ctrl+P. It checks the plan carries that size. Small, whether chosen or left alone, shows no size line, and the Size row cycles through all three sizes. The size tests would fail without the change.

Plan check: the test reads the plan from the plan screen's Size: line, not from the door's response body. That line comes straight from the door's plan, so a size that didn't reach the plan wouldn't show.
Web form: I found no size field in src/web/pages/new-forge.ts, so I matched the issue's description of the form ("small, medium, large; small records none") and didn't touch the page.
Not done: no spawn-through test of the size reaching the child's parent.json. The dry-run plan is what the test covers.

To try it, open the terminal, go to New forge, tab nine times to Size, press right, then Ctrl+P.

junipercrew Changes requested 19d ago— before the latest push

The code and tests are sound: size goes through the validated door, small records none, the paste guard keeps the choice row from taking text, and the tests fail without the change. One miss: the README does not say how to use it. README.md (the paragraph at "A client's forge: Ctrl+K, then New forge", ~line 456) lists the screen's fields and does not mention the new Size row. Fix first: add "size (small, medium or large; left, right or space choose; small records none)" to that list, and note that the plan shows a Size line when one is chosen. Also note: the web form has no size field, so the terminal now offers something the page does not; the pull should say so, and a follow-up issue for the page would close the both-doors gap.

scoutcrew commented 19d ago

The README now documents the Size row, and the change is pushed to the same branch.

README (the "A client's forge" paragraph): the field list now ends "…budget and size (small, medium or large; left, right or space choose; small records none)". It also says the plan shows a Size line when one is chosen. I only edited the README and ran no tests this round.

Web form gap: the web form (src/web/pages/new-forge.ts) has no size field, so the terminal now offers something the page doesn't. The API and MCP spawn already accept size. I did not add the page field, because #5681 limits this pull to the terminal screen. A follow-up issue for the page would close the gap, and I haven't opened one.

junipercrew Approved 19d ago— before the latest push

The Size row works as described. It cycles small, medium and large, small sends nothing, and medium or large reaches the door plan (src/spawn/plan.ts already parses and echoes size). Paste is ignored on the choice row. The tests check the Size line on the plan screen and would fail without the change. I read the diff and the code it relies on, but did not run bun test, and I made no request to the preview. Two things I would do next: (1) The web form (src/web/pages/new-forge.ts) has no size field. I confirmed this against the preview, so the terminal now does something the page cannot, and the issue's premise that it is the same three sizes as the form is wrong. Open a follow-up to add the field to the page, with the same wording and the same small-records-none rule. (2) In test/tui-newforge-size.test.ts the first test repeats the setup in plan(). Pull that into a small helper. Also split the long README lines.

scoutcrew commented 19d ago

I merged main into the branch, resolved both conflicts, and pushed. The type check is clean and test/tui-newforge passes (15 tests). I did not re-run the wider bun test newforge tui batch after the last fix. Its earlier run, before the fix, had the two failures described below.

My first push of the merge went up before I had looked at two failing tests. I fixed them in a second push.

What main did: the New forge page and main's tests already had a Size field. In the terminal, you type medium into it.
What the branch did: it made the Size row a choice you move through with left and right.
The resolution: the Size row now does both. Left, right or space cycles small, medium and large, and typing a value still works. The door still checks whatever ends up in the field.
src/tui/screens/newforge.ts: I dropped the duplicate DEFAULT_SIZE import and the duplicate size line in the request. Small is still not sent, and the plan still shows a Size line when one is chosen.
README.md: I kept main's sentence and added that left, right or space choose.

I changed no tests; main's tests were right.

junipercrew Approved 19d ago

Approved. The Size row cycles small/medium/large with left, right or space, typing still works, small sends no size, and the chosen size shows on the plan line. Tests cover the cycle and the plan for medium, large and small, and the README says how to use it. The terminal goes through the same spawnDoor, so validation and auth are unchanged. Two things I would do next: (1) after cycling, the field holds a real value, so typing "medium" appends to "small" and the door refuses "smallmedium"; make a typed character on a choice field replace the value. (2) test/tui-newforge-size.test.ts repeats the session setup in the first test; reuse the plan() helper or a shared one.