refactor(agent): cut the agent's outside edges and fence them - #1297
Conversation
The agent builds WizardError for every decided failure, and it had to import the process-exit module to do so. The class now lives in src/lib/errors/wizard-error.ts; wizard-abort re-exports it so every other importer is unchanged. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
TokenUsageDelta, SpinnerHandle and AuthErrorDetail are what the agent puts on its progress events, and AgentChunk is what the streaming prompt runner yields. They move next to the code that produces them; wizard-ui.ts and the MCP prompts service re-export them so every UI importer keeps its path. The agent no longer imports from src/ui for these. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
OutroKind, OutroData, AskQuestion, AskAnswers, PendingQuestion and TaskNotice are what runAgent returns and asks with, so they move into src/lib/agent/progress.ts. Credentials moves next to the API types, AdditionalFeature next to the other program enums in constants, and the session's CloudRegion copy points at the one in @utils/types. wizard-session.ts re-exports all of them, so its 127 importers are unchanged. The agent no longer imports @lib/wizard-session for a shape; the one remaining WizardSession reference is ProgramRun's session hooks. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
…e program hooks The agent's RunConfig.run is now AgentRunDefinition: prompt, skill, tools, copy, ask policy. ProgramRun extends it in src/lib/programs/program-run.ts with the three session-taking completion hooks (postRun, buildOutroData, buildOutroNextSteps) that only the legacy adapter calls. Programs import ProgramRun from there; the agent no longer references WizardSession anywhere. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
Both read and write the session, drive the OAuth flow through the UI and take a ProgramId. Plan section 4.5: programs own authentication and refresh. File and test move unchanged; five importers repoint. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
… programs yara-hooks and the audit ledger tools imported filename constants and the AuditCheck shape from three programs. The document names now live in @lib/constants, the ledger contract (file, check shape, read-side coercion) in the new leaf @lib/audit-ledger, and each program re-exports its own, so the audit views and the ledger watcher are unchanged. Nothing agent-side imports @lib/programs at runtime any more. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
…data PackageManagerDetector, PackageManagerInfo, DetectedPackageManager and detectNodePackageManagers move into @utils/package-manager, which already owns the lockfile detection they wrap. src/lib/detection/package-manager re-exports them, so every framework config is unchanged; the agent's two harnesses and the tools server now import the detector contract without importing detection. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
createBenchmarkPipeline takes the run's emitter; the summary and JSON writer plugins receive it through MiddlewareFactoryOptions and emit one `log` event per line they used to print through getUI(). The legacy adapter's reducer maps each back to WizardUI.log.info, so --benchmark output is unchanged. Nothing under src/lib/middleware imports src/ui. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
flushScanReport takes `{ yaraReport }` and returns the report line when
it wrote one. The runner emits it as a `log` progress event from its
single flush seam; the legacy adapter's cleanup prints it through the UI
as before. yara-hooks no longer imports the UI or the session type.
Generated-By: PostHog Desktop
Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
publish_handoff emits `{ kind: 'handoff', text }` instead of calling
getUI().setHandoffText. Both facades (the MCP server and the pi tools)
receive the run's emitter, the snapshot keeps the text as
RunSnapshot.handoffText, and the legacy adapter's reducer maps the event
to WizardUI.setHandoffText so the TUI and the headless host see exactly
what they saw before.
Generated-By: PostHog Desktop
Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
ESLint no-restricted-imports on today's agent paths: no @ui, session, detection, registry, runners, commands, steps, frameworks, setup-utils or oauth imports of any kind, no wizardAbort, and @lib/programs as types only until B1 moves PROGRAM_BINDINGS. No file allowlist; the rule runs in the editor and in `pnpm lint`. A2b collapses the path list to src/agent/**. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
🧙 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. |
There was a problem hiding this comment.
This will thrash a lot, so consider this a transitional state
There was a problem hiding this comment.
It is: A2b narrows this fence to src/agent/**, and C2d deletes it for per-layer TypeScript configs, where the agent's paths map only @env, @shared and @utils:
wizard/src/agent/tsconfig.layer.json
Lines 7 to 16 in 548ef5f
| // programs declare their own doc paths, the hooks read from a generic | ||
| // registry. For now: every new program emitting a PII-shaped report has | ||
| // to be added here. Land that cleanup before adding a fourth entry. | ||
| // TODO(wizard#594): invert this dependency. The document names are shared |
gewenyu99
left a comment
gewenyu99
left a comment
There was a problem hiding this comment.
NotVincent — automated review. Not written or checked by a person. Verify before acting on any of it.
| '**/ui/**', | ||
| '@lib/wizard-session', | ||
| '**/wizard-session', | ||
| '@lib/detection/**', |
There was a problem hiding this comment.
Here's the potential issue: The import rule is meant to keep the agent separate from the UI and other layers, but directory imports can bypass it.
agent imports the UI directory itself -> rule only matches files inside that directory -> forbidden UI dependency passes lint
Suggested fix: Cover directory imports as well as files inside them, for both aliases and relative imports.
There was a problem hiding this comment.
Fixed: the fence now also matches the directories themselves and relative steps paths, so ../../ui and @lib/detection fail lint:
Lines 65 to 88 in 3c7dfb5
| ], | ||
| excludedFiles: ['**/__tests__/**'], | ||
| rules: { | ||
| '@typescript-eslint/no-restricted-imports': [ |
There was a problem hiding this comment.
Here's the potential issue: The import rule blocks static UI imports but allows the same dependency through a dynamic import.
agent loads the UI with a dynamic import -> rule doesn't inspect it -> forbidden UI dependency passes lint
Suggested fix: Apply the same dependency restrictions to static and dynamic imports.
There was a problem hiding this comment.
Left for C2d: ESLint's import rule can't see import(), and the per-layer TypeScript configs that replace this fence reject dynamic imports too; the fence comment now says only direct static imports are checked (https://github.com/PostHog/wizard/blob/3c7dfb5a/.eslintrc.cjs).
| { | ||
| // The agent surface. It takes resolved data in, reports through | ||
| // progress events and asks through an injected answerer, so nothing | ||
| // here may reach a UI, the session, detection, the CLI or a program at |
There was a problem hiding this comment.
Here's the potential issue: This comment promises the agent won't load the UI at runtime, but allowed helpers still bring it in indirectly.
agent imports the debug helper -> helper imports the UI -> agent can load the UI while lint passes
Suggested fix: Describe this as a direct import restriction until the indirect UI dependencies are removed.
There was a problem hiding this comment.
Reworded: the comment now says only direct static imports are checked:
Lines 34 to 39 in 3c7dfb5
| }), | ||
| execute(_id, args) { | ||
| const result = publishHandoff(args.content); | ||
| const result = publishHandoff(args.content, ctx.emit); |
There was a problem hiding this comment.
Here's the potential issue: A later edit could stop handoff text reaching the host while the tool still reports success, and the current tests wouldn't catch it.
handoff tool stops passing its progress callback -> report file is written but host receives no handoff text -> focused tests still pass
Suggested fix: Exercise the registered Pi and MCP handoff tools and check that their report text reaches the host.
There was a problem hiding this comment.
Added a test that calls both registered publish_handoff tools, Pi and MCP, and checks that the report reaches the host's emit:
wizard/src/lib/wizard-tools/__tests__/handoff-tools.test.ts
Lines 57 to 95 in 3c7dfb5
|
|
||
| const middleware = input.flags.benchmark | ||
| ? createBenchmarkPipeline(spinner, runOptions(input)) | ||
| ? createBenchmarkPipeline(emit, spinner, runOptions(input)) |
There was a problem hiding this comment.
Here's the potential issue: Benchmark reporting is connected through the runner, but the tests supply their own callback. They won't catch that connection being lost.
benchmark mode is on -> runner passes a callback that does nothing -> benchmark output disappears while middleware tests still pass
Suggested fix: Run a benchmark-enabled sequence in a test and check that its progress observer receives the benchmark output.
There was a problem hiding this comment.
Added a test that runs a linear sequence with --benchmark and checks that the benchmark lines reach onProgress:
wizard/src/lib/agent/__tests__/run-agent-standalone.test.ts
Lines 698 to 730 in 3c7dfb5
| } finally { | ||
| flushScanReport({ yaraReport: input.flags.yaraReport }); | ||
| const report = flushScanReport({ yaraReport: input.flags.yaraReport }); | ||
| if (report) log(report); |
There was a problem hiding this comment.
Here's the potential issue: The scan tests never produce a summary, so they won't catch it disappearing from the terminal while the report file still exists.
scan finishes or run is aborted -> generated summary is discarded -> report file remains but terminal summary disappears, and focused tests still pass
Suggested fix: Return a real summary in completion and abort tests, and check that each path sends it to terminal output.
There was a problem hiding this comment.
The tests now return a real summary and check that it reaches onProgress on completion, agent abort, crash and both host cancels, and #1303 fixes the cancel-before-start path that dropped it:
wizard/src/agent/__tests__/run-agent-standalone.test.ts
Lines 1032 to 1107 in 38f57e8
Carries main's eight fixes and A1's per-request ask signal and host-owned analytics shutdown. One conflict, in the project-scope test's imports: main dropped AGENTIC_DETECTION_TIMEOUT_MS with its detection retry, and A2a moved authenticate to programs. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
Carries the guard that keeps a successful run successful when its terminal analytics flush fails. One import conflict in the adapter test: authenticate lives in programs here. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
The agent fence matched files inside ui, detection, runners and steps but not the directories themselves, so `../../ui` or `@lib/detection` passed lint. Relative steps imports were not covered at all. A probe with eight such imports gave 1 error before and 8 after. The fence comment now says that only direct static imports are checked; shared helpers still reach the UI transitively. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
Each of these reaches the host through a callback the runner or a tool facade passes on, and the existing tests supplied their own callback. The new tests go through the real wiring: both registered publish_handoff tools (Pi and MCP), a linear run with --benchmark, and a scan summary on completion, agent abort and crash. Dropping any of those callbacks now fails a test; each was checked by removing it. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
Brings in main (#1235, 2.77.0) through A1. One conflict: the adapter's runner import keeps A2a's removal of ProgramRun and takes A1's TASK_OUTCOMES_KEY. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
Brings in main (#1319). No conflicts. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
Release A landed on main as squash commits (#1293, #1297, #1299, #1303). B1 already carries that content through the A3 branch, so the merge keeps B1's tree and adds #1334, the one change main has beyond A3, with B1 import paths. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
2 participants
Stacked on #1293 (A1′). First of the two PRs that replace the plan's A2′: this one cuts every import the agent still made outside itself and the shared set, then fences them with ESLint. The follow-up moves the files to
src/agentandsrc/sharedwith no logic in it.Intent
Cuts every import the agent still makes into the UI, the session, detection, the programs or the CLI, and makes ESLint refuse new ones. No behavior change: the same lines reach the terminal, now as progress events the legacy adapter maps back to
getUI().Next PR (A2b) moves
src/lib/agenttosrc/agentand the shared modules tosrc/shared. Pure renames, no logic, reviewable withgit diff -M.Files
Behavior changed, read these.
publishHandoff(content, emit)emits ahandoffevent, nogetUI()emitto the handoffemitto the handoffhandoffevent kindRunSnapshot.handoffTexthandofftosetHandoffTextflushScanReportreturns the report line instead of printing itcreateBenchmarkPipeline(emit, spinner, options)logevents instead ofgetUI().log.infoThreading
emitonly: agent-interface.ts, index.ts, task.ts, linear.ts, index.ts, types.ts.Tests, each failed first with
@uimocked to throw: benchmark-emit.test.ts, yara-flush-report.test.ts, handoff.test.ts, agent-progress.test.ts, progress-collector.test.ts.Plumbing. A definition moves, the old path re-exports it, typecheck proves it. New homes first, then the re-exporting owners.
Everything else in the diff is an import line following one of those moves, plus
known-violations.jsonregenerated (78 → 73).Review by commit
One edge per commit. The first seven are moves with re-exports; typecheck proves them. The last four change behavior and each has a test that failed first with
@uimocked to throw.3be61a86WizardError→errors/9ce78997AgentChunk→ agentprogress.ts,wizard-ui.tsf15dc7a4Credentials/AdditionalFeature→ sharedprogress.ts,wizard-session.tshead53bddf83ProgramRunsplitprogram-run.ts,types.ts; 14 programs swap one import54f8e4beauthenticate→ programsgit diff -Mrename85ed50e4audit-ledger.ts,constants.ts,yara-hooks.tsimportsa88c5816utils/package-manager.tstail0b98bd37emitbenchmark.ts,summary.ts,benchmark-emit.test.ts856b613dflushScanReport,runner/index.tsfinally13fa0a28handoffprogress eventhandoff.ts,progress.ts,agent-progress.ts5cffae63.eslintrc.cjsoverrideVerification
Full notes and numbers:
workbench/wizard-functional-evidence/a2a-edges-evidence.md.pnpm typecheck,pnpm lint(0 errors),pnpm build:cipass.pnpm vitest run: 191 files, 3161 tests, 5 new. No golden regenerated.wizard-session,wizard-ui,wizard-ask-bridge,handoff,program-run→ agent).--cion a fresh express-todo copy: exit 0,PostHog set up: 7/7 steps completed (1 skipped as not required), app instrumented (index.js, posthog.js, package.json, package-lock.json). Log:workbench/wizard-functional-evidence/a2a-ci-run-express-todo.log.Fence probe: six imports in a scratch file under src/lib/agent, linted once
import type { ProgramId }andregisterCleanuppassed.Headless run tail
Created with PostHog Desktop