Skip to content

[failproofai] Remove dead code and stale daemon residue - #666

Open
NiveditJain wants to merge 3 commits into
mainfrom
luv-legion-666
Open

[failproofai] Remove dead code and stale daemon residue#666
NiveditJain wants to merge 3 commits into
mainfrom
luv-legion-666

Conversation

@NiveditJain

@NiveditJain NiveditJain commented Aug 7, 2026

Copy link
Copy Markdown
Member

Summary

  • enable TypeScript unused-local checking and remove compiler-proven dead code
  • remove the obsolete daemon hook wrapper and unused html2canvas dependency
  • correct stale failproofaid documentation and package metadata

Verification

  • bunx tsc --noEmit
  • bun run lint
  • bun run test:run
  • bun run test:e2e
  • cargo fmt --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace
  • knip dependency scan

Notes

The exact Docker CLI smoke was blocked by the repository's dogfood policy forbidding failproofai invocation. The package-import fallback was stopped after npm install stalled.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This maintenance change enables noUnusedLocals, removes unused bindings, replaces tryDaemonHook with failure-preserving usage, updates daemon documentation and configuration, removes obsolete hook files, and replaces html2canvas with html-to-image.

Changes

Cleanup and daemon maintenance

Layer / File(s) Summary
Unused-local cleanup
tsconfig.json, __tests__/*, app/*, scripts/translate-docs/*, src/audit/*, src/hooks/*
Enables noUnusedLocals and removes unused imports, locals, parameters, and lint suppressions.
Daemon failure-preserving API migration
src/hooks/daemon-client.ts, __tests__/hooks/daemon-client.test.ts
Removes tryDaemonHook. Tests use attemptDaemonHook through a local adapter that preserves null-result assertions.
Daemon packaging and protocol documentation
crates/PROTOCOL.md, crates/failproofaid/*, crates/fpai-ipc/*, src/hooks/daemon-service.ts, bin/failproofaid-shim.mjs, src/hooks/collector-config.ts
Updates daemon behavior, protocol mismatch handling, service scope, installation fallback, worker paths, security notes, and configuration documentation.
Dependency and release cleanup
package.json, CHANGELOG.md, .claude/settings.json, .codex/hooks.json
Replaces html2canvas, records the cleanup, and removes obsolete Claude and Codex hook configuration files.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: hermes-exosphere

Poem

A rabbit trims imports in a row,
Old daemon wrappers hop and go.
Clear errors guide each hook anew,
New package paths come into view.
“No unused locals!” the bunny sings,
While clean docs flutter on testing wings.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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 the main changes: removal of dead code and stale daemon-related content.
Description check ✅ Passed The description explains the changes and verification results, but it does not use the required Type of Change and Checklist sections.

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.

🧹 Nitpick comments (2)
tsconfig.json (1)

12-12: 📐 Maintainability & Code Quality | 🔵 Trivial

Complete the required validation before pushing.

The PR objectives report blocked Docker smoke testing and an npm install fallback. Use Bun for local package installation and scripts. Complete the required unit, local smoke, Docker clean-install, and E2E checks, or obtain an approved exception before pushing.

🤖 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 `@tsconfig.json` at line 12, Before pushing, use Bun for dependency
installation and script execution, then complete the required unit, local smoke,
Docker clean-install, and E2E checks. If Docker smoke testing or any required
validation remains blocked, obtain an approved exception and document it before
pushing.

Source: Coding guidelines

__tests__/hooks/daemon-client.test.ts (1)

78-83: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add direct tests for DaemonAttempt failure categories.

daemonResult converts both "unreachable" and "protocol-mismatch" to null. The tests at Line 147 and Lines 210-217 therefore no longer verify the structured result introduced by attemptDaemonHook. Add direct assertions for both failure categories. Keep daemonResult for tests that intentionally preserve the legacy null behavior.

As per coding guidelines, always add unit tests for new behaviour.

Also applies to: 147-147, 210-217

🤖 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/daemon-client.test.ts` around lines 78 - 83, Keep
daemonResult for legacy null-behavior tests, but add direct attemptDaemonHook
assertions in the affected tests to verify structured failures: assert the
unreachable case returns ok false with category "unreachable", and the protocol
mismatch case returns ok false with category "protocol-mismatch".

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@__tests__/hooks/daemon-client.test.ts`:
- Around line 78-83: Keep daemonResult for legacy null-behavior tests, but add
direct attemptDaemonHook assertions in the affected tests to verify structured
failures: assert the unreachable case returns ok false with category
"unreachable", and the protocol mismatch case returns ok false with category
"protocol-mismatch".

In `@tsconfig.json`:
- Line 12: Before pushing, use Bun for dependency installation and script
execution, then complete the required unit, local smoke, Docker clean-install,
and E2E checks. If Docker smoke testing or any required validation remains
blocked, obtain an approved exception and document it before pushing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 93758f0f-0796-458a-b96a-7a9a982c5473

📥 Commits

Reviewing files that changed from the base of the PR and between 822022f and 2d02ad5.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (64)
  • CHANGELOG.md
  • __tests__/actions/update-scheduled-audit.test.ts
  • __tests__/audit/cache.test.ts
  • __tests__/components/button.test.tsx
  • __tests__/components/date-picker-input.test.tsx
  • __tests__/e2e/hooks/codex-integration.e2e.test.ts
  • __tests__/hooks/cloud-enrollment-cli.test.ts
  • __tests__/hooks/collector-config.test.ts
  • __tests__/hooks/configure-wizard.test.ts
  • __tests__/hooks/daemon-client.test.ts
  • __tests__/hooks/fp-home.test.ts
  • __tests__/hooks/new-telemetry.test.ts
  • __tests__/lib/codex-sessions.test.ts
  • __tests__/lib/pi-sessions.test.ts
  • __tests__/lib/telemetry-id.test.ts
  • __tests__/lib/telemetry.test.ts
  • __tests__/scripts/translate-docs/cache.test.ts
  • app/actions/get-hooks-config.ts
  • app/audit/_components/audit-dashboard.tsx
  • app/audit/_components/audit-progress-strip.tsx
  • app/audit/_components/come-back-better-section.tsx
  • app/audit/_components/empty-state.tsx
  • app/audit/_components/how-to-improve-section.tsx
  • app/audit/_components/quirks-section.tsx
  • app/audit/_components/report-footer.tsx
  • app/audit/_components/run-progress.tsx
  • app/audit/_components/strengths-section.tsx
  • app/components/pause-notices.tsx
  • app/components/raw-log-viewer.tsx
  • app/components/toast.tsx
  • app/policies/hooks-client.tsx
  • app/settings/settings-client.tsx
  • bin/failproofaid-shim.mjs
  • crates/PROTOCOL.md
  • crates/failproofaid/Cargo.toml
  • crates/failproofaid/src/worker.rs
  • crates/failproofaid/tests/daemon_e2e.rs
  • crates/fpai-ipc/src/envelope.rs
  • crates/fpai-ipc/src/peer.rs
  • lib/auth/auth-store.ts
  • package.json
  • scripts/translate-docs/cache.ts
  • scripts/translate-docs/cli.ts
  • scripts/translate-docs/mintlify-nav.ts
  • src/audit/cache.ts
  • src/audit/dashboard-cache.ts
  • src/audit/report.ts
  • src/hooks/builtin-policies.ts
  • src/hooks/cloud-enrollment-cli.ts
  • src/hooks/cloud-enrollment.ts
  • src/hooks/cloud-managed-policies.ts
  • src/hooks/collector-config.ts
  • src/hooks/configure-wizard.ts
  • src/hooks/custom-hooks-loader.ts
  • src/hooks/daemon-client.ts
  • src/hooks/daemon-download.ts
  • src/hooks/daemon-service.ts
  • src/hooks/hook-logger.ts
  • src/hooks/integrations.ts
  • src/hooks/loader-utils.ts
  • src/hooks/manager.ts
  • src/hooks/onboarding-lock.ts
  • src/hooks/session-pause.ts
  • tsconfig.json
💤 Files with no reviewable changes (22)
  • lib/auth/auth-store.ts
  • src/hooks/integrations.ts
  • package.json
  • tests/lib/telemetry.test.ts
  • tests/e2e/hooks/codex-integration.e2e.test.ts
  • tests/lib/telemetry-id.test.ts
  • src/audit/report.ts
  • app/audit/_components/strengths-section.tsx
  • app/audit/_components/quirks-section.tsx
  • src/audit/cache.ts
  • src/hooks/session-pause.ts
  • src/hooks/custom-hooks-loader.ts
  • app/audit/_components/report-footer.tsx
  • src/hooks/configure-wizard.ts
  • src/hooks/hook-logger.ts
  • app/actions/get-hooks-config.ts
  • src/hooks/daemon-download.ts
  • src/hooks/cloud-managed-policies.ts
  • app/audit/_components/empty-state.tsx
  • src/hooks/cloud-enrollment-cli.ts
  • tests/hooks/cloud-enrollment-cli.test.ts
  • src/audit/dashboard-cache.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant