Repository navigation
Conversation
Stop the main integration flow with DetectNoFramework when detection finds no framework, before it can surface a misleading missing-skill error. Include Tal Gluck's replay-vision fix and actionable guidance from #1205. Thank you, Tal, for identifying and fixing the replay-vision path. Carry that work in this signed commit alongside the main integration fix and regression coverage for both flows. Co-authored-by: Tal Gluck <talagluck@gmail.com>
🧙 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. |
Route early error outros before walking the program's incomplete screens. Framework-detection aborts otherwise wait for dismissal while the intro keeps displaying its detection spinner. Exit after dismissal and preserve the existing failed-run handoff. Exercise the real outro dismissal wait for both integration and replay vision, and check early-error routing across registered programs.
| ctx.setUnsupportedVersion(versionResult.supported); | ||
| } | ||
| } else { | ||
| await wizardAbort({ |
There was a problem hiding this comment.
astra this is not dry do beter
There was a problem hiding this comment.
maybe not entirely relevant but I ban else statements entirely XD
Use one abort helper for integration and replay-vision detection in both interactive and CI paths. Keep replay-specific guidance with its program and provide the same actionable message to CI runs. Extend the no-framework regression coverage to both CI pre-run hooks.
sarahxsanders
left a comment
sarahxsanders
left a comment
There was a problem hiding this comment.
the abort looks like it would remove manual framework picking
| } else { | ||
| await abortNoFrameworkDetected(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
could we keep the manual framework picker when a project exists but detection returns no result?
this exit also blocks users who could select their supported framework and continue
There was a problem hiding this comment.
Good catch (robot slop removed)
There was a problem hiding this comment.
Fixed in be6ea8d. Interactive detection now finishes and shows the manual framework picker when it finds nothing. The no-framework guard runs at the integration program's run entry, before agent execution; CI still aborts during pre-run detection.
The regression test simulates a missed Next.js project, selects Next.js through the real terminal picker, and verifies the resulting run configuration. All 111 focused tests, typecheck, and build pass. I also checked the rebuilt CLI manually. The commit is signed and verified by GitHub.
It's done, NotVincent will tell Vincent to double check.
Separate the short error heading, recovery instruction, supporting detail, and labeled documentation link. Give the terminal exit hint extra space and name the actual exit action. Preserve recovery instructions in CI logs. Use structured outro data for the shared no-framework abort and keep replay-specific headings and links in the replay program.
Let interactive detection finish without a framework so users can select one manually. Guard the integration run entry before constructing a run when no framework configuration exists. Keep CI and replay guards. Cover detection miss, keyboard selection, and missing-framework run abort.
…ndbox Team PR reviews now run the operator's own pr-review skill, so a lane review holds the same standard as a hand-run one, and the skill's Step 4 posting JSON is kept and posted verbatim after the operator approves. The posted shape matches hand-run reviews: the automated-review note, a **[tag] SEVERITY** header and multi-paragraph bodies with a suggested fix. The code-review block supplies verdict, findings and summary and is the fallback render; verdicts use code-review's vocabulary, and drafts saved with the old verdicts still load. The session reviews untrusted PR text with permissions bypassed, so it starts in an empty work dir and reaches the checkout by absolute path only (an --add-dir loads the checkout's .claude/skills). GitHub credentials, git credential helpers, ssh and push urls and GLIMMERVOID secrets are withheld; MCP servers are off; and Claude Code's Bash sandbox pins egress to OpenAI, ChatGPT and api.github.com, blocks ssh, gh, keychain and git-credentials reads, and fails closed: a session that cannot apply its sandbox never spawns. The prompt states Codex lanes are expected, and Codex runs inside the sandbox. Constraint: api.github.com stays on the egress list because Codex 0.155 syncs curated plugins from it at startup and stalls when blocked; no GitHub credential is reachable inside Constraint: Codex reads ~/.codex/auth.json inside the sandbox only because the dotfiles Read deny on it was lifted (dotfiles fa25be3) Rejected: a separate CODEX_HOME login for the lane | one more credential to keep alive Rejected: prefix deny rules as the only barrier | bypassable via command, absolute paths, bash -c and python Directive: HIGH round-3 fix (sandbox fail-open when the hooks settings file is not written, session/session-hook-lifecycle.ts) landed after the last review; not promoted until the operator has looked at it Confidence: medium Scope-risk: broad Not-tested: round-3 fixes (Session refuses to spawn when a requested sandbox is not applied; server/AGENTS.md invariant rewording) were not re-reviewed, only unit-tested and exercised by a sandboxed live run on PostHog/wizard#1347; Linux bubblewrap path not run; suite failures limited to pre-existing trace-wiring (26) and a timing-flaky integration-ref-watch
johncwaters
left a comment
johncwaters
left a comment
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
The no-framework abort is small, guarded and consistent across the integration, CI, error-tracking and replay-vision paths. One question on exit status below; otherwise looks good.
| session.runPhase === RunPhase.Error && | ||
| session.outroData?.kind === OutroKind.Error | ||
| ) { | ||
| return session.outroDismissed ? ScreenId.Exit : ScreenId.Outro; |
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
After dismissal this routes to ScreenId.Exit, and ExitScreen calls process.exit(0) unless a mint handoff is set, while wizardAbort also resumes on the same dismissal and exits with code 1 (after emitting its error line). If the Exit screen wins, an aborted run exits 0. Ordering probably favors wizardAbort, but nothing pins it. Could a test assert the exit code on dismissal?
| @@ -78,24 +78,19 @@ export class WizardRouter { | |||
| return this.overlays[this.overlays.length - 1]; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
Nit: this branch now applies to any run in RunPhase.Error with an error outro, replacing the Auth-only special case, so later post-run screens are skipped after any error abort. Probably intended, just confirming.
3 participants
Problem
When framework detection finds nothing, the main integration flow continues into orchestrator preflight and reports that setup instructions failed to download. The scout report attributes 37 production users in seven days to this misleading error, including version 2.77.0.
Changes
DetectNoFrameworkand app-root guidance before agent execution or skill preflight. CI uses the same no-framework outcome during pre-run detection.abortNoFrameworkDetectedhelper for integration run/CI checks and replay vision's interactive/CI detection. Keep replay-specific headings and documentation in the replay program; CI receives the same actionable guidance as interactive runs.Credit
Thank you to Tal Gluck (@talagluck), who diagnosed and implemented the replay-vision fix in #1205. The replay-vision guard and actionable guidance originated with Tal. This PR carries that work forward through a shared abort helper and a shorter, spaced-out presentation. That contribution directly helped this fix.
This PR incorporates that work into a signed commit, with Tal credited as a co-author, so both affected flows can be fixed without waiting for the original commits to be signed. It supersedes #1205's code change and adds the main integration fix and regression coverage for both paths.
Test plan
pnpm typecheckpassed.pnpm buildpassed, including CLI and warlock smoke tests.git diff --checkpassed, with two existing non-null assertion warnings in the warehouse-reporting test.LLM context
Implemented and verified with Codex from the supplied scout report; replay-vision behavior and wording were carried over from Tal's PR #1205.