Repository navigation
refactor: route the cli, e2e harness and examples through public entries - #1385
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. |
f8c053f to
f0d8c36
Compare
eeede8c to
f70ef67
Compare
f70ef67 to
eaa3240
Compare
f0d8c36 to
c03101f
Compare
dfdc9c3 to
9c1d85d
Compare
24a0712 to
95d85e8
Compare
johncwaters
left a comment
johncwaters
left a comment
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
| // and it runs before any TUI takes the terminal. | ||
| .middleware((argv) => { | ||
| if (typeof argv.logFile === 'string' && argv.logFile) { | ||
| configureLogFile({ path: argv.logFile }); |
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
This path gets overwritten later without warning. With --benchmark, createBenchmarkPipeline (src/agent/middleware/benchmark.ts:77) calls configureLogFile({ path: config.output.logPath }). In --ci/headless runs, runNonInteractive calls configureLogFileFromEnvironment() (src/cli/runners/run-non-interactive.ts:59), so POSTHOG_WIZARD_LOG_DIR wins. In both cases --log-file is ignored.
Could an explicit --log-file take priority, e.g. through one resolver the benchmark and env-dir paths defer to? A test combining --log-file with --benchmark would pin it.
95d85e8 to
4be5b3f
Compare
602458a to
e88b9e5
Compare
4be5b3f to
1b7941f
Compare
Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
1b7941f to
81ab207
Compare
e88b9e5 to
6b6345e
Compare
edwinyjlim
left a comment
edwinyjlim
left a comment
There was a problem hiding this comment.
Review of #1385 at 6b6345e7. Comments inline.
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.
| '--import', | ||
| pathToFileURL(require.resolve('tsx')).href, | ||
| '--import', | ||
| pathToFileURL(path.join(__dirname, '../mocks/preload.ts')).href, |
There was a problem hiding this comment.
[P2] The MSW relocation is the first time the mocks can reach the built wizard, and nothing runs the suite
The deleted main.ts block was gated on NODE_ENV === 'test', which tsdown inlines as production, so it was dead in every built output that test:e2e spawns. The new preload will intercept for the first time. No workflow runs e2e-tests/, the test plan lists only unit tests, and the recorded fixtures were last touched on 2026-03-31.
Suggested fix: Run pnpm test:e2e once on the stack tip and put the result in the test plan. If the suite is not maintained, say so in the body. Add a one-line comment in preload.ts that it replaced the env gate.
| describe: 'Enable verbose logging\nenv: POSTHOG_WIZARD_DEBUG', | ||
| type: 'boolean' as const, | ||
| }, | ||
| 'log-file': { |
There was a problem hiding this comment.
[P3] A user-visible flag ships under a refactor: commit
release-please builds the changelog from commit types, so --log-file will not appear in the release notes.
Suggested fix: Split it into a feat(cli) commit ahead of this one, or add a feat body line.
| ScreenId.SelfDrivingHandoff, | ||
| SelfDrivingScreenId.IntegrationDetect, | ||
| SelfDrivingScreenId.IntegrationCheck, | ||
| SelfDrivingScreenId.IntegrationDetect, |
There was a problem hiding this comment.
[P3] SelfDrivingScreenId.IntegrationDetect is listed twice in NO_ACTION_SCREENS
Suggested fix: Delete line 100. Harmless in a set, but it reads as if a second screen was meant.
| ]); | ||
|
|
||
| /** Decide a program screen from the commits the state lists for it. */ | ||
| function decideProgramScreen( |
There was a problem hiding this comment.
[P3] No test names decideProgramScreen's contract
The drivable-screens list now holds only core screens, so the "no listed screen waits forever" test no longer covers intros, detection checks, handoffs or outros. A new program screen whose only action id is outside the commit set would stall a run with no unit test pointing at the cause.
Suggested fix: One table test: for every registry key not in the core set, build a state with actionsForScreen(screen) and assert an action when any id is in the commit set, otherwise wait.
3 participants
Phase 2 of Release C (#1320), PR 9 of 16, after #1384. Review PR. PRs 1 to 14 of this phase split one change into reviewable pieces and compile only as a stack: review them one by one, then land the whole phase together.
Problem
On main,
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.Changes
runCli(src/cli/index.ts) registers every command, andmain.tscalls onlyrunCli.bin.tsstill checks the Node version before it loadsmain.ts. The MSW mock server moves frommain.tstoe2e-tests/mocks/preload.ts, whichstartWizardInstancepreloads into the wizard's process through tsx.@programs(getProgramConfig, each program'sconfig) and print throughconsoleLoginstead ofsetUI(new LoggingUI()).chooseFamilyChilddraws through the TUI'srenderFamilyPicker, so the CLI holds no Ink code.src/cli/wizard.ts:--log-file(envPOSTHOG_WIZARD_LOG_FILE), which sets the file the debug log goes to.profiles.tsreads each program'stest/e2e.jsonfrom disk, by itsprogramid, so a new program needs no harness edit.e2e-profile.tsdecides a program's own screen from the actions the state lists for it.action-registry.ts,wizard-ci-driver.tsande2e-profile.tstake the store and every screen id from the@tuientry, and the run state from@shared.vitest.config.tsmaps the public entries (@tui,@cli,@headless,@tools), drops the@ui,@stepsand@libaliases, adds aheadlesstest project, and renames the catch-alllegacyproject toshared.docs/examples/run-agent-quack.ts,run-program-quack.tsand the smoke scripts import only public entries such as@programs,@agentand@env.main.ts,runCliand the command handlers: every command must parse and dispatch as on main.Test plan
pnpm typecheck0 errors,pnpm lint0 errors, 3,515 tests pass in 236 files,pnpm buildok.Created with PostHog Desktop