Skip to content

[beta.13] Extra capture paths per agent CLI, the deployment/version rename, and two silent enforcement gaps - #663

Open
chhhee10 wants to merge 11 commits into
mainfrom
chore/wizard-copy-no-safety-net
Open

[beta.13] Extra capture paths per agent CLI, the deployment/version rename, and two silent enforcement gaps#663
chhhee10 wants to merge 11 commits into
mainfrom
chore/wizard-copy-no-safety-net

Conversation

@chhhee10

@chhhee10 chhhee10 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Ten commits, and the original title (a one-line wizard copy change) is now the smallest thing in here. Retitled to match what it actually contains.

Lands with FailproofAI/agenteye#559. The two are one change split across two repos: #559 renames the cloud-policy vocabulary through AgentEye's schema, server and dashboard; this renames it through the daemon and moves the wire contract to schemaVersion: 2. Verified together end to end — see Cross-repo verification below.


1. Setup flow — failproofai flush, back-navigation, five defects (fd9cf998, d0132a8e, 15363627)

failproofai flush delivers what is already spooled instead of waiting out the sweeper's interval, with --wait blocking until the spool drains or its timeout expires. The install stopped waiting on a blind sleep; the wizard gained ← back-navigation; cloud-managed policies list with their deployment, version and status.

2. The rename — a generation is a deployment, a revision is a version (921b7f60)

Words the product used nowhere else. Renamed through the daemon, its persisted state and every test name.

3. Capture sessions from more than one location per agent CLI (3ecab827)

Every source watched exactly the place its own installer chose. That is right for one laptop and wrong for a second profile, a mounted team share, a container's home bind-mounted beside the host's, or an agent an operator relocated — all of which hold real sessions that nothing collected.

failproofai harness add-path claude work=/srv/team/.claude/projects
failproofai harness list
failproofai harness remove-path claude work

Writes [collector.sources.<harness>] extra_paths in config.toml; FAILPROOFAI_<HARNESS>_EXTRA_PATHS overrides for containers. 12 harnesses, 13 sourcesclaude covers subagent transcripts, which share a root and would otherwise be dropped silently under an added path. Grammar, folder-name label fallback and <label>-<agentId> namespacing match AgentEye's own collector rather than inventing a second answer.

Three things are structural, not cosmetic:

Decision What breaks without it
Each labelled path is its own task roots is walked by one task with one Params, and the label lives in Params — a second root there inherits the default's agent id
Each gets its own cursor directory The store rewrites its whole map atomically; sharing means both clobber each other's watermark and re-read from zero after every restart
Each gets its own health key Otherwise instances overwrite each other's record and root_present alternates, destroying the "absent root versus idle source" distinction. Hermes hit exactly this with per-profile databases

The <label>- prefix is applied in SpoolWriter::push — the one point every event passes through — and not in either engine. A SQLite format takes params.agent_id only as a fallback and derives the real id from the row (devin::agent_iddevin-<project>, which never consults the fallback for a session with a working directory, i.e. all of them). Prefixing in the engine would namespace only the sessions that failed to derive an id; a test fails on precisely that mistake.

No restart and no sudo: the collector manager already re-reads config.toml on an interval and cycles on any CollectorConfig change, and sources rides that by living inside the compared value (79be966b).

4. Upgrades silently emptied the policy set (9c498e5b)

Layout 1 kept the user's policy selection at ~/.failproofai/policies-config.json; layout 2 keeps it at policies/local-policies/policies-config.json — and both were on the reset list. The reset runs unattended on the first command after an upgrade, and the machine still read as configured afterwards (isConfigured() is a union that sees the agent CLIs' untouched settings files), so the wizard never re-asked.

Measured in a container, identical starting state, two upgrade targets:

@latest → published @next @latest → this branch
enabledPolicies absent all three carried
decision log 252 → 252 252 → 252
claude hook entries 56 56
enforces after upgrade NO YES

Hook entries stayed installed in every agent CLI and kept firing — against an empty policy set. An allowlist, not a copy: layout 1's file also held a collector block in camelCase, and layout 2 moved those to snake_case in config.toml, so carrying it would add a key nothing reads that looks preserved and behaves absent. Read-before/write-after, because its source and its destination's parent are both on the reset list.

5. Review fixes (18eaebb5) — 9 CodeRabbit threads, two of them real bugs

  • ← hung the wizard. BACK is a symbol the shared handler injects, and multiSelect's summary calls values.includes(...)TypeError, thrown inside finish before resolve(). The promise never settled and input stopped responding. selectOne rendered Symbol(failproofai.back) as the user's answer. Handled once in collapse().
  • Pre-rename cloud state became unreadable. Both persisted structs carry deny_unknown_fields, so an upgraded daemon failed to parse its own active.json on three counts at once.
  • flush reported success on platforms with no daemon; mocked git errors said unknown version, a string git never produces; #PR placeholders, --wait wording, revversion in the activity detail.

6. schemaVersion: 2 (76acc68f)

The v1 payload named its fields generation and revision; after the rename it carried neither, at the same version number. #559 now emits 2, and this accepts both — 1 only for files already on disk, never from a server, because a machine that ran an earlier beta has a desired-state.json and active.json at version 1 and deny_unknown_fields would make them unreadable.

That asymmetry decides where the aliases live: kept on the persisted ActiveDeployment/ActivePolicy (bytes possibly written by an older daemon), removed from the wire DesiredState/DesiredPolicy (no server can emit the old spelling, so an alias is dead code and a silently-tolerated stale field is how two sides drift apart). Tests pin each half, including that the wire now refuses the old spelling.

The TypeScript hook reader had the same constant and accepted only 1 — the worst shape of this bug: the daemon reconciles, writes a correct active.json, logs "deployment 1 active", and the hook path alone refuses it, so cloud policy stops being enforced while every other signal reads healthy. Reproduced exactly that way against a live #559 server before fixing it.


Cross-repo verification

Against a real local stack (Postgres + ClickHouse + Redis + server + dashboard) running #559's branch, with a daemon built from this branch:

  • fresh database applies all 68 migrations; a database seeded through the old API keeps its rows across the rename
  • /enforcement/v1/desired-state emits deployment/version at schemaVersion: 2
  • the daemon pulls it, SHA-256 verifies every artifact, writes active.json at v2, and a matching tool call is denied with that policy's reason

Multi-path, five harnesses across both engines, default and extra path:

harness engine default extra path
claude file-tail claude-work-default share-claude-work-share
goose SQLite goose-work-goose alt-goose-work-goose
opencode SQLite opencode-work-oco alt-opencode-work-oco
openclaw file-tail openclaw-main alt-openclaw-main
pi file-tail pi-work-pi alt-pi-work-pi

claude, goose and opencode were driven with a real LLM. OpenClaw is the strongest case — both sessions derive the same id, so only the label separates them. A path added to a running daemon was captured within one poll interval (task count 21 → 23), and its first transcript reached the server.

Tests

8 engine tests through the real filetail/sqlitepoll loops; 16 grammar unit tests; a cross-language contract test parsing the exact config.toml bytes the TypeScript CLI emits; a test reading HARNESS_KEYS out of main.rs so the two hand-maintained lists cannot drift; 7 migration tests; 3 pre-rename compatibility tests; back-navigation and validation-row tests; and an e2e case driving the real binary against a hand-edited config. Two were explicitly proved non-vacuous by reverting the fix and watching them fail.

cargo test --workspace 624 passing, clippy + fmt clean; tsc --noEmit clean; bun run lint 0 errors; bun run test:run 3261 passing.

One pre-existing failure, not from this branch: daemon-service.test.ts > refuses to half-install when it cannot elevate asserts /etc/systemd/system/failproofaid@<user>.service is absent, and the dev machine it ran on has a real one installed. It passes in CI, and fails identically on this branch with every change stashed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CCSFM55BcUEAHECabpeSr4

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The CLI adds flush with optional waiting and timeout handling. The daemon consumes flush requests and immediately drains pending batches. The setup wizard supports back navigation, uses “harnesses”, and places API-key connection first. Cloud policy metadata now uses deployment/version terminology.

Changes

Flush and interface updates

Layer / File(s) Summary
Add flush command and daemon delivery
src/hooks/flush-cli.ts, bin/failproofai.mjs, crates/failproofaid/src/..., crates/fpai-collect/src/..., related tests
The CLI counts pending batches, checks prerequisites, writes flush requests, optionally waits for delivery, and reports timeout state. The daemon consumes requests and triggers an immediate sweeper pass.
Update setup navigation and prompt rendering
src/hooks/configure-wizard.ts, src/hooks/tui.ts, bin/failproofai.mjs, __tests__/hooks/configure-wizard.test.ts, __tests__/hooks/tui.test.ts
The wizard supports back navigation, uses “harnesses”, and uses API-key-first ordering. Prompts truncate long content to terminal width.
Rename cloud policy deployment metadata
crates/failproofaid/src/cloud_policies.rs, src/hooks/cloud-managed-policies.ts, src/hooks/handler.ts, crates/fpai-collect/src/sources/hooks/transform.rs, app/policies/hooks-client.tsx, related tests
Cloud policy structures, paths, manifests, attribution fields, telemetry fields, and UI output use deployment/version terminology.
Improve daemon readiness and cloud policy listing
src/hooks/daemon-service.ts, src/hooks/manager.ts, related tests
Daemon startup polling checks service state and socket readiness. Hook listing displays active cloud-managed policies and suppresses manifest read failures.
Document the beta.13 release
CHANGELOG.md
The changelog records the flush feature and related setup, prompt, terminology, cloud listing, daemon, and CLI documentation updates.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested labels: enhancement

Suggested reviewers: hermes-exosphere

Poem

A rabbit sends a flush request,
The daemon clears the waiting nest.
Harnesses guide the setup right,
Cloud deployments shine in light,
Beta.13 completes its quest.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies major changes, including the deployment/version rename and enforcement fixes, but it omits some other changes.
Description check ✅ Passed The description gives detailed scope, cross-repository context, verification results, and known test limitations, but omits the template headings and checkboxes.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `#PR` placeholder in the changelog entry with `#663`,
preserving the existing dated, versioned entry and single-line description.
🪄 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: 46e07879-1743-42f6-86a2-69eaf27e55d0

📥 Commits

Reviewing files that changed from the base of the PR and between 9a51e57 and 01d4973.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • src/hooks/configure-wizard.ts

Comment thread CHANGELOG.md Outdated
@chhhee10
chhhee10 force-pushed the chore/wizard-copy-no-safety-net branch from bfd4dc3 to 01d4973 Compare August 7, 2026 10:04
@coderabbitai coderabbitai Bot added the enhancement New feature or request label Aug 7, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/hooks/flush-cli.ts (1)

540-560: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Consume and validate all flush arguments.

flush --wait unexpected ignores unexpected. flush --timeout 5 accepts a timeout that has no effect because --wait is absent. Repeated --timeout flags silently use only the first value.

Parse the argument list in one pass. Reject positional arguments, duplicate flags, and --timeout without --wait.

🤖 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 `@src/hooks/flush-cli.ts` around lines 540 - 560, Update the flush CLI argument
parsing in the relevant flow of flush-cli.ts to consume and validate the full
argument list in one pass. Reject positional arguments such as unexpected,
duplicate --wait or --timeout flags, and any --timeout usage without --wait;
preserve the existing valid option behavior while ensuring all supplied
arguments are accounted for.
🧹 Nitpick comments (1)
crates/fpai-collect/src/delivery.rs (1)

251-277: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a direct flush-sweep behavior test.

The new branch is not exercised by the current tests. crates/fpai-collect/tests/delivery.rs passes an AtomicBool that remains false. The existing stale_batches tests do not verify that sweep observes a request and bypasses both the minimum age and the per-pass limit.

Start a sweeper with fresh batches, set the shared flag, and assert that all batches drain without waiting for SWEEP_MIN_AGE.

🤖 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/delivery.rs` around lines 251 - 277, Add a delivery
sweep test in the existing delivery test suite that starts the sweeper with
fresh batches and a shared AtomicBool flush flag, sets the flag while the
sweeper is running, and verifies all batches are delivered without waiting for
SWEEP_MIN_AGE or being limited by SWEEP_MAX_FILES. Use the existing sweep and
delivery test helpers, ensuring the test exercises the flush_now branch
directly.
🤖 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 `@__tests__/hooks/tui.test.ts`:
- Around line 143-189: Add coverage to the promptText redraw tests for a failing
validate callback after pressing Enter, including the validation-error display
path. Update the measurement helper to split each ANSI-stripped write on newline
boundaries and measure the widest individual row rather than the complete write,
then assert every rendered row stays within the terminal width.

In `@CHANGELOG.md`:
- Line 6: Update the changelog entry for `failproofai flush --wait` to state
that it waits until the spool drains or the configured timeout expires, while
preserving the existing description of flush behavior.

In `@src/hooks/flush-cli.ts`:
- Around line 109-121: Update the flush flow around isDaemonSupportedPlatform()
to return the same error response before writing flush-request.json when the
platform is unsupported; preserve the existing daemon status handling for
supported platforms. Add a regression test for the flush entry point that mocks
an unsupported platform and verifies an error is returned and no request file is
created.

---

Outside diff comments:
In `@src/hooks/flush-cli.ts`:
- Around line 540-560: Update the flush CLI argument parsing in the relevant
flow of flush-cli.ts to consume and validate the full argument list in one pass.
Reject positional arguments such as unexpected, duplicate --wait or --timeout
flags, and any --timeout usage without --wait; preserve the existing valid
option behavior while ensuring all supplied arguments are accounted for.

---

Nitpick comments:
In `@crates/fpai-collect/src/delivery.rs`:
- Around line 251-277: Add a delivery sweep test in the existing delivery test
suite that starts the sweeper with fresh batches and a shared AtomicBool flush
flag, sets the flag while the sweeper is running, and verifies all batches are
delivered without waiting for SWEEP_MIN_AGE or being limited by SWEEP_MAX_FILES.
Use the existing sweep and delivery test helpers, ensuring the test exercises
the flush_now branch directly.
🪄 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: 0a8b3f07-7bb3-4cd4-b609-d7a175423fde

📥 Commits

Reviewing files that changed from the base of the PR and between 01d4973 and 9e23454.

📒 Files selected for processing (12)
  • CHANGELOG.md
  • __tests__/hooks/configure-wizard.test.ts
  • __tests__/hooks/flush-cli.test.ts
  • __tests__/hooks/tui.test.ts
  • bin/failproofai.mjs
  • crates/failproofaid/src/main.rs
  • crates/failproofaid/src/paths.rs
  • crates/fpai-collect/src/delivery.rs
  • crates/fpai-collect/tests/delivery.rs
  • src/hooks/configure-wizard.ts
  • src/hooks/flush-cli.ts
  • src/hooks/tui.ts

Comment thread __tests__/hooks/tui.test.ts
Comment thread CHANGELOG.md Outdated
Comment thread src/hooks/flush-cli.ts Outdated
@coderabbitai coderabbitai Bot added the bug Something isn't working label Aug 7, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@src/hooks/tui.ts`:
- Around line 509-512: Update the internal prompt result handling around
multiSelect and selectOne to recognize the Back result before array or choice
formatting, returning the “back” summary without passing the sentinel to
values.includes or rendering it as a choice. Widen the internal result type to
include Back, and add a direct left-arrow multiSelect test in the existing TUI
hook tests covering successful back navigation.
🪄 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: b45b7acb-e577-43a2-8388-89112d66bb61

📥 Commits

Reviewing files that changed from the base of the PR and between 9e23454 and a632ebb.

📒 Files selected for processing (8)
  • CHANGELOG.md
  • __tests__/hooks/configure-wizard.test.ts
  • __tests__/hooks/daemon-service.test.ts
  • __tests__/hooks/manager-cloud-listing.test.ts
  • src/hooks/configure-wizard.ts
  • src/hooks/daemon-service.ts
  • src/hooks/manager.ts
  • src/hooks/tui.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md

Comment thread src/hooks/tui.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
__tests__/hooks/worker-server.test.ts (1)

247-247: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Rename the stale directory variable.

generationDir now points to deployments/42. Rename it to deploymentDir and update its local uses. This keeps the test aligned with the deployment terminology.

Proposed rename
-    const generationDir = join(managedRoot, "deployments", "42");
+    const deploymentDir = join(managedRoot, "deployments", "42");
🤖 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 `@__tests__/hooks/worker-server.test.ts` at line 247, Rename the local variable
generationDir to deploymentDir in the affected test and update every local
reference accordingly, preserving the existing deployments/42 path and test
behavior.
__tests__/hooks/policy-attribution.test.ts (1)

119-123: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Assert the active deployment in the plain-allow case.

evaluateHookEvent records cloudDeployment when an active cloud deployment exists, even when no policy decides the event. This test only checks that cloudVersion is absent. Add an assertion for cloudDeployment to protect this contract.

Proposed assertion
     expect(row().policySource).toBeUndefined();
     expect(row().cloudVersion).toBeUndefined();
+    expect(row().cloudDeployment).toBe(184);
🤖 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 `@__tests__/hooks/policy-attribution.test.ts` around lines 119 - 123, Update
the plain-allow test around evaluateHookEvent to also assert that
row().cloudDeployment matches the active cloud deployment, while preserving the
existing undefined assertions for policySource and cloudVersion.
🤖 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 `@__tests__/hooks/builtin-policies.test.ts`:
- Line 2756: Update the command-error mocks in the base-ref and related cases
around opts.baseRefExists, including the referenced cases, to use realistic Git
diagnostics such as “unknown revision or path not in the working tree,”
“ambiguous argument,” or “Needed a single revision” instead of “unknown
version”; ensure the production classifier recognizes those diagnostics if the
tests exercise it.

In `@app/policies/hooks-client.tsx`:
- Around line 399-400: Update the cloud policy detail template in the relevant
render expression to label item.cloudVersion as “version” instead of “rev”,
preserving the existing cloud metadata and policySource branches.

In `@CHANGELOG.md`:
- Line 8: Remove the duplicate “### Features” heading from the changelog while
preserving the cloud-policy entry under the existing Features section for
version 1.0.0-beta.13.

In `@crates/failproofaid/src/cloud_policies.rs`:
- Around line 30-44: Preserve schemaVersion 1 compatibility across the
cloud-policy migration: in crates/failproofaid/src/cloud_policies.rs:30-44,
update DesiredState deserialization to accept legacy generation and revision
fields; in crates/failproofaid/src/cloud_policies.rs:71-81, apply the same
compatibility to the active-manifest deserializer. In
src/hooks/cloud-managed-policies.ts:36-62, normalize legacy schemaVersion 1
fields, including generations/<id> paths, before validation. Add the
corresponding upgrade fixture in
__tests__/hooks/cloud-managed-policies.test.ts:19-28.

---

Nitpick comments:
In `@__tests__/hooks/policy-attribution.test.ts`:
- Around line 119-123: Update the plain-allow test around evaluateHookEvent to
also assert that row().cloudDeployment matches the active cloud deployment,
while preserving the existing undefined assertions for policySource and
cloudVersion.

In `@__tests__/hooks/worker-server.test.ts`:
- Line 247: Rename the local variable generationDir to deploymentDir in the
affected test and update every local reference accordingly, preserving the
existing deployments/42 path and test behavior.
🪄 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: b43d42a3-41d7-46ce-a1d7-bb6c91d01850

📥 Commits

Reviewing files that changed from the base of the PR and between a632ebb and 96bb33f.

📒 Files selected for processing (34)
  • CHANGELOG.md
  • __tests__/hooks/builtin-policies.test.ts
  • __tests__/hooks/cloud-connect-permissions.test.ts
  • __tests__/hooks/cloud-enrollment-cli.test.ts
  • __tests__/hooks/cloud-enrollment.test.ts
  • __tests__/hooks/cloud-managed-policies.test.ts
  • __tests__/hooks/configure-wizard.test.ts
  • __tests__/hooks/fail-closed-force-decision.test.ts
  • __tests__/hooks/handler.test.ts
  • __tests__/hooks/manager-cloud-listing.test.ts
  • __tests__/hooks/policy-attribution.test.ts
  • __tests__/hooks/session-pause-enforcement.test.ts
  • __tests__/hooks/worker-server.test.ts
  • __tests__/integration-suite/is-error.test.ts
  • app/policies/hooks-client.tsx
  • crates/failproofaid/src/cloud_client.rs
  • crates/failproofaid/src/cloud_policies.rs
  • crates/failproofaid/src/main.rs
  • crates/failproofaid/src/paths.rs
  • crates/failproofaid/src/telemetry.rs
  • crates/failproofaid/tests/collector_reload_e2e.rs
  • crates/fpai-collect/src/health.rs
  • crates/fpai-collect/src/sources/hooks/transform.rs
  • crates/fpai-collect/tests/hooks_source.rs
  • src/hooks/cloud-connection.ts
  • src/hooks/cloud-enrollment-cli.ts
  • src/hooks/cloud-enrollment.ts
  • src/hooks/cloud-managed-policies.ts
  • src/hooks/custom-hooks-loader.ts
  • src/hooks/daemon-service.ts
  • src/hooks/fp-home.ts
  • src/hooks/handler.ts
  • src/hooks/hook-activity-store.ts
  • src/hooks/manager.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • tests/hooks/manager-cloud-listing.test.ts
  • tests/hooks/configure-wizard.test.ts
  • src/hooks/manager.ts
  • crates/failproofaid/src/paths.rs
  • src/hooks/daemon-service.ts
  • crates/failproofaid/src/main.rs

Comment thread __tests__/hooks/builtin-policies.test.ts Outdated
Comment thread app/policies/hooks-client.tsx Outdated
Comment thread CHANGELOG.md Outdated
Comment thread crates/failproofaid/src/cloud_policies.rs
chhhee10 and others added 5 commits August 7, 2026 19:53
… a safety net

"let's set up your safety net" was the first line of `failproofai config`, and
it says nothing a first-time user can act on: it names no tool, describes no
step, and leaves them guessing what the next four screens will change on their
machine. The wizard installs hook entries across up to twelve agent CLIs and a
root-owned system service — an opening line that reaches for a metaphor instead
of naming the thing is at odds with what follows.

"let's set up failproofai" is what the command actually does.

This was the only occurrence in the wizard. The word survives in
`src/audit/archetypes.ts` and `src/audit/findings.ts`, which are user-facing
audit copy written in a deliberate persona voice — a separate call, left alone
here rather than swept up mechanically.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015BDLTPtbQvUE62eCfQrUbf
Reported from a live run-through of `failproofai config`.

**The API-key prompt printed one copy of itself per character.** `\r\x1b[2K`
erases the row the cursor is on and nothing above it. `API key for <host>` plus
the masked value plus the `needs events:add · policies:pull …` hint is past 80
columns before the key is half typed, so the line WRAPPED, the erase reached
only its last row, and every keystroke left the previous row on screen. Pasting
a 40-character key stacked 40 prompts down the terminal. The prompt now
truncates to one physical row. The regression test is red-proven: without the
fix it renders 88 columns into an 80-column terminal.

**Cloud was offered second and "stay local" was preselected**, which is not what
most people running the wizard came to do. The two options are swapped; neither
option's copy changed, so staying local is still stated as plainly as it was and
is one keystroke away.

**"AI assistants" is a word no other surface uses.** The wizard protects agent
CLIs — harnesses. Four user-facing strings, including the `12 assistants` count
on the closing line.

**`--help` did not mention `backfill` at all**, nor any of the `config` cloud
flags (`--connect`, `--token`, `--machine-id`, `--machine-label`,
`--no-transcripts`, `--disconnect`, `--status`, `--pause`, `--resume`). A flag
nobody can discover is a flag that does not exist.

**`failproofai flush` is new.** The collector is unhurried by design — swept
once older than two minutes, at most 64 per pass, every 60 seconds — pacing that
keeps a backlog from stampeding the server and is exactly wrong for somebody
watching a dashboard for their own events, where "not delivered yet" and "not
working" look identical. `flush` asks the daemon for a pass with no minimum age
and no cap; `--wait` blocks until the spool drains so a script can flush and
then assert.

It hands off rather than doing the work, for the same reason `backfill` does:
the uploader's concurrency limiter and in-flight set live in the running daemon,
so a second uploader started by the CLI would POST every batch twice. It does
NOT hand off the checking — collection off, no credential, daemon stopped are
each verified synchronously, because a CLI that prints "requested" while the
daemon is down has told the user the opposite of what happened.

The daemon side is a flag rather than a channel: the sweeper is rebuilt whenever
the collector cycles, and a receiver would go with it, dropping a request
already taken off disk.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015BDLTPtbQvUE62eCfQrUbf
…nav, list cloud policies

**The daemon install slept 750ms for no reason, twice.** Measured on a real
machine: systemctl stop/start/enable/daemon-reload are 3-6ms each, the socket
appears at 13ms, and a real hook is answered at 125ms. Then
`waitForDaemonRunning` slept a flat `SERVICE_SETTLE_MS` and read the status once
at the end — wrong in both directions. Healthy: setup sat out the remaining
~600ms with the answer already in hand, and the repair path does this twice
(uninstall, reinstall). Broken: a daemon that died at 100ms was not noticed
until 750ms, because nothing looked until the sleep was over.

The window is now watched rather than slept through. Leaving `running` fails
immediately, and the wait ends early once the daemon accepts a connection — a
strictly stronger signal of "did not die at startup" than "still active after an
arbitrary sleep", which is all the settle ever established. The connect check,
not a hook evaluation: `probeDaemon` runs the end-to-end one moments later and
paying twice is the thing being fixed.

The timing is now injectable, so the tests assert the SHAPE of the wait against
a virtual clock instead of spending it: early exit, startup-death detected on
the first poll, full window still held when nothing answers, give-up when the
unit never starts.

**The wizard could not go back.** Cancel and "go back" were both `null`, so
changing an earlier answer meant abandoning setup. `←` on the harness step now
returns to the policy step with the previous selection still ticked.

`BACK` is a symbol, not a sentinel string: a caller's value type is its own and
`selectOne<string>` could legitimately carry "back" as a real choice. It reaches
the return type through overloads, so the dozens of existing call sites are
unchanged rather than widened to handle a value they can never receive. The
policy step deliberately takes no `allowBack` — the scope question before it is
often not asked at all, so `←` there would sometimes go nowhere.

**Cloud-managed policies were invisible to `failproofai policies`.** They
enforce on the machine exactly like the builtins and convention policies the
command already lists, so it was answering "what is enforcing here?" with a
subset — and the policies an operator pushed to a fleet were the ones the person
standing at the machine could not see.

Listed read-only, because they are owned by the deployment: `--uninstall` cannot
switch one off, and printing them beside toggleable rows without saying so would
imply it can. `observe` renders OBS rather than ON — its verdict is discarded,
so a row claiming enforcement would be claiming something it deliberately is not
doing. An unreadable manifest drops the section rather than breaking the listing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015BDLTPtbQvUE62eCfQrUbf
The daemon half of the AgentEye rename. Both words were product vocabulary
nobody outside the codebase used: a customer reads "deployment 7" and
"version 3" without a glossary.

Renamed through the wire format and the on-disk manifest, not just the labels —
`DesiredState`, `DesiredPolicy`, `ActiveDeployment` (was `ActiveGeneration`),
`ActivePolicy`, and every path and identifier behind them.

NO COMPATIBILITY ALIAS, deliberately. Nothing is deployed against the old
names, so a `serde(alias)` would be dead code guarding a case that cannot
occur. The consequence is that this must ship with the server change: a daemon
reading `generation` from a server emitting `deployment` parses nothing and
stops reconciling, and `ActiveDeployment` carries `deny_unknown_fields`, so it
fails hard rather than degrading.

Two boundary bugs the first sweep missed, both silent-failure class:

1. Snake_case identifiers — `generation_dir`, `generation_path`,
   `generation_valid` — survived `\bgeneration\b`, because `_` is a word
   character so there is no boundary to match. Four Rust files.
2. `cloudRevision` / `cloudGeneration` are `#[serde(rename)]` keys in the
   hook-activity JSONL that the TypeScript writes and the Rust collector reads.
   The TS side was renamed and the reader was not, which would have dropped
   cloud-policy attribution from every hook decision — no error, just an
   empty column.

Verified: 218 Rust tests, 3217 TypeScript tests, clippy and fmt clean. The one
TypeScript failure (`refuses to half-install when it cannot elevate`) is proven
pre-existing — it fails identically with this entire change stashed, because
this machine has passwordless sudo and the test asserts `canElevate()` is false.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015BDLTPtbQvUE62eCfQrUbf
…ng an unhandled rejection

CI went red on a run where every test passed: `190 files passed, 3218 tests
passed`, then `Errors 2 errors` and exit 1.

`readline.emitKeypressEvents()` calls `listenerCount` on the stream it is given,
and the mock in the new promptText tests did not have it. The call threw, and
because the test deliberately does not await the prompt — `void promptText(...)`,
since the prompt only resolves on a keypress — the throw surfaced as an
UNHANDLED REJECTION rather than a failing assertion.

That is why it passed locally and failed in CI: an unhandled rejection is a
warning in a terminal and a job failure in the pipeline. The tests were testing
the right thing; the mock was incomplete.

Adds `listenerCount`, `once`, `off`, `emit` and `addListener` — the surface
`emitKeypressEvents` actually touches.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015BDLTPtbQvUE62eCfQrUbf
SiddarthAA and others added 4 commits August 7, 2026 21:14
Every source watched exactly the place its own installer puts it —
~/.claude/projects, ~/.hermes/state.db. That is right for one machine and
wrong for every other arrangement: a second profile, a mounted team share, a
container's home beside the host's, an agent an operator relocated. Those
hold real sessions and nothing collected them.

`failproofai harness add-path <harness> [<label>=]<path>` (plus list /
remove-path) writes [collector.sources.<harness>] extra_paths in config.toml;
FAILPROOFAI_<HARNESS>_EXTRA_PATHS overrides it for containers. All twelve
harnesses — thirteen sources, since `claude` carries its subagent
transcripts, which share a root and would otherwise be silently dropped under
an added path.

Grammar, folder-name label fallback and <label>-<agentId> namespacing match
AgentEye's collector (--openclaw-extra-path, --hermes-extra-path) rather than
inventing a second answer to what an extra path is.

Three things are structural rather than incidental:

* Each labelled path is its OWN task. `roots` is walked by one task with one
  Params, and the label lives in Params — so a second root added there would
  be captured under the same agent id as the default. Two copies of one
  project derive the SAME id (from the cwd inside the transcript, identical in
  both), which is precisely what the label exists to separate.
* Each gets its own cursor directory. The store writes its whole map
  atomically, so two instances sharing one clobber each other's watermark and
  both re-read from zero after every restart.
* Each gets its own health key, or they overwrite each other's record and
  root_present alternates — destroying the "absent root versus idle source"
  distinction that file exists to draw. Hermes hit exactly this with its
  per-profile databases; that is the shape every source now takes.

The label is applied in SpoolWriter::push — the one point every event from
every source passes through, where machine_id and user are already stamped
for the same reason — and NOT in either engine. A SQLite format is handed
params.agent_id only as a fallback and derives the real id from the row:
devin::agent_id returns devin-<project> and never consults the fallback for a
session that has a working directory, which is all of them. Prefixing there
would namespace only the sessions that failed to derive an id.

A path overlapping one already captured is refused at startup rather than
collected twice under two ids, and an unrecognised [collector.sources.*]
table is reported instead of silently capturing nothing.

A machine with nothing configured is unchanged, byte for byte, including its
config.toml.

Tests: 8 engine tests through the real filetail and sqlitepoll loops
(multi-path steady state, backfill from a newly added path, cursor resume,
and a default-only regression); 16 grammar unit tests; a cross-language
contract test parsing the exact config.toml bytes the TypeScript CLI emits,
because that seam has nothing else holding it together; and a test that reads
HARNESS_KEYS out of main.rs so the two hand-maintained copies of that list
cannot drift.

Found and fixed while testing: `harness list` returned "No extra capture
paths configured" before it checked for unknown tables, so the typo warning
was unreachable in exactly the case it exists for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CCSFM55BcUEAHECabpeSr4
Layout 1 kept it at ~/.failproofai/policies-config.json; layout 2 keeps it
at policies/local-policies/policies-config.json — and BOTH were on the reset
list. So moving between layouts discarded every builtin the user had
enabled, every explicit customPoliciesPaths entry and every policyParams
value.

The reset runs unattended from checkLayoutForCli on the first command after
an upgrade, and the machine still read as configured afterwards
(isConfigured() is a union that sees the agent CLIs' untouched settings
files) — so the wizard never re-asked and hooks kept firing against a policy
set that had quietly become the default one. Same silent enforcement gap
migrateConventionPolicies() and migrateHookActivity() already close, by a
third route.

Deliberately narrow, and inside the standing wipe-and-re-setup decision:
everything derived (cursors, spool, health, audit cache) still goes and is
rebuilt. What moves is only what a person typed and nothing regenerates —
the same test migrateHookActivity() applies to the decision log.

An ALLOWLIST rather than a copy. Layout 1's file also carried a `collector`
block in camelCase; layout 2 moved those settings to [collector] in
config.toml in snake_case, where fpai-collect's Settings deserializes them.
Carrying it would put a key into the new file that nothing reads — looking
preserved and behaving absent, which is the exact bug that Settings' own doc
comment records.

Two phases, because the source AND the destination's parent are both on the
reset list: read before the deletions, write after. Writing first would have
the reset delete the carry moments after it happened — what the note on
hookActivityDir() in resettablePaths() records happening once already.

A reset FROM the current layout no longer clears local-policies/. That is
not a layout migration, and there it would discard a current, valid
selection, including one just carried. The single production caller always
passes a DETECTED stale layout, so this only changes the forced same-layout
case.

An existing layout-2 config is never overwritten by the layout-1 file beside
it — a stale file winning would undo configuration rather than preserve it.
The reset still clears a stale layout-2 config on a layout migration, which
the pre-existing "still clears the machine-owned children of policies/" test
pins and which this change leaves intact.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CCSFM55BcUEAHECabpeSr4
`harness add-path` told the user to run
`sudo systemctl restart failproofaid@$USER`. That was wrong twice over.

The daemon's collector manager already re-reads config.toml on an interval
and cycles the collector whenever the resolved CollectorConfig differs from
the one the live generation was built from — the same path that picks up
`--connect`, a stream being switched off and a verbosity change. `sources`
lives inside `Settings`, which is part of that compared value, so extra
paths ride it for free.

Verified live against a running daemon: adding a path logged "collector
configuration changed; cycling the collector", took the task count 21 -> 23
(claude's extra path adds two, since the main and subagent formats share a
root), and the first transcript written under the new path reached the
server.

So the restart was unnecessary — and asking for root from a command that
writes one file in the user's own home and needs none is the worse half of
the mistake.

Two tests, because nothing registers `sources` with the reload machinery
explicitly and moving it anywhere resolved later breaks the promise
SILENTLY: the CLI keeps reporting success, the config keeps parsing, and the
daemon just never captures the path until an unrelated restart.

  - collector_config_change_cycles_the_collector (unit) pins that adding an
    extra path changes the compared value at all.
  - an_extra_capture_path_added_by_hand_is_registered_without_a_restart
    (e2e) drives the REAL binary against a hand-edited config, alongside the
    credential-rotation cases already there and for the same stated reason —
    the property is "an edit somebody else made is noticed", and a fleet
    tool or a sed is a legitimate way to make it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CCSFM55BcUEAHECabpeSr4
Nine findings; eight fixed, one already handled during the #667 merge.

**Back navigation was broken, and one half of it hung the wizard.** `BACK` is
a symbol the shared key handler injects on ←, so it is not a value of any
prompt's own result type — and both callers mishandled it differently.
`multiSelect`'s summary calls `values.includes(...)`, which throws TypeError
on a symbol, and it throws INSIDE `finish` before `resolve()`: pressing ←
never settled the promise and the wizard stopped responding to input at all.
`selectOne` fell through to `String(value)` and rendered the literal text
`Symbol(failproofai.back)` as the user's answer. Handled centrally in
`collapse()` rather than in each `summaryFor`, so a prompt never has to know
about a symbol it did not declare. Verified non-vacuous: reverting the fix
reproduces `TypeError: values.includes is not a function`.

**Pre-rename cloud-policy state on disk became unreadable.** The
generation→deployment / revision→version rename left `ActiveDeployment` and
`ActivePolicy` carrying `deny_unknown_fields`, so an upgraded daemon failed
to parse its OWN `active.json` on three counts at once — `generation`
unrecognised, `deployment` missing, and the same again per policy. The
machine would silently lose the deployment it was enforcing until a poll
succeeded, which on a fail-closed machine is the gap this subsystem exists to
close. `serde(alias)` on all four fields, covering `desired-state.json` too
since it is both received and persisted. The regression test uses a
byte-exact `active.json` captured from a live pre-rename daemon; without the
aliases two of its three cases fail.

**`failproofai flush` reported success on unsupported platforms.** It skipped
the daemon check, wrote `flush-request.json` and exited 0 — but nothing on
Windows ever reads it, so the only symptom is data that never arrives. Now
refuses before writing, matching what `failproofai config` on this same
branch already does rather than the two commands disagreeing about whether
the machine has a daemon.

**The rename reached mocked git output.** Four `builtin-policies` mocks threw
`unknown version`, which no git ever produces; restored to the real
`ambiguous argument: unknown revision or path not in the working tree`.
Production classifies on none of it, hence minor — but the string was
invented by a sweep.

Also: `#PR` placeholders resolved in the 1.0.0-beta.13 section only (the 72 in
older released sections are left alone), with the two entries from #667
attributed there rather than to #663; `--wait` documented as waiting until the
spool drains OR its timeout expires, which is what `runFlushCommand` does; the
activity detail says `version` rather than `rev`, matching the rename.

Tests: back navigation for both prompt kinds plus an inert-← case, and
validation-error row widths for `promptText` — measured per WRITE and per
newline, since each write is one redraw and only the error path adds a row.
The duplicate `### Features` heading was already collapsed while merging #667.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CCSFM55BcUEAHECabpeSr4
@SiddarthAA
SiddarthAA marked this pull request as ready for review August 7, 2026 17:16
The v1 desired-state payload named its fields `generation` and `revision`.
After the rename it carried neither, at the same version number — same
endpoint, same version, different shape, which is the one thing a schema
version exists to prevent. AgentEye#559 now emits 2; this accepts both.

1 is accepted ONLY for files already on disk. A machine that ran an earlier
beta has a `desired-state.json` and an `active.json` written at version 1,
and both structs carry `deny_unknown_fields` — so refusing that version would
leave the daemon unable to read its own persisted state, silently not
enforcing cloud policy until a poll re-materialised everything.

That same asymmetry decides where the field aliases live. They stay on the
persisted `ActiveDeployment`/`ActivePolicy`, whose bytes may have been written
by an older daemon than the one now reading them. They are REMOVED from the
wire `DesiredState`/`DesiredPolicy`, because no server can emit the old
spelling — an alias there is dead code, and a silently-tolerated stale field
is exactly how two sides drift back apart. A test pins each half, including
that the wire now REFUSES the old spelling rather than quietly taking it.

The TypeScript hook reader had the same constant and accepted only 1. That is
the worst-shaped version of this bug: the daemon reconciles, writes a correct
`active.json`, reports "deployment 1 active" — and the hook path alone refuses
it, so cloud policy stops being enforced while every other signal says the
machine is healthy. Reproduced exactly that way while syncing against a live
#559 server, before this fix.

Verified end to end against the real stack: publish a policy, deploy it, the
daemon pulls schemaVersion 2, verifies every digest, activates, and a matching
tool call is denied with that policy's reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CCSFM55BcUEAHECabpeSr4
@SiddarthAA SiddarthAA changed the title chore(wizard): name the tool in the setup intro instead of calling it a safety net [beta.13] Extra capture paths per agent CLI, the deployment/version rename, and two silent enforcement gaps Aug 7, 2026
Found by an adversarial audit of the rename, and confirmed by running the real
reader against the byte-exact fixture the Rust test uses.

`ACCEPTED_ACTIVE_SCHEMA_VERSIONS = [1, 2]` says 1 is accepted "for files a
pre-rename beta daemon left behind". The parser then read only `deployment` and
`version` — a genuine v1 file spells them `generation` and `revision` — so the
acceptance was unreachable and every such file threw `active manifest deployment
is invalid`.

The Rust reader of this same file already handles it, via
`#[serde(alias = "generation")]` and `#[serde(alias = "revision")]`. This is the
TypeScript half of that hand-maintained pair, and it was the half missing.

The failure is the exact shape the module header warns about: the daemon
reconciles, writes a correct `active.json`, reports the deployment active — and
the hook path alone refuses it, so cloud-managed policy silently stops being
enforced while builtins keep working and every other signal says healthy.
Recovery needs a successful poll to rewrite the file, so an offline machine or a
revoked credential leaves the gap open indefinitely.

The tests were hiding it. Every `schemaVersion: 1` fixture stamped version 1 on
POST-rename field names — a combination no writer has ever produced — so v1
looked covered while the one migration path it exists for was broken. The
fixture is now honestly labelled 2, and two new tests use the real v1 spelling.
Red-proven: without the fallback the first one fails with that exact error.

Also from the same audit:

- `crates/CLOUD_POLICIES.md`, the daemon's own contract document, was never
  renamed. Its desired-state example is a payload the daemon now rejects.
- `__tests__/e2e/layout/cloud-pairing.sh` — the live cross-repo harness — still
  spoke the pre-rename wire in nine places: it read `revision` off the publish
  response, sent `revision` in two deploy bodies, read `generation` off the
  deploy response, and asserted the ingested event carried `cloud_generation`.
  It would have failed against the renamed server, and its assertions proved
  nothing about the new contract.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015BDLTPtbQvUE62eCfQrUbf
@chhhee10
chhhee10 force-pushed the chore/wizard-copy-no-safety-net branch from c8fc648 to 39671df Compare August 7, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants