fix(hook): sync session hooks with actual feature state, cut per-turn bloat - #277
Conversation
…runtime shim Drops the preinstall script (installArchSpecificPackage.js) that manually shelled out to npm install for the detected platform package. Replaces it with: - main package declares all 5 platform packages as optionalDependencies (npm/yarn/pnpm resolve+fetch+verify only the one matching this machine's os/cpu natively -- the same mechanism already used for every other dependency, no reimplementation needed) - bin/agentflare.js is a small shim that at *invocation* time (not install time) resolves whichever platform package actually got installed and execs it, forwarding argv/stdio/exit code No script runs at npm install time anymore. Validated end-to-end with real npm-packed tarballs (not just file: links, which have unrelated relative-path quirks): install produces no lifecycle-script output, the shim resolves and execs the platform binary, forwards arguments, and propagates the exit code correctly. Also caught and fixed a real trailing-comma bug in the optionalDependencies JSON-generation while validating (bash %-pattern stripping needs the backslash unescaped inside a single-quoted pattern, confirmed by direct testing). Item #225. Holding this on its branch, uncommitted to master, until there's a batch of other changes ready to release together.
… bloat session_start's closing status line and prompt_submit's per-turn identity bits claimed lean-ctx/Exa/MCP tooling was active unconditionally, even when the matching component wasn't installed or registered. Gate both on each component's own check() instead. prompt_submit also repeated four full sentences into every turn's context forever; now sent once per session (first turn only) in the terse @tag: syntax already used by the rule files, instead of every turn. session_start's section headings get brand-matching color (same codes as banner::colorize), gated on NO_COLOR.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe release script now publishes a runtime platform shim with dynamically generated optional dependencies. Hook output adds colorized headings, component-aware activity tags, and first-turn session reminders with setup and coaching tokens. ChangesNPM release packaging
Hook output and reminders
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseScript as scripts/release-npm.sh
participant NpmRegistry as NPM registry
participant WrapperShim as bin/agentflare.js
ReleaseScript->>NpmRegistry: Publish successful platform packages
ReleaseScript->>NpmRegistry: Publish wrapper with optional dependencies
WrapperShim->>WrapperShim: Resolve installed platform package
WrapperShim->>WrapperShim: Launch resolved platform binary
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/hook.rs (2)
476-490: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant
get_components(agent)call on the first-turn path.On the first turn,
get_components(agent)runs once insideidentity_bits(line 357) and again here for the pending-consent check (line 484-486). Since each component'scheck()can do file/JSON I/O (reading.claude.json,mise_bin(), etc.), this doubles that work on every first-turn prompt. Compute the components list once and reuse it for both the identity gating and the pending check.♻️ Proposed refactor to compute components once
- let mut bits = if first_turn { - identity_bits(agent) - } else { - vec![] - }; + let components = get_components(agent); + let mut bits = if first_turn { + identity_bits_for(&components) + } else { + vec![] + }; if let Some(block) = crate::mentions::expand(prompt) { bits.push(block); } - let pending = get_components(agent) - .iter() + let pending = components + .iter() .any(|c| c.needs_consent && !(c.check)());(
identity_bitswould take&[Component]instead of callingget_componentsinternally.)🤖 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/hook.rs` around lines 476 - 490, Refactor the first-turn flow around identity_bits and get_components so the components list is computed once and reused for both identity generation and the pending-consent check. Change identity_bits to accept a component slice rather than calling get_components internally, while preserving its existing gating behavior and using the same components for pending detection.
149-176: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueTag gating correctly reflects each component's
check().The dynamic tag line replacing the old static "AGENTFLARE ACTIVE" claim is a solid fix — it can no longer overclaim lean-ctx/Exa/MCP availability. Given
identity_bitsdocuments that the "rules" component's on-disk content mirrors this same Exa/clean-commit guidance, theid="rules"→ "Exa search, clean commits" mapping looks intentional rather than a naming mismatch.One loose end: the
agentflare-mcpcomponent'sdescribetext (insrc/components.rs) advertises only "skill_search/skill_load" for consent purposes, but this tag additionally credits "+ memory_* MCP tools" to the samecheck(). If memory tools are gated by the same registration, the setup/consent text arguably should mention them too so users aren't surprised later — but this is cosmetic, not a functional bug.🤖 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/hook.rs` around lines 149 - 176, Update the agentflare-mcp component’s describe/consent text in components.rs to mention the memory_* MCP tools alongside skill_search/skill_load, matching the tools credited by the agentflare-mcp tag in the closing status line.
🤖 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 `@src/hook.rs`:
- Around line 476-490: Refactor the first-turn flow around identity_bits and
get_components so the components list is computed once and reused for both
identity generation and the pending-consent check. Change identity_bits to
accept a component slice rather than calling get_components internally, while
preserving its existing gating behavior and using the same components for
pending detection.
- Around line 149-176: Update the agentflare-mcp component’s describe/consent
text in components.rs to mention the memory_* MCP tools alongside
skill_search/skill_load, matching the tools credited by the agentflare-mcp tag
in the closing status line.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 627b037d-9a33-409f-a83b-9cb77180f8f2
📒 Files selected for processing (2)
scripts/release-npm.shsrc/hook.rs
CodeRabbit nitpick: identity_bits() and the pending-consent check each called get_components(), which does file/JSON I/O per component, on every first-turn prompt. Pass the already-computed list into identity_bits instead of recomputing it.
1 participant
Summary
session_start's closing status line andprompt_submit's per-turn identity bits claimed lean-ctx/Exa/MCP tooling was active unconditionally, even when the matching component wasn't installed or registered — gated both on each component's owncheck()insteadprompt_submitrepeated four full sentences into every turn's context forever; now sent once per session (first turn only) using the terse@tag:syntax already used by the rule filessession_start's section headings get brand-matching color (same codes asbanner::colorize), gated onNO_COLORfeat(npm)commit already on this branch (optionalDependencies + runtime shim), unrelated to this hook workTest plan
cargo build --bin agentflarecargo test --bin agentflare hook::(23/23 passing, including new regression test for the sync fix)cargo fmt -- src/hook.rsSummary by CodeRabbit