Repository navigation
Test the run orchestration - #52
Merged
Merged
Conversation
Chunk 10, the last one. Adds src/run.test.ts, 46 tests. No source changes. Every collaborator is mocked, so these assert branch decisions and the funnel rather than copy. The telemetry-ownership matrix is the centre: stdout markers for a terminal run, MCP only for a GUI handoff with the plugin actually present, none for GUI-without-plugin, manual, mock and telemetry-off. Exactly one start per run from whoever owns it, with the wizard staying quiet when the plugin will log its own richer start. Marker forwarding covers the two things that make it safe and correct: the harness is stamped by the wizard so a forged marker cannot claim a different one, and the dedup set is per launch so phase 1 cannot suppress a step name phase 2 reuses. Phase 2 gets its own section because it runs after the snippet is already in: declining it, cancelling the picker, a crashing launch and a non-zero exit all still close the funnel as success and return 0. Three of my first-pass expectations were wrong rather than the code, and the corrections are worth knowing. --print-prompt does call telemetry.step; what makes it a dry run is that authorize is never called, so an unauthorized Telemetry drops every event. The default harness reports no install marker, so exit 0 correctly closes as partial, not success. And asserting single-send for a marker needs phase 2 declined, since a second pass reporting the same step again is the intended behavior.
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 10 of the test plan — the last one.
src/run.test.ts, 46 tests. No source changes.Every collaborator is mocked, so this asserts branch decisions and the funnel, never copy.
The telemetry-ownership matrix
The real payload of this chunk. A credential never reaches the agent's process — an install subprocess could read it — so the transport depends on the path:
startstdoutmarkersmcpnonenone--mock/ telemetry offnoneExactly one
startper run, from whoever owns it. The GUI-with-plugin case is the subtle one: the agent logs a richer start (harness + model), so the wizard staying quiet is the only thing preventing double-counting.Marker forwarding
Two properties make it safe and correct, and both are pinned:
harnessitself, so a marker claimingharness: "forged"can't rewrite attribution.exploreis sent by both phases.--print-promptis a dry runNo agent spawned, no app opened, no plugin setup (that path spawns the agent CLI), no open-agent offer in the demo guide — but both prompts still printed.
Phase 2 cannot fail the run
It runs after the snippet is already installed, so declining it, cancelling the integration picker, a crashing launch, and a non-zero exit all still close the funnel as
successand return 0.Plus the failure paths: a non-zero phase-1 exit returns the agent's code and closes as
failwithout touching plugin setup or phase 2;CancelledError→skipped/130; anything else →fail/1; and the harness stays attached to a late failure so the funnel doesn't lose which agent the run was on.Three corrections worth reading
My first pass had three wrong expectations — the code was right each time, and the corrections are the interesting part:
--print-promptdoes calltelemetry.step. What makes it a dry run is thatauthorizeis never called, so an unauthorizedTelemetrydrops every event because there's nowhere to send it. The test now asserts the actual mechanism.partial. I'd writtensuccess. codex and gemini exit 0 even when the model refuses the install, which is exactly why that distinction exists.Verification
46 tests passing (99 on this branch), no unhandled errors,
typecheckclean. Mutation-checked all three load-bearing behaviours — sharing the dedup set across launches, letting marker metadata overrideharness, and hardcodinginstallConfirmed = true— and confirmed exactly the right tests fail, then reverted.That's the plan complete
Chunks 0–10 across #42–#52. Suggested merge order: #42 first (the rest retarget to
mainautomatically), then the others in any order.Note
Low Risk
Test-only addition with mocked dependencies; no runtime or API behavior changes.
Overview
Adds
src/run.test.ts(~46 Vitest cases) that exerciserunWizardwith every collaborator mocked, so behavior is pinned at the orchestration layer rather than UI copy. No production code changes.Coverage centers on three guarantees the PR description calls out: who owns telemetry (prompt transport
stdout/mcp/none, whenauthorizeruns, and a singlestartper path—including wizard silence when a GUI agent with plugin self-reports),--print-promptas a dry run (no launch/plugin setup, no telemetry authorization, both prompts still printed), and phase 2 not downgrading a finished install (decline, cancelled integrations, enrich crash/non-zero exit still exit 0 withcompletesuccess).Additional suites lock manual vs GUI vs terminal handoffs, phase-1 failure (agent exit code, no phase 2), install outcome from agent install markers vs exit 0 alone, marker forwarding (wizard-stamped
harness, per-launch step dedup), failure exits (cancel 130 / error 1, harness on late fail,flushalways), and pre-handoff gates (browser consent,--yesvs prompt review, review-tools consent).Reviewed by Cursor Bugbot for commit ffc1443. Bugbot is set up for automated code reviews on this repo. Configure here.