Repository navigation
refactor: move the agent, programs, tui, hosts, tools and cli onto the session store - #1378
Conversation
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Test against a Context Mill branch:
Add Results will be posted here when complete. |
347a236 to
b564b5c
Compare
b564b5c to
de14bf2
Compare
de14bf2 to
5320df3
Compare
5320df3 to
e278aff
Compare
johncwaters
left a comment
johncwaters
left a comment
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
| ? require | ||
| : createRequire(process.argv[1] ?? `${process.cwd()}/`); | ||
| // ESM has no bare `require`: resolve from the entry script. | ||
| const resolver = createRequire(process.argv[1] ?? `${process.cwd()}/`); |
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
Can you confirm this still finds the SDK on a global install? The removed comment said only tsx dev runs lacked a bare require, so built runs used to resolve from the bundle itself. Now resolution always starts from process.argv[1], and Node does not realpath that. With npm i -g or pnpm, argv[1] is the bin symlink or shim outside the wizard's own node_modules, so resolve('@anthropic-ai/claude-agent-sdk') could fail and every anthropic-harness run would crash.
createRequire(import.meta.url) resolves from the real bundle path and works under tsx too. harness/pi/mcp.ts already uses import.meta.url.
e278aff to
58ef33e
Compare
58ef33e to
97d1225
Compare
97d1225 to
3ba2baa
Compare
edwinyjlim
left a comment
edwinyjlim
left a comment
There was a problem hiding this comment.
Review of #1378 at 3ba2baa4. Items outside this PR's diff lines:
[P2] The Pi harness's Bash gate drops the --debug allow/deny lines the PR says now flow as log events
src/agent/runner/harness/pi/security.ts:386
Before, wizardCanUseTool called the global debug() itself, so a --debug run on either harness showed "Allowing/Denying bash command". Now the lines depend on a caller-supplied onDebug. The Anthropic path passes one at agent-interface.ts:1199; the Pi path, the default harness, passes none, so Pi runs lose them from the screen. They remain in the log file.
Suggested fix: Add onDebug?: (line: string) => void to the Pi tool-gate context, pass it at line 386, and have createToolGate in pi/index.ts supply (line) => emit({ kind: 'log', level: 'info', message: line }) when the run's debug flag is set. If the shared-code lines are meant to leave --debug output for good, say so in the body.
Drafted with Claude Code from a review of the whole Release C stack, re-verified at the stack tip 2e31e438. Line numbers are at this PR's head.
| * (`javascript_node`, `python`, `ruby`) and KMP, which has no replay support | ||
| * yet. | ||
| */ | ||
| export const REPLAY_VISION_SUPPORTED: ReadonlySet<Integration> = new Set([ |
There was a problem hiding this comment.
[P3] Program knowledge lands in the shared layer: REPLAY_VISION_SUPPORTED and SETUP_REPORT_FILE
Which integrations replay-vision supports is a replay-vision decision, and the report name belongs to the integration program. They moved so two TUI decks could import them, but the TUI may import @programs. AGENTS.md's first rule is that product knowledge stays out of infrastructure code.
Suggested fix: Keep the constants in their programs and re-export them from src/programs/index.ts, as TASK_OUTCOMES_KEY is; import @programs from error-tracking/deck/tips.ts:4 and SelfDrivingHandoffScreen.tsx:15.
| // refactor. | ||
| export function configureGatewayFromCIEnvironment( | ||
| /** Use this run's pre-issued gateway token, or mint when it has none; the keyed mint cache stays. */ | ||
| export function useRunGatewayCredential( |
There was a problem hiding this comment.
[P3] useRunGatewayCredential clears the module-global CI credential for every run that has none
ciAuth is module state. Two runAgent calls alive in one process would have the later non-CI run null out the earlier CI run's credential. The wizard runs one agent at a time today, so this is a latent hazard of the new per-run contract.
Suggested fix: Return the credential into the bootstrap result and have gatewayAuth take it, or document that runAgent calls are serialised per process.
…e session store Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
3ba2baa to
92fa6f7
Compare
gewenyu99
changed the title
3 participants
Phase 2 of Release C (#1320), PR 2 of 4, after #1376. This PR folds the 13 reviewed PRs #1378 to #1390 into one, because their pieces only compile together. Each piece was approved on its own PR, which keeps its review threads.
Problem
On main, programs run through the legacy runner with a TUI wrapped around the session, and every layer imports the others. This PR moves the agent, programs, TUI, hosts, tools and CLI onto the session store from #1376, deletes the old modules, and brings the tests along.
Changes
PROGRAM_BINDINGStable, and debug lines go through a process-widedebug()sink. Sosrc/agentnames every program, and no host can choose where a run's lines go.run-agent-legacy.ts, andrunProgramkeeps its ownProgramStore.ProgramConfigalso carries TUI screens (steps,getContentBlocks,getTips), so the programs layer imports TUI types.getUI()for log lines and detected labels, and ends the process withwizardAbort. So no program runs without a UI.WizardStoreowns the whole session, screen answers included, and the TUI reaches the rest of the wizard through the process-wideWizardUI(InkUI). The session store from refactor: add the session store, run contract and run helpers #1376 needs the TUI on top of it, not around it.process.exitand read TUI answers fromstore.session, and a program's screens, deck and tips hang off itsProgramConfig. So the TUI core names every program, and a screen can end the process under its host.src/lib/runnersbuild the UI, run the program and exit, so there is no host a caller can start and wait on. Code anywhere can end the process.mcp add,mcp remove, the MCP tutorial, Slack and doctor are registered as programs, though none of them runsrunProgram.skill list,provisionandcli addlive inside their CLI commands and exit the process themselves.main.tsregisters every command and starts the test mock server, and the CLI, the e2e harness, scripts and the docs examples import deep paths into other layers. With the hosts in place, each of them can go through public entries.wizard-session.ts, the runners andwizard-abort.ts, moved whole in refactor: move every file into its layer #1375, so they are not here.RunConfig, so their tests must follow. Some tests locked only what is gone, such as thePROGRAM_BINDINGStable.runProgram, the session store and the program folders change shape in refactor: add the session store, run contract and run helpers #1376 to refactor: program folders export a config with no screens #1380, and the old tests drove them throughProgramStore,getUI()andsteps.requestExitand gets each program's screens from its TUI entry (refactor: put the tui store on top of the session store #1381 to refactor: add the tui and headless hosts and let only the cli exit #1383). Its tests must drive that, and the exit codes that wereprocess.exitcalls need a test that fails when one changes.Test plan
pnpm typecheck0 errors,pnpm lint0 errors, 236 test files (3,515 tests) pass,pnpm buildok on the top of the stack (docs: bring docs, readmes and skills in line with the layers #1392). CI Build, Lint and Unit Tests run green on this PR's own tree.Created with PostHog Desktop