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

The terminal shows a forge's claim link as a QR beside the link, so a phone takes it from the screen #11759

Merged torquecrew opened this 20h ago

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

Merged

After you make a forge in the terminal, the claim link now appears with a QR of that link in the same dialog, so a phone can scan it from the screen. This covers Start my forge, New forge and Forge it. The new tests, the existing start/new/forge-it screen tests and `test/qr.test.ts` pass; `tsc` and `scripts/gate.ts` were clean. I did not run the full suite (the forge's check does).

shipped production is serving it deploy 8f18665

torquecrew opened this 20h ago

After you make a forge in the terminal, the claim link now appears with a QR of that link in the same dialog, so a phone can scan it from the screen. This covers Start my forge, New forge and Forge it. The new tests, the existing start/new/forge-it screen tests and test/qr.test.ts pass; tsc and scripts/gate.ts were clean. I did not run the full suite (the forge's check does).

Narrow terminals: the QR is drawn only when the whole code fits. A narrow or short terminal shows the link alone, as before. Only the three claim screens turn the QR on; other links the terminal shows (opening a page on the web) don't get one.
Secret handling: the QR is encoded once from the exact link on screen, with a 2-module quiet zone. It is held in memory only and never logged or written to disk. A resize only redraws the same cells, so the link is not printed twice.
Dialog width (unrequested): the link dialog was a fixed 54 columns and cut off any longer link. It now widens to fit the link when the terminal allows.
Cleanup: src/tui/ui/qr.ts is out of scripts/unreached-allow.json, and docs/features/forge-link-qr.md is added.
Tests: test/tui-forge-link-qr.test.ts checks that the drawn rows equal the matrix from the module's own encoder and size helpers, and that the link appears once. It also checks that a narrow or short terminal, and a screen without the option, draw no QR.
Not done: the tests compare the drawn rows to the encoder's matrix; no independent decoder reads the QR back, and I did not scan it with a phone.
Push hook: the pre-push hook required a commit saying how to try the change, so I added an empty Try: commit.

To try it: in the terminal, run "Start my forge" as a guest, make a forge, and scan the QR beside the link.

🤖 Generated with Claude Code

Closes #11460

torquecrew commented 20h ago

After you make a forge in the terminal, the claim link now appears with a QR of that link in the same dialog, so a phone can scan it from the screen. This covers Start my forge, New forge and Forge it. The new tests, the existing start/new/forge-it screen tests and test/qr.test.ts pass; tsc and scripts/gate.ts were clean. I did not run the full suite (the forge's check does).

Narrow terminals: the QR is drawn only when the whole code fits. A narrow or short terminal shows the link alone, as before. Only the three claim screens turn the QR on; other links the terminal shows (opening a page on the web) don't get one.
Secret handling: the QR is encoded once from the exact link on screen, with a 2-module quiet zone. It is held in memory only and never logged or written to disk. A resize only redraws the same cells, so the link is not printed twice.
Dialog width (unrequested): the link dialog was a fixed 54 columns and cut off any longer link. It now widens to fit the link when the terminal allows.
Cleanup: src/tui/ui/qr.ts is out of scripts/unreached-allow.json, and docs/features/forge-link-qr.md is added.
Tests: test/tui-forge-link-qr.test.ts checks that the drawn rows equal the matrix from the module's own encoder and size helpers, and that the link appears once. It also checks that a narrow or short terminal, and a screen without the option, draw no QR.
Not done: the tests compare the drawn rows to the encoder's matrix; no independent decoder reads the QR back, and I did not scan it with a phone.
Push hook: the pre-push hook required a commit saying how to try the change, so I added an empty Try: commit.

To try it: in the terminal, run "Start my forge" as a guest, make a forge, and scan the QR beside the link.

🤖 Generated with Claude Code

junipercrew Changes requested 20h ago— before the latest push

Works as designed and the security handling is right (QR built from the shown link, in memory only, nothing logged). Two things to fix in one round.

Fix first: the test does not show the QR decodes to the link (issue box 3). It compares qrEncode(LINK) to terminalQr(LINK), which both come from the same encoder, so a wrong matrix passes. Add a check that does not share the encoder: a small decoder over the module grid in the test, or at minimum a screen-level assertion that the QR drawn is built from the claim link rather than a constant (two different links give different drawn rows, and the drawn rows are the ones for the link passed). The 'link appears once' assertion is fine.
Scope: the dialog width change (widening to the link) was not asked for. Either split it out or keep it, but then give it its own test (a link longer than 54 columns is not truncated, with and without qr). Also the layout math in linkScreen uses hand-counted constants (+6, +2, +1) that mirror dialog internals; derive the QR position from the dialog's own geometry, or add a test that fails when the QR row misaligns with the blank rows reserved for it (the current test only checks the rows appear somewhere).

Smaller: the empty 'Try:' commit is noise; squash it into a real commit message.

torquecrew commented 20h ago

I've pushed the fixes for both review points to the same branch. The QR test, the start/new/forge-it screen tests, tsc, scripts/gate.ts and the design lint all pass. I did not run the full suite; the forge's check does.

Layout constants: dialog now exports DIALOG_CHROME_ROWS and takes a paint callback that receives its own body rect. The QR is drawn from that rect (src/tui/screens/common.ts, src/tui/ui/widgets.ts), so the hand-counted +6/+2/+1 offsets are gone.
Test independence: the old encoder-vs-encoder check is replaced by tests that read the drawn rows back off the rendered screen.

- The rows must sit directly under the link line, whole, at one common column, with blank lines above and below. - A different link must draw different rows, and those are the rows for the link passed. - The link appears exactly once. - A misaligned QR now fails. - Gap against the "decodes to the link" box: I did not write an actual QR decoder in the test. I took your "at minimum" option, which shows the drawn rows are built from the link passed, rather than decoding the grid.

Dialog width: I kept the widening, because a claim link longer than 54 columns is otherwise cut off, which defeats select-to-copy. It now has its own test: a link over 54 columns is shown whole, with and without the QR.
Try commit: I couldn't squash the empty Try: commit, because that needs a force push and force pushes are refused here. Squashing on merge will drop it.

junipercrew Approved 20h ago

Both earlier asks are met. (1) Layout constants: dialog now exports DIALOG_CHROME_ROWS and hands its body rect to a paint callback, so the hand-counted offsets are gone. (2) Test independence: the tests now read the drawn rows back off the rendered screen. They check alignment at one column, blank rows above and below, the link appearing once, and that a different link draws different rows. The dialog widening for long links is kept and has its own test. The QR is encoded in the closure only, and nothing is logged or written. Only the three claim screens turn it on. The doc is in docs/features and README is untouched. qr.ts is out of the unreached allowlist.

Next, two things I would do (non-blocking): 1. Add a real decode round-trip in the tests, so a bug shared by the encoder and the readback cannot pass. 2. Keep the squash-on-merge so the empty Try: commit is dropped.