-
Notifications
You must be signed in to change notification settings - Fork 22
[auto] sync agent CLI harnesses with upstream docs #485
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4b8df5f
2cd5eda
5830f7c
2272768
2eaadd0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,7 +125,7 @@ describe("Claude Code integration", () => { | |
| const entry = claudeCode.buildHookEntry("/usr/bin/failproofai", "PreToolUse", "user"); | ||
| expect(entry.command).toBe('"/usr/bin/failproofai" --hook PreToolUse'); | ||
| expect(entry.command).not.toContain("--cli"); | ||
| expect(entry.timeout).toBe(60_000); | ||
| expect(entry.timeout).toBe(60); | ||
| expect(entry[FAILPROOFAI_HOOK_MARKER]).toBe(true); | ||
| }); | ||
|
|
||
|
|
@@ -191,7 +191,7 @@ describe("OpenAI Codex integration", () => { | |
| expect(codex.scopes).toEqual(["user", "project"]); | ||
| }); | ||
|
|
||
| it("eventTypes are exactly the 6 documented Codex events (snake_case)", () => { | ||
| it("eventTypes are exactly the 10 documented Codex events (snake_case)", () => { | ||
| expect(codex.eventTypes).toEqual(CODEX_HOOK_EVENT_TYPES); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 Nit (non-blocking, test hygiene) — this assertion is tautological. Fix — assert the length so the title's invariant is actually enforced: expect(codex.eventTypes).toHaveLength(10);
expect(codex.eventTypes).toContain("permission_request");Separately, the comment two tests below (line ~208, |
||
| // PR 185 omitted permission_request — make sure we have it. | ||
| expect(codex.eventTypes).toContain("permission_request"); | ||
|
|
@@ -414,7 +414,7 @@ describe("Cursor Agent integration", () => { | |
| const entry = cursor.buildHookEntry("/usr/bin/failproofai", "preToolUse", "user") as Record<string, unknown>; | ||
| expect(entry.type).toBe("command"); | ||
| expect(entry.command).toBe('"/usr/bin/failproofai" --hook preToolUse --cli cursor'); | ||
| expect(entry.timeout).toBe(60_000); | ||
| expect(entry.timeout).toBe(60); | ||
| expect(entry[FAILPROOFAI_HOOK_MARKER]).toBe(true); | ||
| // Cursor entries use the Claude-style `command` field, not Copilot's bash/powershell split. | ||
| expect(entry.bash).toBeUndefined(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -154,7 +154,10 @@ export const claudeCode: Integration = { | |
| return { | ||
| type: "command", | ||
| command, | ||
| timeout: 60_000, | ||
| // Claude reads `timeout` in SECONDS per https://code.claude.com/docs/en/hooks | ||
| // ("Seconds before canceling. Defaults: 600 for command ...; 60 for agent"), | ||
| // NOT milliseconds. 60 = 60s; the old 60000 meant ~16.7h. (#482-class unit fix) | ||
| timeout: 60, | ||
| [FAILPROOFAI_HOOK_MARKER]: true, | ||
| }; | ||
| }, | ||
|
|
@@ -285,8 +288,10 @@ export const codex: Integration = { | |
| : `"${binaryPath}" --hook ${eventType} --cli codex`; | ||
| return { | ||
| type: "command", | ||
| // Codex reads `timeout` in SECONDS (its `timeout_sec` field, default 600) — | ||
| // NOT milliseconds like Claude/Cursor/Gemini. 60 = 60s, not 60000ms (~16.7h). | ||
| // Codex reads `timeout` in SECONDS (the field is literally `timeout`, | ||
| // default 600 per https://developers.openai.com/codex/hooks) — same unit as | ||
| // Claude/Cursor/Copilot, and unlike Gemini, whose `timeout` is in | ||
| // milliseconds (default 60000). 60 = 60s, not 60000ms (~16.7h). | ||
| command, | ||
| timeout: 60, | ||
| [FAILPROOFAI_HOOK_MARKER]: true, | ||
|
|
@@ -591,11 +596,13 @@ export const cursor: Integration = { | |
| scope === "project" | ||
| ? `npx -y failproofai --hook ${eventType} --cli cursor` | ||
| : `"${binaryPath}" --hook ${eventType} --cli cursor`; | ||
| // `timeout` is documented as ms in Cursor's schema (matches Claude). | ||
| // `timeout` is documented in SECONDS in Cursor's schema per | ||
| // https://cursor.com/docs/hooks ("Execution timeout in seconds"; doc examples | ||
| // use 30 and 10), NOT milliseconds. 60 = 60s; the old 60000 meant ~16.7h. | ||
| return { | ||
| type: "command", | ||
| command, | ||
| timeout: 60_000, | ||
| timeout: 60, | ||
|
Comment on lines
+599
to
+605
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Count events in .cursor/hooks.json and check Cursor event types in types.ts
cat .cursor/hooks.json | jq '.hooks | keys | length'
rg -n 'cursor' src/hooks/types.ts -iRepository: FailproofAI/failproofai Length of output: 2224 🏁 Script executed: #!/bin/bash
# View the CURSOR_HOOK_EVENT_TYPES array definition
sed -n '187,196p' src/hooks/types.ts
echo "---"
# View the .cursor/hooks.json to see exactly which events are configured
cat .cursor/hooks.jsonRepository: FailproofAI/failproofai Length of output: 1473 🏁 Script executed: # Search for any mention of "7" in the file or "7-event" in PR context
rg '7\s*event|7-event' src/hooks/integrations.ts -i -A 2 -B 2
rg '7' src/hooks/types.ts -A 1 -B 1Repository: FailproofAI/failproofai Length of output: 1205 Cursor timeout fix is correct; verify event count discrepancy. The timeout correction from 🤖 Prompt for AI Agents |
||
| [FAILPROOFAI_HOOK_MARKER]: true, | ||
| }; | ||
| }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 Minor (test quality): the description now says "exactly the 10 documented Codex events", but the assertion below (
expect(codex.eventTypes).toEqual(CODEX_HOOK_EVENT_TYPES)) is tautological —codex.eventTypesisCODEX_HOOK_EVENT_TYPES(seeintegrations.ts:257), so it passes for any array of any length and never actually pins the count at 10.If the intent is to guard the count, assert it directly:
Not blocking, but worth tightening while you're touching this line — otherwise a future accidental add/remove of a Codex event slips through this test silently.