From 914461a42531045b3c54bebb448eee42c1a2f9d7 Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Fri, 7 Aug 2026 19:10:03 +0530 Subject: [PATCH 1/2] Hard-fail failproofai config on unsupported platforms 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 --- CHANGELOG.md | 5 ++ CLAUDE.md | 20 +++---- __tests__/hooks/configure-wizard.test.ts | 67 +++++++++++++++------- __tests__/hooks/onboarding-attempt.test.ts | 23 ++++++++ src/hooks/configure-wizard.ts | 34 +++++++++-- src/hooks/onboarding-attempt.ts | 18 ++++-- 6 files changed, 126 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc419549..a2fd92f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 1.0.0-beta.13 — 2026-08-07 + +### Fixes +- Make `failproofai config` refuse setup on an unsupported platform (Windows, today) instead of completing it unenforced. The wizard used to skip the daemon requirement and finish anyway, leaving the machine reading as configured while enforcing in-process with no fail-closed guarantee — now it prints why and exits 1 before drawing a single prompt, writing nothing. (#PR) + ## 1.0.0-beta.12 — 2026-08-07 ### Fixes diff --git a/CLAUDE.md b/CLAUDE.md index f8d98664..7c030f32 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -906,22 +906,22 @@ denies until `failproofai config` runs. `publish.yml` ships both from one commit itself. **In-process evaluation still exists**, reachable only when `daemonConfigured` is false — -which is exactly three situations, none of them a configured user machine: +which is exactly two situations, neither of them a configured user machine: | Case | Why | |------|-----| | This repo's dogfood configs | Standing decision above: a flaky dev daemon must not block contributors' tool calls in the same loop where the daemon is being developed. | -| Unsupported platforms | `isDaemonSupportedPlatform()` is linux + darwin only. | | Not yet set up | No hooks are installed either, so nothing evaluates anything. | -**Known gap — Windows.** The wizard *skips* the daemon requirement on an unsupported -platform rather than refusing setup, so a Windows user completes setup, reads as -configured, and enforces **in-process**: slower (~850ms vs ~57ms), and with no fail-closed -guarantee, because `daemonConfigured` is never set and there is nothing to fail closed -against. The policies themselves are identical and do enforce. This is a deliberate -trade — refusing setup would drop the platform entirely — and it is the one place -"all enforcement routes through the daemon" is not literally true. Revisit it if -failproofaid ever gains a Windows service target. +**Unsupported platforms refuse setup, not degrade into it.** `isDaemonSupportedPlatform()` +is linux + darwin only, and `failproofai config` checks it before drawing a single prompt: +on anything else (Windows, today) it prints why and exits 1, writing nothing — no hooks +installed, no `daemonConfigured` flag, no half-configured machine. This used to be a known +gap — the wizard *skipped* the daemon requirement instead, so a Windows user completed +setup, read as configured, and enforced in-process with no fail-closed guarantee at all. +Refusing is the more honest failure: it says plainly that the platform isn't supported yet, +rather than silently shipping a weaker guarantee than every other configured machine has. +Revisit when failproofaid gains a Windows service target. ### How the daemon is supervised diff --git a/__tests__/hooks/configure-wizard.test.ts b/__tests__/hooks/configure-wizard.test.ts index 895eaf99..db003436 100644 --- a/__tests__/hooks/configure-wizard.test.ts +++ b/__tests__/hooks/configure-wizard.test.ts @@ -231,10 +231,16 @@ beforeEach(() => { vi.mocked(installHooks).mockClear(); vi.mocked(runPostSetupAudit).mockClear(); vi.mocked(outro).mockClear(); - vi.mocked(isDaemonSupportedPlatform).mockReset().mockReturnValue(false); - vi.mocked(installDaemonService) - .mockReset() - .mockResolvedValue({ installed: false, reason: "mocked" }); + // Supported-and-already-healthy is the safe default for every test that + // isn't specifically about the daemon step: it makes step 0 a one-line + // no-op ("already installed and running — leaving it alone") without + // demanding sudo — and, now that an unsupported platform hard-fails setup + // before a single prompt is drawn, without aborting every other test in + // this file. Tests that actually exercise the daemon step override these. + vi.mocked(isDaemonSupportedPlatform).mockReset().mockReturnValue(true); + vi.mocked(daemonServiceStatus).mockReset().mockReturnValue("running"); + vi.mocked(daemonServiceNeedsUpgrade).mockReset().mockReturnValue(false); + vi.mocked(installDaemonService).mockReset().mockResolvedValue({ installed: true }); // Reset too, or call counts leak across tests and "was never asked for sudo" // silently passes on history from an earlier one. vi.mocked(primeElevation).mockReset().mockReturnValue(true); @@ -496,6 +502,20 @@ describe("first-run redirect", () => { expect(handled).toBe(false); expect(selectOne).not.toHaveBeenCalled(); }); + + it("hard-fails cleanly on an unsupported platform, and does not nag again next command", async () => { + vi.mocked(isDaemonSupportedPlatform).mockReturnValue(false); + + const first = await maybeFirstRunConfigure(ttyIO()); + expect(first).toBe(true); // took over the turn + expect(hasSeenLauncher()).toBe(false); // never completed + expect(installHooks).not.toHaveBeenCalled(); + + // The next command must not relaunch the wizard and hard-fail again. + const second = await maybeFirstRunConfigure(ttyIO()); + expect(second).toBe(false); // a one-line hint instead of a relaunch + expect(selectOne).not.toHaveBeenCalled(); + }); }); describe("assistant selection summary", () => { @@ -681,20 +701,37 @@ describe("configure-wizard daemon integration", () => { expect(hasSeenLauncher()).toBe(false); }); - it("does not require a daemon, or sudo, on an unsupported platform", async () => { - // Requiring an impossible step would lock these users out of setup - // entirely rather than protecting anything. + it("hard-fails on an unsupported platform, before drawing a single prompt", async () => { + // A Windows machine (or any non-linux/darwin platform) has nothing + // running failproofaid — completing setup anyway used to leave it + // reading as configured while enforcing in-process, with no fail-closed + // guarantee. Refusing outright is the honest failure. vi.mocked(isDaemonSupportedPlatform).mockReturnValue(false); drive(HAPPY); const result = await runConfigureWizard(ttyIO()); - expect(result.applied).toBe(true); + expect(result.applied).toBe(false); + expect(result.abort).toBe("unsupported_platform"); + expect(selectOne).not.toHaveBeenCalled(); + expect(multiSelect).not.toHaveBeenCalled(); expect(primeElevation).not.toHaveBeenCalled(); expect(installDaemonService).not.toHaveBeenCalled(); + expect(installHooks).not.toHaveBeenCalled(); expect(readGlobalConfig().daemonConfigured).toBeUndefined(); }); + it("explains why, naming the platform, when it hard-fails on an unsupported platform", async () => { + vi.mocked(isDaemonSupportedPlatform).mockReturnValue(false); + const stdout = mkTtyStdout(); + + await runConfigureWizard({ stdin: mkTtyStdin(), stdout }); + + const written = vi.mocked(stdout.write).mock.calls.map((c) => String(c[0])).join(""); + expect(written).toContain("Linux"); + expect(written).toContain("macOS"); + }); + it("skips the install, and the password prompt, when a daemon is already running", async () => { // Re-running setup on a configured machine must not demand sudo for work // that is already done. @@ -927,19 +964,12 @@ describe("configure-wizard daemon integration", () => { expect(props.reason).not.toContain("/home/"); }); - it("mentions the daemon in the outro only when one is actually there", async () => { + it("mentions the daemon in the outro when one is actually there", async () => { vi.mocked(isDaemonSupportedPlatform).mockReturnValue(true); vi.mocked(installDaemonService).mockResolvedValue({ installed: true }); drive(HAPPY); await runConfigureWizard(ttyIO()); expect(vi.mocked(outro).mock.calls[0]![0]).toContain("daemon on"); - - // Unsupported platform: no daemon, so no claim of one. - vi.mocked(outro).mockClear(); - vi.mocked(isDaemonSupportedPlatform).mockReturnValue(false); - drive(HAPPY); - await runConfigureWizard(ttyIO()); - expect(vi.mocked(outro).mock.calls[0]![0]).not.toContain("daemon on"); }); it("shows the daemon row in the review only when one will be installed", async () => { @@ -997,10 +1027,6 @@ describe("configure-wizard daemon integration", () => { }); }); describe("scope targets", () => { - beforeEach(() => { - vi.mocked(isDaemonSupportedPlatform).mockReturnValue(false); - }); - it("installs once per scope when Both is chosen", async () => { drive({ ...HAPPY, target: "both" }); @@ -1059,7 +1085,6 @@ describe("scope targets", () => { describe("connect step", () => { beforeEach(() => { - vi.mocked(isDaemonSupportedPlatform).mockReturnValue(false); vi.mocked(connectToCloud).mockClear(); vi.mocked(validateIngestKey).mockClear().mockResolvedValue({ ok: true }); // ONE prompt now, not two. The endpoint is no longer asked for: there is diff --git a/__tests__/hooks/onboarding-attempt.test.ts b/__tests__/hooks/onboarding-attempt.test.ts index 726c3cd9..755bda78 100644 --- a/__tests__/hooks/onboarding-attempt.test.ts +++ b/__tests__/hooks/onboarding-attempt.test.ts @@ -198,6 +198,28 @@ describe("cancelled — a deliberate stop", () => { }); }); +describe("unsupported_platform — a permanent property of the machine", () => { + it("stays blocked on the same CLI version", () => { + // Nagging every command would just repeat the hard-fail this reason + // records — the machine's platform has not changed. + expect( + blockerCleared( + attempt({ reason: "unsupported_platform", cliVersion: "1.0.0" }), + probe({ cliVersion: "1.0.0" }), + ), + ).toBe(false); + }); + + it("re-offers after an upgrade, in case the new version supports it", () => { + expect( + blockerCleared( + attempt({ reason: "unsupported_platform", cliVersion: "1.0.0" }), + probe({ cliVersion: "1.1.0" }), + ), + ).toBe(true); + }); +}); + describe("reasons that are properties of the invocation, not the machine", () => { it("re-offers for not_a_tty and running_as_sudo", () => { for (const reason of ["not_a_tty", "running_as_sudo"] as const) { @@ -226,6 +248,7 @@ describe("what the user is told", () => { "needs_root", "daemon_failed", "cancelled", + "unsupported_platform", "not_a_tty", "running_as_sudo", ] as const) { diff --git a/src/hooks/configure-wizard.ts b/src/hooks/configure-wizard.ts index 91e22cef..a8ebfb67 100644 --- a/src/hooks/configure-wizard.ts +++ b/src/hooks/configure-wizard.ts @@ -126,6 +126,7 @@ export type WizardAbort = | "cancelled" | "needs_root" | "daemon_failed" + | "unsupported_platform" | "not_a_tty" | "running_as_sudo"; @@ -712,6 +713,25 @@ export async function runConfigureWizard(io: WizardIO = {}): Promise { @@ -733,9 +753,9 @@ export async function runConfigureWizard(io: WizardIO = {}): Promise elevation is now possible without a prompt - * daemon_failed -> the service manager now reports something different - * cancelled -> the CLI version changed; an upgrade is a fair moment to - * ask again, and nothing else about a deliberate cancel - * should re-nag + * needs_root -> elevation is now possible without a prompt + * daemon_failed -> the service manager now reports something different + * cancelled -> the CLI version changed; an upgrade is a fair moment to + * ask again, and nothing else about a deliberate cancel + * should re-nag + * unsupported_platform -> the CLI version changed (a future release might + * support this platform) * * Every probe is local and takes milliseconds. None of them runs on the hook * path — `--hook` never reaches the first-run gate at all. @@ -150,6 +152,11 @@ export function blockerCleared(attempt: OnboardingAttempt, probe: RetryProbe): b // this module removes — but an upgrade is a new thing to say, so it is // allowed to ask once more. return probe.cliVersion !== attempt.cliVersion; + case "unsupported_platform": + // A permanent property of the machine, not the invocation — nagging every + // command would just repeat the hard-fail. Only a new release is a reason + // to ask again (it might add support for this platform). + return probe.cliVersion !== attempt.cliVersion; case "not_a_tty": case "running_as_sudo": // Both are properties of the invocation, not of the machine, so the next @@ -166,6 +173,7 @@ export function attemptHintLines(attempt: OnboardingAttempt): string[] { needs_root: "it needs root to install the failproofaid service", daemon_failed: "the failproofaid service could not be started", cancelled: "it was cancelled", + unsupported_platform: "failproofaid does not run on this platform", not_a_tty: "there was no terminal to ask in", running_as_sudo: "it was run under sudo", }; From 90f46ceb2ac227809c448d292669997f7d88bcc1 Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Fri, 7 Aug 2026 19:24:30 +0530 Subject: [PATCH 2/2] Address CodeRabbit review on PR #664 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- CHANGELOG.md | 2 +- __tests__/hooks/configure-wizard.test.ts | 37 ++++++++++++++- __tests__/hooks/onboarding-attempt.test.ts | 8 ++++ src/hooks/configure-wizard.ts | 55 ++++++++++++++-------- src/hooks/onboarding-attempt.ts | 10 +++- 5 files changed, 90 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2fd92f2..0108aee1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## 1.0.0-beta.13 — 2026-08-07 ### Fixes -- Make `failproofai config` refuse setup on an unsupported platform (Windows, today) instead of completing it unenforced. The wizard used to skip the daemon requirement and finish anyway, leaving the machine reading as configured while enforcing in-process with no fail-closed guarantee — now it prints why and exits 1 before drawing a single prompt, writing nothing. (#PR) +- Make `failproofai config` refuse setup on an unsupported platform (Windows, today) instead of completing it unenforced. The wizard used to skip the daemon requirement and finish anyway, leaving the machine reading as configured while enforcing in-process with no fail-closed guarantee — now it prints why and exits 1 before drawing a single prompt, writing nothing. (#664) ## 1.0.0-beta.12 — 2026-08-07 diff --git a/__tests__/hooks/configure-wizard.test.ts b/__tests__/hooks/configure-wizard.test.ts index db003436..cf34d52a 100644 --- a/__tests__/hooks/configure-wizard.test.ts +++ b/__tests__/hooks/configure-wizard.test.ts @@ -135,6 +135,7 @@ import { } from "../../src/hooks/daemon-service"; import { buildAgentChoices, + buildCompletionSummary, buildPresetChoices, clisSupportingScope, resolvePresetSelection, @@ -335,6 +336,40 @@ describe("configure-wizard pure builders", () => { // not read like the wizard dropped the selection. expect(lines).toContain("failproofai policies --install"); }); + + // The 3-column gutter ("└ ") that outro() prepends when rendered. + const GUTTER = 3; + + it("keeps the completion summary within 80 columns for the longest real combination", () => { + // Every builtin policy, every supported CLI, and every optional note + // present at once — the worst case now that an unsupported platform + // aborts before this line is ever reached (so "no daemon" can no longer + // shrink it). + const message = buildCompletionSummary( + 99, // headroom above today's real policy count + INTEGRATION_TYPES.length, + true, // custom enabled + true, // daemon installed + true, // connected + ); + expect(message.length + GUTTER).toBeLessThanOrEqual(80); + expect(message).toContain("custom"); + expect(message).toContain("daemon"); + expect(message).toContain("reporting"); + }); + + it("keeps the completion summary within 80 columns with custom policies explicitly off", () => { + // "off" is the longer of the two custom-policies tags, and stacks with + // the other two notes the same way "custom" does above. + const message = buildCompletionSummary(99, INTEGRATION_TYPES.length, false, true, true); + expect(message.length + GUTTER).toBeLessThanOrEqual(80); + expect(message).toContain("custom off"); + }); + + it("omits every optional note when nothing is present", () => { + const message = buildCompletionSummary(2, 1, undefined, false, false); + expect(message).toBe("Setup complete — 2 policies · 1 assistant"); + }); }); describe("configure-wizard orchestration", () => { @@ -969,7 +1004,7 @@ describe("configure-wizard daemon integration", () => { vi.mocked(installDaemonService).mockResolvedValue({ installed: true }); drive(HAPPY); await runConfigureWizard(ttyIO()); - expect(vi.mocked(outro).mock.calls[0]![0]).toContain("daemon on"); + expect(vi.mocked(outro).mock.calls[0]![0]).toContain("daemon"); }); it("shows the daemon row in the review only when one will be installed", async () => { diff --git a/__tests__/hooks/onboarding-attempt.test.ts b/__tests__/hooks/onboarding-attempt.test.ts index 755bda78..7c4da6e4 100644 --- a/__tests__/hooks/onboarding-attempt.test.ts +++ b/__tests__/hooks/onboarding-attempt.test.ts @@ -257,4 +257,12 @@ describe("what the user is told", () => { expect(text, reason).not.toContain("undefined"); } }); + + it("does not tell an unsupported-platform machine to re-run the command that just hard-failed", () => { + // blockerCleared only re-offers this reason on a CLI version change, so + // `failproofai config` would hit the exact same guard right now. + const text = attemptHintLines(attempt({ reason: "unsupported_platform" })).join("\n"); + expect(text).not.toContain("Run `failproofai config`"); + expect(text).toContain("update"); + }); }); diff --git a/src/hooks/configure-wizard.ts b/src/hooks/configure-wizard.ts index a8ebfb67..eeae2357 100644 --- a/src/hooks/configure-wizard.ts +++ b/src/hooks/configure-wizard.ts @@ -416,6 +416,37 @@ export function describeCustomPolicies(cwd: string): { return { active, warnings, fileCount, scopes }; } +/** + * The wizard's one-line completion summary. Pure and exported so the widest + * real combination — every policy, every CLI, custom/daemon/reporting all + * present — can be pinned by a test without having to drive the whole wizard + * through a real chdir + on-disk custom-policy fixture. + * + * Kept inside a standard 80-column terminal: `writeLines` truncates with a + * hard cut and no ellipsis, so an over-long line doesn't just lose its tail + * — it reads as broken output. Naming all ten CLIs once took it to 182 + * characters; the count alone carries the same information, and the user + * picked them two screens ago. A single grouped "· a, b, c" clause bounds the + * optional notes to one separator and short tags, rather than three + * independent " · " clauses stacking up. + */ +export function buildCompletionSummary( + policiesCount: number, + assistantsCount: number, + customEnabled: boolean | undefined, + daemonInstalled: boolean, + connected: boolean, +): string { + const extras: string[] = []; + if (customEnabled === true) extras.push("custom"); + else if (customEnabled === false) extras.push("custom off"); + if (daemonInstalled) extras.push("daemon"); + if (connected) extras.push("reporting"); + const extrasNote = extras.length > 0 ? ` · ${extras.join(", ")}` : ""; + const assistants = `${assistantsCount} assistant${assistantsCount === 1 ? "" : "s"}`; + return `Setup complete — ${policiesCount} policies · ${assistants}${extrasNote}`; +} + export function reviewLines(state: { /** What the scope step resolved to. Expands to one or two real scopes. */ target: SetupTarget; @@ -1431,26 +1462,12 @@ export async function runConfigureWizard(io: WizardIO = {}): Promise