feat: activate Agent Attention as installed plugin - #3
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds the Agent Attention approval-gate runtime, shared Stop-hook handling, Python-backed Reminders workflows, Bun packaging, plugin integration, and installed-payload validation. ChangesAgent Attention runtime
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Codex
participant BunRuntime
participant PythonRuntime
participant AppleReminders
Codex->>BunRuntime: invoke hook-stop or agent-attention command
BunRuntime->>PythonRuntime: forward command and structured input
PythonRuntime->>AppleReminders: create, inspect, or update approval reminder
AppleReminders-->>PythonRuntime: return reminder state
PythonRuntime-->>BunRuntime: return correlated result
BunRuntime-->>Codex: return command output and exit status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
packages/agent-attention/src/stop.ts (1)
31-37: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
cwdis required but never used.
isAgentAttentionStopInputrejects any payload without a nonemptycwd. The handler never readscwd. A Stop payload that omitscwdtherefore returns the fail-closed block and forces one extra continuation turn. Validate only the fields the decision needs, or document whycwdmust be present.♻️ Proposed narrowing of the validated surface
export function isAgentAttentionStopInput(value: unknown): value is AgentAttentionStopInput { if (!value || typeof value !== "object" || Array.isArray(value)) return false const input = value as Record<string, unknown> - if (typeof input.cwd !== "string" || input.cwd.trim() === "") return false + if (input.cwd !== undefined && typeof input.cwd !== "string") return false if (typeof input.session_id !== "string" || input.session_id.trim() === "") return false return input.stop_hook_active === undefined || typeof input.stop_hook_active === "boolean" }🤖 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 `@packages/agent-attention/src/stop.ts` around lines 31 - 37, Update isAgentAttentionStopInput to stop requiring a nonempty cwd, since the stop decision does not use it. Continue validating session_id and the optional stop_hook_active boolean, while preserving rejection of invalid payload shapes.
🤖 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 `@packages/agent-attention/src/main.ts`:
- Around line 34-47: Update the fabricated missing_python payload in runPython
to include contract_id, schema_version, and run_id consistently with
base_result. Regenerate plugin/runtime/agent-attention-202d4ab2aa5da04a.js from
the corrected packages/agent-attention/src/main.ts; do not edit the generated
bundle manually.
- Around line 78-87: Update main so the JSON write via process.stdout.write
completes before returning the exit status and invoking process.exit; await the
write completion or use an equivalent stream-draining mechanism. Preserve the
existing output formatting and exit-code behavior.
In `@packages/agent-attention/src/stop.ts`:
- Around line 44-45: Update the stop handling around runtime.checkStop and the
submit flow so both use the same canonical Codex identifier, rather than
allowing session_id and the owning thread UUID to diverge. Add an explicit
session_id-to-thread_id mapping or standardize both interfaces on one
identifier, then add an end-to-end test proving the Stop hook checks the
submitted thread and blocks when required.
In `@plugin/runtime/agent-attention.py`:
- Around line 1645-1658: Update the exception handler’s base_result call to keep
changed boolean-valued, matching all other result payloads and the Bun fallback;
move the unknown-state information into a separate appropriate field without
changing the existing error category or retry behavior.
- Around line 606-626: Pass an explicit timeout_seconds value to every relevant
run_json invocation: the remindctl add call in _submit_approval, the remindctl
edit call in update_request_state, and the read_exact_completed_reminder call.
Use the existing timeout configuration or constant, and ensure each call
executes within a finite bound while the per-thread request lock is held.
- Around line 165-181: Update release_request_lock so it never unlinks the lock
path; retain unlocking and descriptor cleanup in the finally block. Ensure
acquire_request_lock continues using the persistent lock-file inode for mutual
exclusion without adding path removal.
- Around line 183-188: Update append_audit so newly created audit files are
opened with private mode 0o600 from creation, rather than relying on os.chmod
after writing. Preserve the existing append behavior and JSON serialization,
while retaining chmod only if needed to enforce permissions on pre-existing
files.
In `@scripts/agent-attention-installed.test.ts`:
- Around line 69-90: The test currently invokes the commands diagnostics path
instead of validating Stop-event safety. Update the test around the installed
bundle invocation to call hook-stop with AGENT_ATTENTION_PYTHON set to
/missing/python3, then assert the fail-closed blocking response and expected
exit behavior for an unavailable Python runtime.
In `@scripts/prove-distribution.ts`:
- Around line 201-203: Update the error thrown by the expanded closure check in
scripts/prove-distribution.ts to use a generic closure-mismatch message or
report the actual mismatched keys, rather than naming capability-tour. Preserve
the existing surfaceKeys diagnostic and validation behavior.
---
Nitpick comments:
In `@packages/agent-attention/src/stop.ts`:
- Around line 31-37: Update isAgentAttentionStopInput to stop requiring a
nonempty cwd, since the stop decision does not use it. Continue validating
session_id and the optional stop_hook_active boolean, while preserving rejection
of invalid payload shapes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 38af4192-f030-45d4-9899-d0f06190c230
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
.claude-plugin/marketplace.jsonexperiments/agent-attention/hooks/agent-attention-codex-stop.tsexperiments/agent-attention/runtime/agent-attention/test_agent_attention.pyexperiments/agent-attention/skill/SKILL.mdpackages/agent-attention/package.jsonpackages/agent-attention/src/main.tspackages/agent-attention/src/stop.tsplugin.config.jsonplugin/.claude-plugin/plugin.jsonplugin/.codex-plugin/plugin.jsonplugin/bin/agent-attentionplugin/hooks/codex/hooks.jsonplugin/runtime/agent-attention-202d4ab2aa5da04a.jsplugin/runtime/agent-attention.pyplugin/runtime/bundle-inventory.jsonplugin/runtime/bundle-inventory.shplugin/runtime/skill-catalog.shplugin/skills/agent-attention/SKILL.mdruntime/skill-catalog.jsonscripts/agent-attention-installed.test.tsscripts/agent-attention-payload.tsscripts/build.test.tsscripts/build.tsscripts/generate.tsscripts/native-capability-surface.test.tsscripts/plugin-config.tsscripts/prove-distribution.tsscripts/prove-harness-install.tsscripts/runtime-custody-config.test.tsscripts/runtime-custody-generation.test.ts
Preserve structured error envelopes and flush hook output before exit. Keep request locking, audit creation, and Reminders calls safe under failure, with installed-path regression proof.
Addressed: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@experiments/agent-attention/runtime/agent-attention/agent-attention.py`:
- Around line 621-622: Pass REMINDCTL_COMMAND_TIMEOUT_SECONDS to the post-create
read_inventory call in _submit_approval so verification is bounded like
remindctl add. Update
experiments/agent-attention/runtime/agent-attention/test_agent_attention.py
lines 917-953 to accept the timeout in the read_inventory mock and assert
post-create verification receives the same value.
- Around line 174-182: Harden append_audit by adding O_NOFOLLOW to the open
flags when supported, then immediately validate the descriptor with fstat:
require a regular file (S_ISREG) with st_nlink equal to 1 before writing. Close
the descriptor and reject the path on validation failure, while preserving the
existing permission handling.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d229d42-61d6-4e41-a163-e0577fe8703b
📒 Files selected for processing (12)
experiments/agent-attention/hooks/agent-attention-codex-stop.test.tsexperiments/agent-attention/runtime/agent-attention/agent-attention.pyexperiments/agent-attention/runtime/agent-attention/test_agent_attention.pypackages/agent-attention/src/main.test.tspackages/agent-attention/src/main.tspackages/agent-attention/src/stop.tsplugin/runtime/agent-attention-5133bb12807899e9.jsplugin/runtime/agent-attention.pyplugin/runtime/bundle-inventory.jsonplugin/runtime/bundle-inventory.shscripts/agent-attention-installed.test.tsscripts/prove-distribution.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/agent-attention/src/main.ts
- scripts/agent-attention-installed.test.ts
- plugin/runtime/bundle-inventory.sh
- plugin/runtime/bundle-inventory.json
- scripts/prove-distribution.ts
Reject redirected audit paths and bound post-create Reminders verification while holding request state.
Summary
Completes the Agent Attention playground arc after #1 added the bounded experiment. Agent Attention is now an installed Codex skill with a custody-launched runtime and an exact-task Stop hook, so fresh tasks can discover and execute it without repository source paths.
Design
Validation
bun run prove:all: passed the commit-bound repository, runtime-custody, native isolated Claude/Codex install, deterministic distribution, and DX proofs.agent-attention, executed its installed launcher, returnedagent-attention.approval-gate, and completed the installed Stop hook.Fresh Codex Desktop activation and the separately approved disposable live Reminders gate remain final acceptance receipts outside this PR's automated claim boundary.
Summary by CodeRabbit
New Features
Bug Fixes
Tests