Hard-fail failproofai config on unsupported platforms - #664
Conversation
isDaemonSupportedPlatform() previously let setup skip the daemon requirement and complete anyway on Windows, leaving the machine reading as configured while enforcing in-process with no fail-closed guarantee. Setup now refuses outright before drawing a single prompt, writing nothing. Also wires the new abort reason into the onboarding memory so an unsupported machine gets a one-line hint instead of relaunching (and re-failing) on every command, and tightens the outro summary line so the now-mandatory daemon note still fits in 80 columns on the widest real case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe configure wizard now rejects unsupported platforms before prompts or writes. It returns ChangesUnsupported Platform Setup
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ConfigureWizard as configure-wizard
participant Onboarding as onboarding-attempt
User->>ConfigureWizard: run failproofai config
ConfigureWizard->>ConfigureWizard: check platform support
alt unsupported platform
ConfigureWizard-->>User: explain unsupported platform
ConfigureWizard->>Onboarding: record unsupported_platform
else supported platform
ConfigureWizard-->>User: continue daemon-backed setup
end
Onboarding->>Onboarding: compare recorded and current CLI version
Onboarding-->>User: suppress or re-offer onboarding
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 6: Replace the unreplaced (`#PR`) placeholder in the changelog entry with
the actual PR identifier (`#664`), or remove the parenthetical reference if this
changelog does not include PR numbers.
In `@src/hooks/configure-wizard.ts`:
- Around line 1438-1447: The completion summary assembled near customNote can
exceed 80 columns for the longest combination of assistants and enabled optional
notes. Update the summary formatting to ensure the final rendered line,
including customNote, daemon status, and reporting status, stays within 80
columns without hard truncation; add coverage for the maximum-output combination
with customEnabled, daemonInstalled, and connected enabled.
In `@src/hooks/onboarding-attempt.ts`:
- Line 176: Update the recovery instruction associated with the
`unsupported_platform` reason in the onboarding attempt handling so it does not
use the generic `failproofai config` retry guidance. Provide a version-specific
action that remains appropriate when `blockerCleared` suppresses onboarding
until `cliVersion` changes, while preserving the existing behavior for other
reasons.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b8b27d22-157b-4271-923b-993ce15b7e0b
📒 Files selected for processing (6)
CHANGELOG.mdCLAUDE.md__tests__/hooks/configure-wizard.test.ts__tests__/hooks/onboarding-attempt.test.tssrc/hooks/configure-wizard.tssrc/hooks/onboarding-attempt.ts
- CHANGELOG.md: fill in the real PR number instead of the (#PR) placeholder. - configure-wizard.ts: extract the completion summary into a pure, exported buildCompletionSummary() that bounds custom/daemon/reporting into one grouped note. The previous fix only covered the customEnabled===false case; the true worst case (custom on, daemon on, reporting on, every CLI) still overflowed 80 columns. Added direct unit tests for the worst case rather than relying on driving the full wizard through a real chdir + on-disk custom-policy fixture. - onboarding-attempt.ts: give unsupported_platform its own hint action instead of "Run `failproofai config`" — blockerCleared only re-offers that reason on a CLI version bump, so the generic retry would just hit the same hard-fail again in the meantime. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@hermes-exosphere review |
Summary
failproofai confignow refuses setup outright on non-linux/darwin platforms (Windows, today), before drawing a single prompt, instead of completing in a degraded state with no daemon and no fail-closed guarantee.unsupported_platformabort reason into the onboarding-attempt memory so an unsupported machine gets a one-line hint instead of relaunching (and re-failing) the wizard on every command; the hint's suggested action is version-specific rather than the genericfailproofai configretry, since that would just hit the same hard-fail again.buildCompletionSummary()that bounds the custom/daemon/reporting notes into a single grouped clause, so the true worst case (every policy, every CLI, all three notes present) still fits an 80-column terminal — not just the case that happened to be covered before.CLAUDE.md's daemon-fallback documentation and adds aCHANGELOG.mdentry (with the real PR number).Test plan
bun run test -- run __tests__/hooks/configure-wizard.test.ts(65/65 passing)bun run test -- run __tests__/hooks/onboarding-attempt.test.ts(22/22 passing)bun run test -- run __tests__/hooks/daemon-service.test.ts(confirmed unaffected)bun run test:run(full suite — no new failures; one pre-existing, unrelated failure suite left untouched)bunx tsc --noEmit(clean)bun run lint(clean, only pre-existing warnings)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes