Repository navigation
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. |
1a9a29c to
a162b69
Compare
442ac61 to
353e334
Compare
a162b69 to
c92c697
Compare
353e334 to
1b87318
Compare
1b87318 to
cb520c2
Compare
9829199 to
f163fa6
Compare
cb520c2 to
0392737
Compare
johncwaters
left a comment
johncwaters
left a comment
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
| expect(s.session.aiSdkStampReported).toBe(true); | ||
| }); | ||
|
|
||
| it('keeps a throwing observer and a late event as diagnostics, not failures', async () => { |
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
The title says a late event is kept as a diagnostic, but this test can't see that. runProgram copies progress.diagnostics() before it returns (run-program.ts line 139), and late?.(...) fires after that, so the late record goes into a list nothing reads. A regression that stopped recording late events would still pass.
Drop "a late event" from the title, or expose the late diagnostic somewhere the test can read it.
| }, | ||
| ); | ||
|
|
||
| it('a program that needs no AI runs without an approval', async () => { |
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
Nothing replaces this for the rule that now decides approval: needsAiApproval skips it for ci and signup sessions (run-program.ts lines 524-532). Every ci: true case in the new file logs in with an org that already approved AI, and every login(null) case is non-CI, so dropping !session.ci or !session.signup would fail no test.
Add an it.each over { ci: true } and { signup: true } stores with credentials: login(null) and no workflow, asserting RunOutcome.Success and one runAgent call.
| // `reportWarehouseSourcesDetected` alone cannot fix, since it only knows | ||
| // about consent, not login state). | ||
| it('stamps once authenticate() completes, not when consent resolves first', async () => { | ||
| it('stamps once the login completes, not when consent resolves first', async () => { |
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
This claims runProgram stamps after login, but it calls the local stampAfterLogin copy, which reads session.apiUser while runProgram passes credentials.apiUser. A regression in runProgram's ordering, arguments or once-guard can't fail it.
Narrow the title to what stampAiSdkDetected gates on its own, or move the ordering claim into run-program.test.ts.
| ); | ||
| }); | ||
|
|
||
| it('fits every area in the checks viewer column', () => { |
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
This removes the check that every seeded area fits COL_AREA_WIDTH, and nothing in this PR replaces it. The constant still exists in the TUI layout. Does a TUI-layer PR later in the stack add the check back? If not, it could move into a TUI test.
There was a problem hiding this comment.
Covered: src/tui/programs/audit/__tests__/checks-viewer-layout.test.ts checks every AUDIT_SEED_CHECKS area fits COL_AREA_WIDTH (lands in #1389).
| expect(result.posthog.host.region).toBe('eu'); | ||
| }); | ||
|
|
||
| it('reports the CI login line, as main printed it', async () => { |
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
This pins the exact prose Using provided API key (CI mode - OAuth bypassed). AGENTS.md says "Do not add tests for prose", but the PR description says locking main's CI login line is deliberate. Is the copy pin worth keeping? The region and retry tests in this file keep the behavioral coverage either way.
| expect(shouldDisableAsk(session)).toBe(true); | ||
| }); | ||
|
|
||
| it('re-enables the bridge when the harness asks for it', () => { |
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
The src/shared/__tests__/ask-policy.test.ts truth table already has the { ci: true, e2eAsk: true } row. This case adds only that buildSession passes e2eAsk through. Is that pass-through worth its own test?
There was a problem hiding this comment.
Keeping this one. The shared table only tests shouldDisableAsk; this is the only case that fails if buildSession stops passing e2eAsk through.
| @@ -0,0 +1,46 @@ | |||
| /** | |||
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
This adds a 5-line header here and a 3-line narration at lines 33-35. AGENTS.md asks to keep new comments to one line, and the describe/it names already carry this intent.
| @@ -1,49 +1,27 @@ | |||
| import { Harness } from '@shared/constants'; | |||
| /** | |||
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
This header points at other test files, and nothing keeps those pointers true. Per the one-line comment rule in AGENTS.md, trim it to one line or drop it.
| @@ -1,52 +1,28 @@ | |||
| /** | |||
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
Same as the other headers: this one lists what other files lock in, and those claims will drift. Trim it to one line or drop it.
0392737 to
be1ef32
Compare
f163fa6 to
2e19791
Compare
be1ef32 to
b91e463
Compare
2e19791 to
0de82f1
Compare
Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
0de82f1 to
7bebc68
Compare
b91e463 to
8d1b5ea
Compare
edwinyjlim
left a comment
edwinyjlim
left a comment
There was a problem hiding this comment.
Review of #1388 at 8d1b5eab. 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.
| expect.anything(), | ||
| ); | ||
| /** The org stamp, read from the session after login. */ | ||
| function stampAfterLogin(session: WizardSession): void { |
There was a problem hiding this comment.
[P3] stampAfterLogin re-implements runProgram's stamp composition, so the consent-gated cases no longer test runProgram
Partly fixed since the last push. The test titles no longer claim to test runProgram, but runProgram still has no negative consent case.
The helper builds the evidence object exactly as run-program.ts:266-273 does. The undecided, declined, no-org and no-evidence cases exercise the test's copy; the run-program test covers only the granted case. A drift in the argument list passes both files.
Suggested fix: Add one negative case to run-program.test.ts (declined consent, no group identify), or export an evidence builder from ai-sdk-stamp.ts and call it from both.
| }); | ||
| import { Integration, REPLAY_VISION_SUPPORTED } from '@shared/constants'; | ||
|
|
||
| describe('replay-vision platform support', () => { |
There was a problem hiding this comment.
[P3] replay-vision's onReady and ciPreRun have no test left
The file now only pins a constant that moved to shared. The program's whole detection is exercised nowhere.
Suggested fix: Mirror self-driving/__tests__/detect.test.ts:135: call onReady on a temp dir with a supported dependency and assert the skill id and context; one ciPreRun case for an unsupported platform rejecting with ProgramAbort.
|
|
||
| describe('error-tracking program', () => { | ||
| test('runs the error-tracking agent flow', () => { | ||
| expect(errorTrackingConfig.agentFlow).toBe('error-tracking'); |
There was a problem hiding this comment.
[P3] agentFlow pins dropped for error-tracking and replay-vision
The flow name selects the context-mill orchestrator flow, so a typo runs the wrong flow with no failing test.
Suggested fix: Pin both, or add one table in the registry test beside the bindings.
| expect(seen).toEqual(visible); | ||
| }); | ||
|
|
||
| it('sends each agent step to onEvent and the UI only the progress it saw before runAgent', async () => { |
There was a problem hiding this comment.
[P3] A test name mentions a UI that no longer exists
Suggested fix: Rename to "forwards only the scan's own progress to onProgress, and each agent step to onEvent".
Phase 2 of Release C (#1320), PR 12 of 16, after #1387. Tests only. 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
runProgram, the session store and the program folders change shape in #1376 to #1380, and the old tests drove them throughProgramStore,getUI()andsteps.Changes
Tests only: 48 files in
src/programs.run-program.test.tsis rewritten forrunProgramon aSessionStore: detection, the run definition, the preflight (outage, settings conflicts), the order of each step, composed runs through the workflow, the audit ledger cleanup and diagnostics.login.test.ts(logIn),api-key-login.test.ts,session/__tests__/interaction.test.ts(storeInteractiondismisses an open question on abort),detection/__tests__/detected-framework.test.ts,warehouse-decline.test.tsandwarehouse-source/__tests__/reporting.test.ts.--ciciPreRungets the login it asked for, so the monorepo scan sees it; a stop decided during detection ends the run as aborted; a failed login is tried again, not cached; the CI login line; a composed run ends completed; the framework variant label on CI and TUI runs.metrics-program.test.tstometrics/__tests__/metrics.test.ts,test/runner-context.tstoshared/__tests__/runner-context.no-jest.ts, the OAuth scope checks from the TUI's OAuth tests intooauth/__tests__/tokens.test.ts,session-properties.test.tsandsession/__tests__/ask-policy.test.tsfrom the shared tests.agentic-progress,agentic-retry,integration) passonProgressinstead of a UI, and program tests read each folder'sconfig.Test plan
pnpm typecheck0 errors,pnpm lint0 errors, 3,515 tests pass in 236 files,pnpm buildok.Created with PostHog Desktop