Repository navigation
Test agent detection and the process helpers - #47
Merged
Merged
Conversation
Chunk 5. Adds src/agents/helpers.test.ts (50 tests) and src/agents/index.test.ts (19). No source changes. sanitizeTerminalOutput gets the most attention: agents echo arbitrary repo content into the user's terminal through the wizard, so what survives is a security boundary. Colour is kept, OSC 52 clipboard writes are stripped in both BEL and ST terminations, and a table-driven test asserts the real guarantee — no escape byte survives unless it opens a colour sequence. One thing that surprised me and is now written down: ESC ( B leaves "(B" behind as plain text. The escape byte is gone so the terminal can't act on it, which is the property that matters, but the payload characters are not removed. runTerminalAgent's line buffer covers chunks split mid-line, blank lines being delivered rather than swallowed, and the flush on 'end' for a final line with no trailing newline — that last one is where an agent's closing telemetry marker would otherwise be dropped. index.ts substitutes fakes for the four agent modules rather than probing the machine, so detection order and failure handling don't depend on what the runner has installed. Covers a rejecting probe being dropped without costing the user the other agents, and --agent throwing for an undetected id rather than silently falling back to a picker a CI run would hang on. Control characters are built with String.fromCharCode so they stay visible in the source rather than sitting in the file as invisible bytes.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Chunk 5 of the test plan. Two files —
src/agents/helpers.test.ts(50 tests) andsrc/agents/index.test.ts(19). No source changes.sanitizeTerminalOutputgets the most weightAgents echo arbitrary repo content — READMEs, file bodies, command output — straight into the user's terminal through the wizard. Escape sequences in that stream can spoof output or drive the terminal, so what survives this function is a security boundary rather than a formatting choice.
One thing worth knowing
ESC ( B(charset selection) leaves(Bbehind as plain text. The escape byte is removed, so the terminal has nothing to act on — the security property holds — but the payload characters aren't stripped. My first draft asserted'text'and failed; the test now records what actually happens. Not proposing a change, just no longer a surprise.runTerminalAgent's line bufferChunks split mid-line, blank lines delivered rather than swallowed, stdio shape per mode,
closewith a null code (signal kill) treated as failure, and a spawn error rejecting.The one to keep: the flush on
endfor a final line that never got a trailing newline. That's where an agent's closing summary — and its last telemetry marker — would otherwise be dropped, and the funnel would silently lose the final step.Detection
index.test.tssubstitutes fakes for the four agent modules instead of probing the machine. The realAGENTSlist hits PATH,~/.claude, and macOS bundles, so results would depend on whatever the runner has installed.--agent <id>throws for something undetected, naming what was found, rather than silently falling back to a picker a CI run would hang onA note on test hygiene
Two tests in my first draft were wrong in ways worth flagging, since both are easy to repeat:
macAppPathtest passed against/Applications/Cursor.appon my machine — that path is checked before the home directory, so a real install would win. Now uses a name nothing will have installed.\uXXXXescapes ended up as literal invisible bytes in the file. They're built withString.fromCharCodenow, and the file is verified to contain none.Verification
69 tests passing (122 on this branch: chunk 0 plus this one),
typecheckclean. Mutation-checked the two load-bearing behaviors — removing the end-of-stream flush and the.catch(() => null)on probes — and confirmed exactly the right tests fail, then reverted.Note
Low Risk
Test-only additions with mocked I/O; no runtime behavior changes.
Overview
Adds Vitest coverage only — no production code changes — for agent process helpers and the agent catalog/selection flow (~69 new tests across two files).
helpers.test.tsexercisessanitizeTerminalOutputas a security boundary (SGR colour kept; OSC 52 clipboard writes, cursor/screen escapes, and C0 controls stripped; invariant that no stray ESC survives except SGR). It also mockschild_process/osto testwhich, path helpers,openAppAtDir, andrunTerminalAgent(stdio modes, exit/signal handling, and stdout line buffering including flush on streamendfor a final line without a newline).index.test.tsreplaces real agent modules with fakes sodetectAgents/chooseAgentare deterministic: catalog ordering (terminal before app), resilient detection when a probe rejects,--agentstrict errors for undetected IDs, interactive picker hints, and cancel handling.Reviewed by Cursor Bugbot for commit 127f5fe. Bugbot is set up for automated code reviews on this repo. Configure here.