fix(harness): remove !busy guard so arrow-key history works during inference - #707
Conversation
…ference Plan #704 (source #701): the !busy guard on the composer arrow-key history event scan (ui.zig:647) blocked ↑/↓ navigation while the agent was busy. This was a reviewer recommendation (#686 R1) that the implementer accepted, but historyApply is a pure in-memory read (no bridge write, no alloc, no I/O) — it's safe during inference. - Remove !busy from the guard; keep queue_editing_index guard - Update doc comment to explain why history works during inference - Update docs/harness-limits.md keyboard table: arrows work while Busy
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #707
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/composer-arrow-busy @ cb342cd · 2 files · remove !busy from composer ↑/↓ history scan
Lenses run: L1, L3, L5, L6, L8, L9 (skip L2: no secrets / API / workflows. skip L4: existing build-harness path; zig → harness.wasm SUCCESS 2m5s on this head; Vercel ignored-build skip is expected for a Wasm-only diff. skip L7: no BYO / config seams)
AGENTS.md read: yes (docs/feature-divide.md also read; plan #704 / source #701 used as contract)
CI: zig → harness.wasm pass · no TS/vitest surface
Findings
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Nit | L8 | docs/harness-limits.md keyboard row now says arrows work while Busy because “the queue band is visible and the spinner is active.” Queue band visibility is queuedCount() > 0 (queue_band.desiredHeight), not Busy. The #701 repro is the common case: one send, empty FIFO, spinner only. |
Operator (or a later edit) reads the row as “history-while-Busy requires the queue band,” or copies the parenthetical into a guard. Empty-queue Busy — the actual bug — looks undocumented. | “Descriptive scene, not a predicate; the sentence still says arrows navigate.” The em-dash states concurrent facts. Line 69 of the same file already tells the truth (band sits above the composer when the queue is non-empty). This cell is the one the PR changed. | high |
Residual risk
Composer event scan (ui.zig ~616–664) still intercepts ↑/↓ from the global dvui.events() list with no focused-widget check. After send, want_composer_focus is true so the typical Busy path is composer-focused and empty — ↑ loading the just-sent user row is the product. The same steal already exists while idle: empty composer + ↑ wins over the status-bar model menu (scan runs before the picker is painted). ↓ is only stolen while history_index != null, so picker ↓ still works when not in history. Queue-row editor remains guarded (queue_editing_index == null). Fingerprint hydrate drop (~539) runs unconditionally, before the scan, so a same-frame promote (tryPromoteQueued → new user row) still drops stale ordinals. No new host-testable seam: the deleted conjunct lived only in frame().
Merge guidance
PASS WITH NOTES: safe to merge from this attack. Nit is optional (drop the queue-band clause; spinner is the Busy tell). Do not treat this comment as an approve.
What was not attacked
Live canvas key-repeat while the 10 Hz busy tick is refresh()-ing; dvui menu focus vs global event scan at runtime; DO runner beyond the green build-harness job; prod Gateway / agent stream. Zig was not in this sandbox PATH — relied on CI zig → harness.wasm plus the existing composer_history unit suite (untouched, 30 cases on the data machine).
Attack log (dropped)
- Re-raising #686 R1 (“Busy must pass arrows through”) — plan #704 explicitly restores #667’s original intent;
historyApplyis a ring walk +prompt_bufcopy;in_hist or buf_emptystill blocks overwrite of a typed follow-up;submitOrEnqueue/resetHistorystill clear history on enqueue/send. - “No new test of the guard” as L6 Major — plan locked
frame()as having no host-testable path; extractingqueue_editing_index == nullwould test a tautology. DI/cost gate N/A (no test file in the diff). - 10 Hz busy tick causing extra history steps —
historyApplyonly runs on.down/.repeatkey events; extrarefresh()frames without a new key event do not walk. - Fingerprint false-drop mid-stream — newest user row is the turn prompt; thinking/assistant/tool_run do not change
USER_KIND; Load earlier is alreadyif (!busy).
…cell — queue band is only visible when queuedCount > 0, not always during Busy (addresses PR #707 Nit L8)
Summary
Fixes #704 (plan)
Refs #701 (source issue)
Remove the
!busyguard on the composer arrow-key history event scan so ↑/↓ navigation works while the agent is busy (thinking/inference running).Why
historyApplyis a pure in-memory read — zero bridge writes, zero alloc, zero I/O. It just scans the message buffer for user rows and copies text intoprompt_buf. The!busyguard was a reviewer recommendation (#686 R1) that blocked the original plan #667 intent (Busy should NOT block history).Changes (2 files, +6/−5)
native/harness/src/ui.zig!busyfrom guard at L647; update doc commentdocs/harness-limits.mdBehavior
Gates
zig fmt --checkzig build harness -Doptimize=Debugzig build test-rich-invariantszig build test-rich--listen=-sandbox failures)build-harnessCINo TS changes — no vitest/typecheck/di-gate/build needed.
Status
Stop — do not merge. Ready for adversarial review after
build-harnessCI green.