Release 1.0.0 — close the ways enforcement failed silently, and make setup two questions - #683
Merged
Merged
Conversation
Each of these was a success path: the policy ran, the verdict was recorded,
the dashboard counted it, and nothing happened. None of them could fail a
test or raise an error, which is why none were caught.
PostToolUse denies now enforce on codex and copilot. Both read a top-level
{decision:"block", reason} and neither reads the hookSpecificOutput shape we
sent, so every PostToolUse deny on them was dropped. Verified rather than
inferred: a live A/B probe on codex 0.147.0 (block shape -> "hook: PostToolUse
Blocked", the reason REPLACES the tool result and the model never sees the real
output; our shape -> "Completed", model reads it verbatim) and a re-read of the
shipped copilot 1.0.78 bundle, where both postToolUse call sites gate on
vK = t => t?.decision==="block" && typeof t.reason==="string". Re-probing codex
also surfaced that several of its capability rows cite source paths that no
longer exist in 0.147.0; they are marked unverified rather than left looking
like evidence.
--status stops reporting a connection the machine no longer has. Everything it
printed came from the credential file, which records --connect time and is
never revisited, so a revoked or expired key left it correct while nothing
arrived. The uploader already writes its verdict into the parked batch's
filename and already declines to retry a definitive refusal; nothing read that
directory. It does now, and the same verdict is emitted once per session at
SessionStart so it surfaces with no flag and no user action.
The daemon honours mode: "oss". --disconnect claimed every cloud code path keys
off that flag; the daemon had no reference to it and decided from credential
files alone, so a surviving credentials.json or legacy cloud.json kept it
polling while the CLI said disconnected. Only an explicit "oss" vetoes, since
mode postdates enrolments in the field.
The daemon reports the deployment it is actually enforcing, from the active.json
it already materialises, on the poll it already makes. The server has only ever
been able to infer delivery from timestamps.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Setup opened by asking scope, policy bundles, harnesses and cloud of somebody who had just installed the tool and did not yet know what any of them meant. Every one of those has a defensible default, so asking all four up front made the person least able to answer do the most work. Recommended is not a shortcut past the decisions, it is a decision taken once on their behalf: global scope, the CLIs actually detected on this machine, and a named 15-policy set. Customize is the previous wizard unchanged. The cloud question is still asked on both paths. RECOMMENDED_POLICIES is written out rather than derived from defaultEnabled, because those answer different questions — defaultEnabled seeds a checklist of 40, this answers "what should guard a machine whose owner did not want to choose" — and deriving one from the other would reshape the recommended set every time somebody flipped a flag on an unrelated policy. It is the 12 already default-on plus three that were off and should not have been: block-rm-rf, block-force-push and block-secrets-write. Both of the first two are precisely scoped — block-rm-rf only fires at depth <=2 under / or a home directory, exempts /tmp and treats an unresolved $VAR as catastrophic, so rm -rf node_modules is untouched; block-force-push allows --force-with-lease and --force-if-includes. The require-*-before-stop gates, the infra blockers, block-read-outside-cwd and the ten warn-* policies are excluded, each for a reason recorded beside the list. Two things the path must not do, both of which the customize expressions would have done. It writes the UNION of the recommended set and whatever was already enabled, because installHooks runs with replace: true and the bare list would switch off anything added by hand — turning "give me the defaults" into a reduction in protection. And it leaves customPoliciesEnabled alone, because the customize expression reads false when no bundle is ticked and no bundle is ever ticked here, which would disable every .failproofai/policies file on disk. Recommended names all 15 rather than composing bundles because it cannot be composed from them: Dangerous Commands is in no preset, so 8 of the 12 default-on policies are bundle-reachable and 4 are not — including the two that stop the agent disabling failproofai itself. defaultsMissingFromRecommended() and a test keep the two lists from drifting apart. The bundle gap itself is still open. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three loose ends from the recommended path, all of them about the user being
able to see and keep what is theirs.
The review screen said "Policies : 15 enabled" — a number they cannot check
and, on this path, did not choose, which makes that screen the one place they
find out what they agreed to. It now names them, sorted and wrapped to the
same 77-column budget the other summary lines use: writeLines truncates with a
hard cut and no ellipsis, so an over-long line ends mid-slug and reads as a
policy name that does not exist. Capped at 20, because "Everything" is 40 and
a wall of slugs is not transparency; past the cap the count stands with the
command that prints the full list, and nothing is half-rendered — a truncated
list of security policies invites the reader to assume the tail is more of the
same.
Custom and convention policies survive, and that is now asserted rather than
reasoned about. The wizard passes no customPoliciesPath and
removeCustomHooks: false, so neither branch in installHooks that rewrites
customPoliciesPaths runs, and {...previousConfig} carries both it and
customPoliciesEnabled through even under replace: true. A manager test pins
it: the enabled set is replaced as asked while an explicit -c path and the
convention flag are untouched.
The union is tested too, seeded as a real config in the suite's isolated HOME
rather than a mock, because readScopedHooksConfig is the genuine
implementation there. Recommended adds to what was already enabled and never
removes it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Setup opened by explaining itself. The daemon step spent three lines on the warm-worker architecture to somebody who was about to type a password, and the cloud step spent five on what connecting sends. Eleven lines of prose became three: say what is happening and what it costs, and drop the mechanism. The cloud screen keeps the one clause that is not explanation — "Sessions include prompts, file contents and command output" — because this is a consent screen and it is the only place that disclosure is ever made (describeOutcome prints "hook activity" afterwards and never mentions transcripts). Compressing it to "telemetry" would be brevity that is really vagueness. Its option hint now says what the cloud GIVES rather than what it takes: "see what your agents did" was wrong, because the local dashboard already shows that and it made connecting look redundant. Central monitoring and policy deployment are what the key's two scopes actually buy. The stay-local hint points at `config` rather than `--connect`, which is the non-interactive flag and not something to send a person who just declined. "No key?" now points at befailproof.ai/get-started rather than the dashboard host: a person reading that line has no key, which usually means no org either, and a marketing route is the safer thing to hard-code into a CLI than an in-app path that can be reorganised. Failproof Cloud -> FailproofAI Cloud throughout, including two occurrences in bin/failproofai.mjs's help text that a sweep over src/ missed — caught by grepping the built bundle rather than trusting the source edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The daemon's `mode: "oss"` veto never fired. It read `mode` as a string, but
`fp-config.ts` persists `mode: { kind }` — an object — so `as_str()` returned
None and a machine put back on OSS kept polling and shipping exactly as it had
before the veto existed.
The unit tests passed because their fixtures were written from the same wrong
assumption as the code. A test that encodes the bug it is meant to catch is
worth less than no test, because it also reports the case as covered. The
fixtures now use the real shape, and the old flat-string form is kept as a case
that must NOT veto — it is precisely what the original fixtures said.
Correcting it surfaced a second problem: five existing tests began failing,
because `with_credentials_file` never set FAILPROOFAI_HOME and so read the
DEVELOPER's own ~/.failproofai/config.json. They passed on a machine that had
not been set up and failed on one that had. That helper now isolates the home
too — a unit test whose result depends on the laptop running it is not testing
what it says.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A single session was arriving under two agent ids. Confirmed on a customer org: `20260812_133702_31ca19f0` under both `hermes-kratos` and `hermes-telegram`, and a cron session under both `hermes-cron` and the bare fallback — from ONE collector, with the other producer's rows excluded. The id was derived from the session's own `cwd` and `source` columns, re-read on every poll. Hermes rewrites those throughout a run (`hermes_state.py` carries ~20 `UPDATE sessions SET …`), so a session split the moment one changed between two polls. The file documented the assumption as a safety property — "session columns are read fresh on every poll" — while the `pending` map directly below states the opposite rule correctly for tool names. Reading fresh is right for a name and wrong for an identity. Identity now comes from WHICH DATABASE the session is in: the root keeps the bare `hermes` every deployment already ships under, and profiles/<name>/state.db becomes `hermes-<name>`. A file path cannot change under us mid-poll, so this is stable by construction rather than by remembering anything, and it keeps the poll function pure — the format contract requires that, or re-read rows dedup into duplicates instead of collapsing. It also matches the standalone collector's `agent_id_for`, so a machine migrating off it is not renamed. Nothing is lost: `hermes_source` and `hermes_cwd` were already emitted on every event, so "which transport" and "which project" stay answerable as filters over one agent's sessions — which is what they should have been rather than a reason to fragment one database into several agents. The regression test polls twice with `cwd` rewritten in between. Every existing test polled once, which is why a shipping bug sat behind a green suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
First stable release. The version drops its prerelease suffix in package.json and Cargo.toml together — CI checks them against each other because the release tag the CLI builds its daemon download URL from is the npm version, while the binary at that URL reports the Cargo one. This also changes where the package lands. `publish.yml` resolves the dist-tag as "next from a branch, beta for a prerelease on main, else latest", so every 1.0.0-beta.N published from main went to `beta` and `latest` has sat on 0.0.15 for the whole beta line. A version without a suffix is the thing that moves it — which means `npm install -g failproofai` and the bare `npx -y failproofai` in every installed hook command start resolving to this release rather than to 0.0.15. The CHANGELOG's top section becomes `1.0.0` and carries what was accumulating under `1.0.0-beta.22`, with two paragraphs stating the guarantees somebody arriving from a beta needs to know: a configured machine fails closed through the daemon, and a policy that cannot be honoured on a given CLI is recorded as unverified rather than silently reported as blocking. The beta sections stay below as their own history. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe project moves to version 1.0.0. It adds Recommended setup, policy previews, delivery-health reporting, OSS cloud-disconnect handling, applied-deployment polling, updated Codex and Copilot deny responses, and stable Hermes identities. ChangesStable release metadata
Setup and policy selection
Delivery reporting
Cloud enrollment
CLI enforcement
Hermes identity
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
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 |
Contributor
Hermes
No summary yet. What this changesNo component map for this revision. RoundsNo review has finished on this pull request yet. FindingsNothing raised yet.
|
coderabbitai
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
crates/fpai-collect/src/sources/hermes/mod.rs (2)
297-331: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnify the profile-name normalization used for agent ids and cursor keys. Two functions now derive a profile name from the same database path under different rules:
hermes::profile_ofreturns the raw directory name for the agent id, andprofile_dir_namereturns a sanitized, 32-character-capped name for the cursor directory and health key. A profile directory namedProd Atherefore yields agent idhermes-Prod Aand cursor directoryProd-A.Identity stability is not affected, because both derivations read the path rather than a mutable column. Cursor and health keying stay collision-free, because
hermes_reservedusesprofile_dir_nameconsistently. The remaining costs are anagent_idthat can carry spaces, non-ASCII bytes, and unbounded length, and an operator who cannot map an agent id back to its cursor directory by name.
crates/fpai-collect/src/sources/hermes/mod.rs#L297-L331: normalize theprofilesegment insideagent_id_for, or document that callers must pass an already-normalized name.crates/failproofaid/src/main.rs#L1085-L1093: derive one profile name per database and use it for the agent id, the cursor directory, and the health key, instead of callingprofile_ofandprofile_dir_nameseparately.Confirm the ingest side places no constraint on
agent_idbefore choosing to leave the raw name in place.🤖 Prompt for 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. In `@crates/fpai-collect/src/sources/hermes/mod.rs` around lines 297 - 331, Unify profile-name normalization across Hermes agent IDs, cursor directories, and health keys. In crates/fpai-collect/src/sources/hermes/mod.rs:297-331, normalize the profile segment within agent_id_for or explicitly require callers to provide an already-normalized name; in crates/failproofaid/src/main.rs:1085-1093, derive one profile name per database and reuse it for the agent ID, cursor directory, and health key instead of separately calling profile_of and profile_dir_name. Confirm ingest imposes no agent_id constraint before retaining raw names.
297-331: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winNormalize profile names before building agent IDs.
profile_dir_namebounds and sanitizes the same profile name, butagent_id_foremits it unchanged. Use a shared normalizer if agent IDs require the same safe, bounded format. Preserve existing IDs during migration.🤖 Prompt for 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. In `@crates/fpai-collect/src/sources/hermes/mod.rs` around lines 297 - 331, Update agent_id_for to normalize named profiles with the same bounded, sanitized logic used by profile_dir_name before constructing the “{base}-{name}” ID. Reuse the shared normalizer rather than duplicating it, and preserve the existing root-profile ID and established IDs during migration where normalization would otherwise rename them.crates/fpai-collect/src/sources/hermes/transform.rs (1)
147-183: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the ignored
_metaparameter fromagent_id.The function has one caller at
crates/fpai-collect/src/sources/hermes/mod.rs:178. Remove the parameter and update that call.SessionMeta.sourceandSessionMeta.cwdremain used bywith_session_context.🤖 Prompt for 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. In `@crates/fpai-collect/src/sources/hermes/transform.rs` around lines 147 - 183, Remove the unused _meta parameter from the hermes agent_id function and update its sole caller in the Hermes source module to pass only profile_agent_id. Keep SessionMeta usage unchanged in with_session_context.crates/fpai-collect/tests/hermes_source.rs (1)
1143-1143: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct coverage for profile identity helpers.
The existing fixture tests profile path discovery, but no test exercises the profile branches of
agent_id_fororprofile_of. Add assertions forhermes-<name>, the profile name, andNonefor the root database.🤖 Prompt for 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. In `@crates/fpai-collect/tests/hermes_source.rs` at line 1143, Add direct assertions in the existing Hermes fixture test for the profile identity helpers: verify agent_id_for returns “hermes-<name>”, profile_of returns the profile name, and profile_of returns None for the root database. Reuse the fixture’s existing profile and root database handles.
🤖 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`:
- Around line 23-43: Update every new changelog entry in the Features, Fixes,
and Docs sections that currently ends with “(`#PR`)” to use the actual PR
reference “(`#683`)”, preserving the existing descriptions and formatting.
In `@crates/failproofaid/src/cloud_client.rs`:
- Around line 575-585: Update EnvGuard::drop to remove the FAILPROOFAI_HOME
environment variable in its existing cleanup block, alongside the
scratch-directory cleanup, so later tests do not retain the deleted path.
In `@src/hooks/configure-wizard.ts`:
- Around line 1253-1261: Prevent the Recommended-mode branch in
configure-wizard.ts (lines 1253-1261) from assigning an empty recommendedClis
selection; when no supported CLI is detected, route the user to CLI selection or
Customize so setup cannot complete without hook installation. Add coverage in
__tests__/hooks/configure-wizard.test.ts (lines 605-655) asserting the
no-detected-CLI path does not complete setup or skip hook installation.
In `@src/hooks/delivery-health.ts`:
- Around line 144-146: Update the filename filter in the entries loop of
delivery-health processing to accept only names ending in .jsonl or
.jsonl.poison, excluding names such as batch.jsonl.tmp before they affect total
or oldest-batch selection. Add a focused test covering that .jsonl.tmp entries
are ignored.
- Around line 75-88: Update the suffix parsing in the delivery-health logic
around cIdx and aIdx to accept only non-empty ASCII decimal digits, rejecting
whitespace, exponent, hexadecimal, decimal-point, signed, and other non-decimal
forms. Enforce the uploader’s bounds before stripping suffixes: clientStatus
must fit u16 and attempt must fit u32; otherwise leave the base and existing
values unchanged. Add regression tests covering non-decimal and out-of-range
suffixes.
In `@src/hooks/handler.ts`:
- Around line 615-638: Add a handler-level regression test covering
handleHookEvent with both canonical and mapped raw SessionStart events,
asserting deliveryHealth rejection warnings while stdout, stderr, and exitCode
remain unchanged. Add coverage for a non-SessionStart event that verifies
delivery health is neither read nor reported, without modifying existing tests.
---
Nitpick comments:
In `@crates/fpai-collect/src/sources/hermes/mod.rs`:
- Around line 297-331: Unify profile-name normalization across Hermes agent IDs,
cursor directories, and health keys. In
crates/fpai-collect/src/sources/hermes/mod.rs:297-331, normalize the profile
segment within agent_id_for or explicitly require callers to provide an
already-normalized name; in crates/failproofaid/src/main.rs:1085-1093, derive
one profile name per database and reuse it for the agent ID, cursor directory,
and health key instead of separately calling profile_of and profile_dir_name.
Confirm ingest imposes no agent_id constraint before retaining raw names.
- Around line 297-331: Update agent_id_for to normalize named profiles with the
same bounded, sanitized logic used by profile_dir_name before constructing the
“{base}-{name}” ID. Reuse the shared normalizer rather than duplicating it, and
preserve the existing root-profile ID and established IDs during migration where
normalization would otherwise rename them.
In `@crates/fpai-collect/src/sources/hermes/transform.rs`:
- Around line 147-183: Remove the unused _meta parameter from the hermes
agent_id function and update its sole caller in the Hermes source module to pass
only profile_agent_id. Keep SessionMeta usage unchanged in with_session_context.
In `@crates/fpai-collect/tests/hermes_source.rs`:
- Line 1143: Add direct assertions in the existing Hermes fixture test for the
profile identity helpers: verify agent_id_for returns “hermes-<name>”,
profile_of returns the profile name, and profile_of returns None for the root
database. Reuse the fixture’s existing profile and root database handles.
🪄 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: f6bdf336-e135-4dce-b2ec-7784a90fd532
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
CHANGELOG.mdCargo.tomlREADME.md__tests__/audit/scheduled-audit.test.ts__tests__/hooks/configure-wizard.test.ts__tests__/hooks/delivery-health.test.ts__tests__/hooks/manager.test.ts__tests__/hooks/policy-attribution.test.ts__tests__/hooks/policy-evaluator.test.ts__tests__/hooks/policy-presets.test.tsbin/failproofai.mjscrates/failproofaid/src/cloud_client.rscrates/failproofaid/src/main.rscrates/fpai-collect/src/sources/hermes/mod.rscrates/fpai-collect/src/sources/hermes/transform.rscrates/fpai-collect/tests/hermes_source.rspackage.jsonsrc/audit/audit-schedule.tssrc/hooks/cloud-connection.tssrc/hooks/cloud-enrollment-cli.tssrc/hooks/cloud-enrollment.tssrc/hooks/configure-wizard.tssrc/hooks/delivery-health.tssrc/hooks/enforcement-capability.tssrc/hooks/fp-config.tssrc/hooks/handler.tssrc/hooks/policy-evaluator.tssrc/hooks/policy-presets.ts
Contributor
Hermes
The PR fixes the prior default-Claude fallback, but Recommended setup can now finish with no hooks at all when no supported CLI is detected. Static parsing of modified TypeScript passed in an isolated container; dependency-backed tests could not run because the isolated container has no cached packages and network access is disabled. What this changesflowchart LR
n0Setupwizard["~ Setup wizard"]
n1Policypresets["~ Policy presets"]
n2Hookenforcement["~ Hook enforcement"]
n3Deliveryhealthreporting["+ Delivery health reporting"]
n4Cloudpolicydaemon["~ Cloud policy daemon"]
n5Hermescollector["~ Hermes collector"]
n0Setupwizard -- "selects recommended policies" --> n1Policypresets
n0Setupwizard -- "installs CLI hook configs" --> n2Hookenforcement
n0Setupwizard -- "installs daemon service" --> n4Cloudpolicydaemon
n3Deliveryhealthreporting -- "emits SessionStart warnings" --> n2Hookenforcement
n4Cloudpolicydaemon -- "supplies managed policies" --> n2Hookenforcement
n5Hermescollector -- "ships session batches" --> n3Deliveryhealthreporting
Rounds
FindingsOpen
|
hermes-exosphere
previously approved these changes
Aug 12, 2026
hermes-exosphere
left a comment
hermes-exosphere
left a comment
Contributor
There was a problem hiding this comment.
Hermes found no blocking issues in this revision.
1 advisory finding
- Medium/High Recommended setup falls back to Claude when no CLI was detected —
recommendedClisis derived solely fromdetectInstalledClis()and is assigned toclisSelat src/hooks/configure-wizard.ts:1260. If detection returns [], the resulting emptyclisarray is passed toinstallHooks; its existing default at src/hooks/manager.ts:167 interprets an empty array as["claude"]. Thus choosing Recommended on a machine with no detected CLI still creates/configures Claude hooks, contrary to the stated detected-CLIs-only behavior. (src/hooks/configure-wizard.ts:1260)
Two e2e tests still asserted `hookSpecificOutput.additionalContext` on a PostToolUse deny for codex and copilot — the shape the previous commit removed for exactly those two CLIs, because neither reads it. They belonged in that commit and are red on CI without them. The shared `assertPostToolUseDeny` is left alone. Sixteen of its eighteen call sites are claude, which does read the nested shape and is unaffected, so widening the helper to accept either form would have made every one of those sites pass on the wrong shape as well. The new assertion also requires `hookSpecificOutput` to be ABSENT rather than merely unread. Emitting both shapes would satisfy a looser check while leaving the file asserting a contract no consumer exercises — and the whole defect this fixes was a response that looked correct and was read by nobody. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The cloud screen's second line read "No key? Create one at …". That phrases the situation as a missing artefact, which lands wrong on the person it is for: somebody seeing this screen on a first run has not mislaid a key, they have never heard of the product, and "No key?" reads to them as an error state rather than an invitation. It now names the product — "New to FailproofAI? Create a key at https://befailproof.ai/get-started/" — which is self-selecting: a reader with a key skips it, and a reader without one is told what to do. The destination is unchanged and still deliberate: get-started rather than the dashboard host, because a person reading this line usually has no org either and get-started issues both. Also resolves the twelve `(#PR)` placeholders in the 1.0.0 section to #683. The seventy-four elsewhere in the file belong to other PRs and are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
hermes-exosphere
previously requested changes
Aug 12, 2026
hermes-exosphere
left a comment
hermes-exosphere
left a comment
Contributor
There was a problem hiding this comment.
Hermes found blocking issues that should be addressed.
High: Recommended setup silently applies no protection when no CLI is detected
- Rule:
COR-001 - Location:
src/hooks/configure-wizard.ts:1260 - Evidence:
recommendedCliscomes only fromdetectInstalledClis()(src/hooks/configure-wizard.ts:1122-1124). Recommended mode assigns that list directly toclisSel(line 1260), while the apply loop callsinstallHooksonly whenclisForScope.length > 0(line 1791). With no detected supported CLI, the wizard still completes and marks the daemon configured, but installs no hooks and never persists the recommended policy set. - Required change: Do not permit a successful Recommended apply with an empty detected-CLI list: direct the user to Customize/select a supported CLI, or require a detected CLI before continuing. Add a regression test for zero detected CLIs.
NiveditJain
approved these changes
Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First stable release.
latesthas sat on 0.0.15 for the whole beta line, becausepublish.ymlresolves the dist-tag as next from a branch, beta for a prerelease on main, else latest — so every1.0.0-beta.Nwent tobeta. Dropping the suffix is what moveslatest, which meansnpm install -g failproofaiand the barenpx -y failproofaiin every installed hook command start resolving here instead of to 0.0.15.The theme
Every fix below is one bug wearing different clothes: the user's belief about their protection diverged from reality and nothing reconciled the two. None of them were crashes or test failures — all are success paths, which is why none were caught.
Enforcement that wasn't
{decision:"block", reason}; we senthookSpecificOutput.additionalContext, which neither reads. So the policy ran, the verdict was logged, the dashboard counted it as enforcement — and nothing happened. Verified rather than inferred: a live A/B probe on codex 0.147.0 (block shape →hook: PostToolUse Blocked, the reason replaces the tool result and the model never sees the real output; our shape →Completed, model reads it verbatim), and a re-read of the shipped copilot 1.0.78 bundle where bothpostToolUsesites gate onvK = t => t?.decision==="block" && typeof t.reason==="string".mode: "oss"never stopped the daemon.--disconnectwrites the flag and claims "every cloud code path keys off this flag" — true of the CLI, false of the daemon, which holds the socket and had never read it. It now readsmode.kind(the shapefp-config.tsactually persists; reading it as a string is why an earlier cut of this fired zero times).--statusread the credential file, which records--connecttime and is never revisited. The uploader already writes its verdict into the parked batch's filename and already declines to retry a definitive refusal — nothing read that directory. Now--statusreports from it, and the same verdict fires once per session atSessionStart, so it surfaces with no flag and no user action.cwd/sourceand re-read every poll, and Hermes rewrites those throughout a run — so a session split the moment one changed. Identity now comes from which database the session is in, which cannot change mid-poll. Matches the standalone collector's scheme, so migrating machines are not renamed.Setup is two questions
failproofai configopens with Recommended vs Customize instead of asking scope, bundles, harnesses and cloud of somebody who just installed the tool. Recommended is a decision taken on their behalf — global scope, detected CLIs, a named 15-policy set — not a shortcut past the decisions. Customize is the previous wizard, unchanged.RECOMMENDED_POLICIESis written out with its reasoning rather than derived fromdefaultEnabled, and includes three that were off and should not have been:block-rm-rf,block-force-push,block-secrets-write. It unions with what was already enabled, becauseinstallHooksruns withreplace: trueand writing the bare 15 would turn "give me the defaults" into a reduction in protection.Setup's prose is down by two thirds. The consent clause stays — this is the only place transcripts are ever disclosed.
Tests that could not fail
Two are worth calling out because they were worse than no test:
modeveto's fixtures carried the same wrong assumption as the code, so they reported a broken case as covered. Fixing them surfaced five more tests that read the developer's own~/.failproofai/config.json— passing on a machine that had not been set up, failing on one that had.cwdrewritten in between.Gates
3456 TS tests · 28 Rust suites · tsc, clippy and fmt clean.
Known, and deliberately not in this PR
enforcement-capability.ts's codex rows cite source paths that no longer exist in 0.147.0. Marked unverified rather than left looking like evidence — a re-probe is outstanding.defaultEnabledpolicies are in no preset, so a first run that picks bundles omits the two that stop the agent disabling failproofai. Recommended routes around it; the bundles still have it.cwd, so it can disagree with the Hermes session source. Same instability class, not addressed here.🤖 Generated with Claude Code
https://claude.ai/code/session_01HEwcerc9jE7ZBkfBiYRbep
Hermes review
3242b91f4d9cf5e9fae42c6d19736ed03f3488a11d8f31d926828f3bae215c58f5b35baa44acbff0gpt-5.6-terraSummary
The PR fixes the prior default-Claude fallback, but Recommended setup can now finish with no hooks at all when no supported CLI is detected. Static parsing of modified TypeScript passed in an isolated container; dependency-backed tests could not run because the isolated container has no cached packages and network access is disabled.
Changes
Validation
Passeddocker run --rm --network=none -v /review/input/workspace:/workspace:ro -w /workspace oven/bun:latest sh -lc 'bun build src/hooks/configure-wizard.ts --target=node --external="*" --outfile=/tmp/configure-wizard.js && bun build src/hooks/delivery-health.ts --target=node --external="*" --outfile=/tmp/delivery-health.js && bun build src/hooks/policy-evaluator.ts --target=node --external="*" --outfile=/tmp/policy-evaluator.js'— All three modified TypeScript modules parsed and bundled successfully in an isolated container. (0s)Skippeddocker run --rm --network=none -v /review/input/workspace:/workspace:ro -w /tmp oven/bun:latest sh -lc 'cp -a /workspace review && cd review && bun install --frozen-lockfile --offline && bunx vitest run __tests__/hooks/configure-wizard.test.ts __tests__/hooks/delivery-health.test.ts'— The isolated container has no dependency cache; installing requires registry access, which is intentionally disabled by the harness network isolation. (0s)Findings
recommendedCliscomes only fromdetectInstalledClis()(src/hooks/configure-wizard.ts:1122-1124). Recommended mode assigns that list directly toclisSel(line 1260), while the apply loop callsinstallHooksonly whenclisForScope.length > 0(line 1791). With no detected supported CLI, the wizard still completes and marks the daemon configured, but installs no hooks and never persists the recommended policy set. (src/hooks/configure-wizard.ts:1260)Open questions
None.
Policy overrides
None.
Summary by CodeRabbit