Skip to content

fix(hooks): deterministic workspace-scoped Stop gate#4

Open
sidneyswift wants to merge 1 commit into
mainfrom
fix/deterministic-stop-gate
Open

fix(hooks): deterministic workspace-scoped Stop gate#4
sidneyswift wants to merge 1 commit into
mainfrom
fix/deterministic-stop-gate

Conversation

@sidneyswift

@sidneyswift sidneyswift commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replaces the always-on LLM Stop-hook reviewer (fired in every workspace, every stop) with a deterministic Python command hook
  • Gate stays silent unless the project is a catalog deal workspace (deals/ present) AND a completion claim (Gate A) or an end-to-end /recoup-catalog-deal run (Gate B) is in play
  • Only then runs the shipped validators (run-deal-checks.py / validate-dashboard.py) and blocks on failure; fails open on any error

Test plan

  • Stop in a non-deal workspace → hook silent
  • Claim "package is ready" in a deal workspace with failing validators → blocked
  • End-to-end run without validated DASHBOARD.html → blocked

Made with Cursor


Summary by cubic

Replaced the always-on Stop reviewer with a deterministic Python Stop hook that only triggers in deal workspaces when a completion claim or an end-to-end /recoup-catalog-deal run is active. This removes noisy, out-of-context blocks while keeping real gating strict.

  • Bug Fixes
    • Scoped to workspaces with a deals/ directory; otherwise stays silent.
    • Gate A: on a completion claim or memo finalization, runs scripts/run-deal-checks.py and blocks on failure.
    • Gate B: during end-to-end runs, requires DASHBOARD.html to exist and pass scripts/validate-dashboard.py; blocks if not.
    • Fails open on any error to avoid false blocks; switches Stop hook in hooks/hooks.json from an LLM prompt to the command python3 ${CLAUDE_PLUGIN_ROOT}/hooks/deal_completion_gate.py.

Written for commit 4290181. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Added a workspace-aware stop check for deal sessions that quietly allows normal progress unless specific completion or demo-run conditions are detected.
  • Bug Fixes

    • Improved safeguards so deal workspaces are only blocked when required checks fail, reducing false alarms and unnecessary interruptions.
    • Better handles incomplete or malformed session history without affecting normal use.

…rkspace-scoped gate

The Stop hook fired an LLM completion-gate review in every workspace on every
stop. Replace it with a Python command hook that stays silent unless the
project is a catalog deal workspace (has deals/) AND a completion claim or an
end-to-end /recoup-catalog-deal run is in play — only then runs the shipped
validators (run-deal-checks.py / validate-dashboard.py) and blocks. Fails open
on errors so a false block can't strand ordinary sessions.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a new Python stop-hook script (deal_completion_gate.py) implementing deterministic gating logic for catalog deal workspaces, replacing an LLM prompt-based reviewer. The hook parses transcripts, detects completion claims or end-to-end triggers, runs validator scripts, and blocks or stays silent accordingly. hooks.json is updated to invoke this script.

Changes

Deal Completion Gate

Layer / File(s) Summary
Module header, imports, and regex constants
hooks/deal_completion_gate.py
Documents the two gating rules and fail-open behavior; defines regex constants for completion claims, memo phrases, and end-to-end triggers.
Transcript parsing utilities
hooks/deal_completion_gate.py
Adds tolerant JSONL event iteration, text extraction from message content, and retrieval of the last assistant and user messages.
Deal resolution and validator execution
hooks/deal_completion_gate.py
Selects the newest deal directory by mtime, runs validator scripts via subprocess with a 90s timeout and fail-open error handling, and defines the block() function to emit a JSON decision.
Main control flow and entrypoint
hooks/deal_completion_gate.py
Reads stdin JSON, applies Gate A (completion claim triggers run-deal-checks.py) and Gate B (end-to-end trigger checks DASHBOARD.html and validate-dashboard.py), blocking or silently exiting; adds __main__ entrypoint.
Wire stop hook to new script
hooks/hooks.json
Updates the description and replaces the prompt-based Stop hook with a command hook running deal_completion_gate.py with a 20s timeout.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ClaudeStopHook as Claude Stop Event
  participant GateScript as deal_completion_gate.py
  participant Transcript as Transcript JSONL
  participant Validator as run-deal-checks.py / validate-dashboard.py

  ClaudeStopHook->>GateScript: stdin JSON (cwd, transcript_path)
  GateScript->>GateScript: check cwd contains deals/
  GateScript->>Transcript: read last assistant/user messages
  Transcript-->>GateScript: message text
  alt Gate A: completion claim detected
    GateScript->>Validator: run run-deal-checks.py
    Validator-->>GateScript: pass/fail
    GateScript->>ClaudeStopHook: block (if fail) or silent (if pass)
  else Gate B: end-to-end trigger detected
    GateScript->>GateScript: check DASHBOARD.html exists
    GateScript->>Validator: run validate-dashboard.py
    Validator-->>GateScript: pass/fail
    GateScript->>ClaudeStopHook: block (if missing/fail) or silent (if pass)
  else no gate matched
    GateScript->>ClaudeStopHook: silent exit
  end
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: replacing the Stop hook with a deterministic, workspace-scoped gate.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/deterministic-stop-gate

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: 3

🧹 Nitpick comments (1)
hooks/deal_completion_gate.py (1)

111-117: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Trailing-JSON status check never fires on the shipped validators.

Both run-deal-checks.py and validate-dashboard.py print pretty-printed JSON (json.dumps(..., indent=2)), so proc.stdout.strip().splitlines()[-1] is just "}", json.loads raises, and the branch falls through to return True. Behavior is currently correct only because those validators exit non-zero on failure (handled at Line 109), but the JSON status safety net documented in the module docstring is effectively dead — a validator that exits 0 while reporting status != "ok" would not be caught. Parse the full stdout instead.

♻️ Proposed fix
-    try:  # honor a trailing JSON {"status": "..."} line if the validator prints one
-        result = json.loads(proc.stdout.strip().splitlines()[-1])
+    try:  # honor a JSON {"status": "..."} payload if the validator prints one
+        result = json.loads(proc.stdout)
         if isinstance(result, dict) and "status" in result:
             return result["status"] == "ok"
     except Exception:
         pass
     return True
🤖 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 `@hooks/deal_completion_gate.py` around lines 111 - 117, The trailing-JSON
status check in deal_completion_gate.py is ineffective because it only inspects
the last stdout line, which fails for pretty-printed JSON from the shipped
validators. Update the status parsing in the gate logic to examine the full
proc.stdout output instead of splitlines()[-1], so a validator that exits 0 but
returns a JSON object with a non-ok status is correctly rejected. Keep the fix
centered in the try block that handles the validator result and preserves the
existing fallback behavior.
🤖 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 `@hooks/deal_completion_gate.py`:
- Around line 66-75: The text_of helper in deal_completion_gate.py can crash
when an event contains a null message because ev.get("message", {}) may return
None and then .get("content", "") fails. Update text_of to safely handle a
missing or null message before accessing content, so main() continues fail-open
and does not emit a traceback on malformed events.
- Around line 98-106: The validator in validator_ok() can run longer than the
Stop hook budget, so it may be killed before the gate reports anything. Update
the subprocess timeout used in validator_ok() to fit within the Stop hook limit,
or adjust the Stop-hook configuration in hooks.json to match the validator’s
budget, keeping the validator and hook timeouts consistent.

In `@hooks/hooks.json`:
- Around line 21-23: The Stop-hook timeout is too short for the runtime budget
used by deal_completion_gate.py, so update the hook configuration in hooks.json
for the command invoking hooks/deal_completion_gate.py to allow more than the
current 20s or reduce the per-validator timeout inside deal_completion_gate.py
so it always fits within the hook budget. Use the hook entry with the "command"
pointing to deal_completion_gate.py and the validator timeout logic in that
script to keep Gate A/B from being cut off before it can report a block.

---

Nitpick comments:
In `@hooks/deal_completion_gate.py`:
- Around line 111-117: The trailing-JSON status check in deal_completion_gate.py
is ineffective because it only inspects the last stdout line, which fails for
pretty-printed JSON from the shipped validators. Update the status parsing in
the gate logic to examine the full proc.stdout output instead of
splitlines()[-1], so a validator that exits 0 but returns a JSON object with a
non-ok status is correctly rejected. Keep the fix centered in the try block that
handles the validator result and preserves the existing fallback behavior.
🪄 Autofix (Beta)

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

Run ID: 699f890d-2b0e-4118-bb86-741eb9b2841e

📥 Commits

Reviewing files that changed from the base of the PR and between db83138 and 4290181.

📒 Files selected for processing (2)
  • hooks/deal_completion_gate.py
  • hooks/hooks.json

Comment on lines +66 to +75
def text_of(ev):
content = ev.get("message", {}).get("content", "")
if isinstance(content, str):
return content
if isinstance(content, list):
return " ".join(
p.get("text", "") for p in content
if isinstance(p, dict) and p.get("type") == "text"
)
return ""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Guard against a null message to preserve fail-open behavior.

ev.get("message", {}) returns None when an event has "message": null (the default only applies to a missing key). The subsequent .get("content", "") then raises AttributeError, which is unguarded here and up through main(), so the hook exits non-zero with a traceback instead of staying silent — violating the fail-open contract in the module docstring.

🛡️ Proposed fix
 def text_of(ev):
-    content = ev.get("message", {}).get("content", "")
+    content = (ev.get("message") or {}).get("content", "")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def text_of(ev):
content = ev.get("message", {}).get("content", "")
if isinstance(content, str):
return content
if isinstance(content, list):
return " ".join(
p.get("text", "") for p in content
if isinstance(p, dict) and p.get("type") == "text"
)
return ""
def text_of(ev):
content = (ev.get("message") or {}).get("content", "")
if isinstance(content, str):
return content
if isinstance(content, list):
return " ".join(
p.get("text", "") for p in content
if isinstance(p, dict) and p.get("type") == "text"
)
return ""
🤖 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 `@hooks/deal_completion_gate.py` around lines 66 - 75, The text_of helper in
deal_completion_gate.py can crash when an event contains a null message because
ev.get("message", {}) may return None and then .get("content", "") fails. Update
text_of to safely handle a missing or null message before accessing content, so
main() continues fail-open and does not emit a traceback on malformed events.

Comment on lines +98 to +106
def validator_ok(project_dir, script_name, deal_rel):
script = os.path.join(os.environ.get("CLAUDE_PLUGIN_ROOT", ""), "scripts", script_name)
if not os.path.isfile(script):
return True # can't validate -> fail open (stay silent)
try:
proc = subprocess.run(
[sys.executable, script, deal_rel],
cwd=project_dir, capture_output=True, text=True, timeout=90,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd -t f 'hooks.json' hooks --exec cat -n {}

Repository: recoupable/recoup-catalogs-plugin

Length of output: 1467


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the timeout path and the surrounding control flow.
sed -n '1,220p' hooks/deal_completion_gate.py | cat -n

# Check the validator scripts for their own timeout behavior.
sed -n '1,240p' hooks/run-deal-checks.py | cat -n
sed -n '1,260p' hooks/validate-dashboard.py | cat -n

Repository: recoupable/recoup-catalogs-plugin

Length of output: 7505


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the validator scripts and inspect their timeouts / expected runtime behavior.
fd -t f 'run-deal-checks.py' . -x sh -c 'echo "FILE: $1"; cat -n "$1"' sh {}
fd -t f 'validate-dashboard.py' . -x sh -c 'echo "FILE: $1"; cat -n "$1"' sh {}

# Show the completion gate around the timeout callsite.
sed -n '90,170p' hooks/deal_completion_gate.py | cat -n

# Search for any other timeout-related settings for these validators.
rg -n "timeout\s*=|run-deal-checks|validate-dashboard|CLAUDE_PLUGIN_ROOT" .

Repository: recoupable/recoup-catalogs-plugin

Length of output: 49509


Align the validator timeout with the 20s Stop hook budget

validator_ok() allows each validator to run for 90s, but hooks/hooks.json gives the Stop hook only 20s. Any run that lands between 20s and 90s can be killed by the harness before this code returns, so Gate B stays silent. Lower the subprocess timeout (or raise the Stop-hook timeout) so the gate can finish inside the hook budget.

🧰 Tools
🪛 ast-grep (0.44.1)

[error] 102-105: Command coming from incoming request
Context: subprocess.run(
[sys.executable, script, deal_rel],
cwd=project_dir, capture_output=True, text=True, timeout=90,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)

🪛 Ruff (0.15.20)

[error] 103-103: subprocess call: check for execution of untrusted input

(S603)

🤖 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 `@hooks/deal_completion_gate.py` around lines 98 - 106, The validator in
validator_ok() can run longer than the Stop hook budget, so it may be killed
before the gate reports anything. Update the subprocess timeout used in
validator_ok() to fit within the Stop hook limit, or adjust the Stop-hook
configuration in hooks.json to match the validator’s budget, keeping the
validator and hook timeouts consistent.

Comment thread hooks/hooks.json
Comment on lines +21 to +23
"type": "command",
"command": "python3 ${CLAUDE_PLUGIN_ROOT}/hooks/deal_completion_gate.py",
"timeout": 20

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the validator subprocess timeout in deal_completion_gate.py vs the hook timeout in hooks.json
fd deal_completion_gate.py --exec rg -nP 'timeout\s*=\s*\d+' {}
fd hooks.json --exec rg -nP '"timeout"\s*:\s*\d+' {}

Repository: recoupable/recoup-catalogs-plugin

Length of output: 305


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the exact hook definition and the gate script around the timeout usage.
printf '\n== hooks/hooks.json ==\n'
nl -ba hooks/hooks.json | sed -n '1,80p'

printf '\n== deal_completion_gate.py ==\n'
file=$(fd -a deal_completion_gate.py . | head -n 1)
printf 'FILE: %s\n' "$file"
nl -ba "$file" | sed -n '1,220p'

Repository: recoupable/recoup-catalogs-plugin

Length of output: 235


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== hooks/hooks.json ==\n'
sed -n '1,120p' hooks/hooks.json | cat -n

printf '\n== deal_completion_gate.py ==\n'
file=$(fd -a deal_completion_gate.py . | head -n 1)
printf 'FILE: %s\n' "$file"
sed -n '1,170p' "$file" | cat -n

Repository: recoupable/recoup-catalogs-plugin

Length of output: 8825


Align the Stop-hook timeout with the validator budget hooks/hooks.json:21-23 caps this hook at 20s, but hooks/deal_completion_gate.py allows each validator to run for up to 90s. Any slow run can kill the hook before it emits a block, so Gate A/B becomes unreliable. Raise the hook timeout above the worst-case validator runtime, or reduce the validator timeout to fit inside 20s.

🤖 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 `@hooks/hooks.json` around lines 21 - 23, The Stop-hook timeout is too short
for the runtime budget used by deal_completion_gate.py, so update the hook
configuration in hooks.json for the command invoking
hooks/deal_completion_gate.py to allow more than the current 20s or reduce the
per-validator timeout inside deal_completion_gate.py so it always fits within
the hook budget. Use the hook entry with the "command" pointing to
deal_completion_gate.py and the validator timeout logic in that script to keep
Gate A/B from being cut off before it can report a block.

@cubic-dev-ai cubic-dev-ai 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.

5 issues found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="hooks/deal_completion_gate.py">

<violation number="1" location="hooks/deal_completion_gate.py:42">
P2: The E2E gate uses an unanchored substring regex over the last user message, which can match prose mentions (e.g., "what does /recoup-catalog-deal do?") and trigger a dashboard validation block even though no end-to-end run was launched. This creates a false-positive block in deal workspaces when a user asks a question before the dashboard exists. Because the hook is meant to stay silent unless a command is genuinely invoked, anchor the pattern so it only matches when the command appears at the start of the message or after whitespace.</violation>

<violation number="2" location="hooks/deal_completion_gate.py:67">
P2: `ev.get("message", {})` returns `None` when an event explicitly contains `"message": null` — the default `{}` only applies when the key is missing entirely. The chained `.get("content", "")` will then raise `AttributeError` on `None`, causing the hook to exit non-zero with a traceback instead of staying silent. This violates the documented fail-open contract. Use `(ev.get("message") or {})` to coalesce both missing and null cases.</violation>

<violation number="3" location="hooks/deal_completion_gate.py:105">
P1: The subprocess timeout here is 90 seconds, but `hooks.json` caps this Stop hook at 20 seconds. If a validator takes longer than 20s, the harness will kill the hook process before it can emit a block decision, silently defeating the gate. Either lower this timeout to fit within the hook budget (e.g., ~15s to leave room for other work) or raise the hook-level timeout in `hooks.json`.</violation>

<violation number="4" location="hooks/deal_completion_gate.py:129">
P1: When both a completion claim (Gate A) and an end-to-end launch (Gate B) are present, the Gate A branch short-circuits to `silent()` after `run-deal-checks.py` passes, so the mandatory `DASHBOARD.html` validation in Gate B is skipped. Since `run-deal-checks.py` does not validate the dashboard, an end-to-end run whose final assistant message also says the package is ready can bypass the dashboard gate entirely. The PR test plan explicitly expects an end-to-end run without a validated dashboard to be blocked. To fix, remove the `silent()` call at the end of Gate A so execution falls through to Gate B when both conditions are met.</violation>

<violation number="5" location="hooks/deal_completion_gate.py:144">
P1: Gate A's `CLAIM_RE` and `MEMO_RE` use substring `re.search()` without safeguards against negated language or incidental mentions, which can produce false blocks — contradicting the stated design goal that "a false block is worse than a missed one".

For example, an assistant message saying "The package is **not** ready for review yet" still triggers `CLAIM_RE` because "ready for review" matches as a substring. Similarly, `MEMO_RE` fires on any mention of memo paths — such as "I still need to create the memos/ic-memo" — regardless of context.

**Suggestion**: Anchor the patterns with word boundaries (`\b`) or negative lookbehinds for common negation words (e.g., "not", "isn't", "never"), and for `MEMO_RE` consider requiring surrounding language that signals finalization rather than a bare path match.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

try:
data = json.load(sys.stdin)
except Exception:
silent()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: When both a completion claim (Gate A) and an end-to-end launch (Gate B) are present, the Gate A branch short-circuits to silent() after run-deal-checks.py passes, so the mandatory DASHBOARD.html validation in Gate B is skipped. Since run-deal-checks.py does not validate the dashboard, an end-to-end run whose final assistant message also says the package is ready can bypass the dashboard gate entirely. The PR test plan explicitly expects an end-to-end run without a validated dashboard to be blocked. To fix, remove the silent() call at the end of Gate A so execution falls through to Gate B when both conditions are met.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/deal_completion_gate.py, line 129:

<comment>When both a completion claim (Gate A) and an end-to-end launch (Gate B) are present, the Gate A branch short-circuits to `silent()` after `run-deal-checks.py` passes, so the mandatory `DASHBOARD.html` validation in Gate B is skipped. Since `run-deal-checks.py` does not validate the dashboard, an end-to-end run whose final assistant message also says the package is ready can bypass the dashboard gate entirely. The PR test plan explicitly expects an end-to-end run without a validated dashboard to be blocked. To fix, remove the `silent()` call at the end of Gate A so execution falls through to Gate B when both conditions are met.</comment>

<file context>
@@ -0,0 +1,171 @@
+    try:
+        data = json.load(sys.stdin)
+    except Exception:
+        silent()
+
+    cwd = data.get("cwd") or ""
</file context>

last_asst, last_user = last_assistant_and_user(data.get("transcript_path") or "")

# Gate A — completion claim in the agent's last message
if CLAIM_RE.search(last_asst) or MEMO_RE.search(last_asst):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Gate A's CLAIM_RE and MEMO_RE use substring re.search() without safeguards against negated language or incidental mentions, which can produce false blocks — contradicting the stated design goal that "a false block is worse than a missed one".

For example, an assistant message saying "The package is not ready for review yet" still triggers CLAIM_RE because "ready for review" matches as a substring. Similarly, MEMO_RE fires on any mention of memo paths — such as "I still need to create the memos/ic-memo" — regardless of context.

Suggestion: Anchor the patterns with word boundaries (\b) or negative lookbehinds for common negation words (e.g., "not", "isn't", "never"), and for MEMO_RE consider requiring surrounding language that signals finalization rather than a bare path match.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/deal_completion_gate.py, line 144:

<comment>Gate A's `CLAIM_RE` and `MEMO_RE` use substring `re.search()` without safeguards against negated language or incidental mentions, which can produce false blocks — contradicting the stated design goal that "a false block is worse than a missed one".

For example, an assistant message saying "The package is **not** ready for review yet" still triggers `CLAIM_RE` because "ready for review" matches as a substring. Similarly, `MEMO_RE` fires on any mention of memo paths — such as "I still need to create the memos/ic-memo" — regardless of context.

**Suggestion**: Anchor the patterns with word boundaries (`\b`) or negative lookbehinds for common negation words (e.g., "not", "isn't", "never"), and for `MEMO_RE` consider requiring surrounding language that signals finalization rather than a bare path match.</comment>

<file context>
@@ -0,0 +1,171 @@
+    last_asst, last_user = last_assistant_and_user(data.get("transcript_path") or "")
+
+    # Gate A — completion claim in the agent's last message
+    if CLAIM_RE.search(last_asst) or MEMO_RE.search(last_asst):
+        if not validator_ok(cwd, "run-deal-checks.py", deal_rel):
+            block(
</file context>

try:
proc = subprocess.run(
[sys.executable, script, deal_rel],
cwd=project_dir, capture_output=True, text=True, timeout=90,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: The subprocess timeout here is 90 seconds, but hooks.json caps this Stop hook at 20 seconds. If a validator takes longer than 20s, the harness will kill the hook process before it can emit a block decision, silently defeating the gate. Either lower this timeout to fit within the hook budget (e.g., ~15s to leave room for other work) or raise the hook-level timeout in hooks.json.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/deal_completion_gate.py, line 105:

<comment>The subprocess timeout here is 90 seconds, but `hooks.json` caps this Stop hook at 20 seconds. If a validator takes longer than 20s, the harness will kill the hook process before it can emit a block decision, silently defeating the gate. Either lower this timeout to fit within the hook budget (e.g., ~15s to leave room for other work) or raise the hook-level timeout in `hooks.json`.</comment>

<file context>
@@ -0,0 +1,171 @@
+    try:
+        proc = subprocess.run(
+            [sys.executable, script, deal_rel],
+            cwd=project_dir, capture_output=True, text=True, timeout=90,
+        )
+    except Exception:
</file context>

r"memos/(ic-memo|seller-cleanup-report|financing-pack|post-close-admin-plan)",
re.IGNORECASE,
)
E2E_RE = re.compile(r"/recoup-catalog-(deal|demo)\b", re.IGNORECASE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The E2E gate uses an unanchored substring regex over the last user message, which can match prose mentions (e.g., "what does /recoup-catalog-deal do?") and trigger a dashboard validation block even though no end-to-end run was launched. This creates a false-positive block in deal workspaces when a user asks a question before the dashboard exists. Because the hook is meant to stay silent unless a command is genuinely invoked, anchor the pattern so it only matches when the command appears at the start of the message or after whitespace.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/deal_completion_gate.py, line 42:

<comment>The E2E gate uses an unanchored substring regex over the last user message, which can match prose mentions (e.g., "what does /recoup-catalog-deal do?") and trigger a dashboard validation block even though no end-to-end run was launched. This creates a false-positive block in deal workspaces when a user asks a question before the dashboard exists. Because the hook is meant to stay silent unless a command is genuinely invoked, anchor the pattern so it only matches when the command appears at the start of the message or after whitespace.</comment>

<file context>
@@ -0,0 +1,171 @@
+    r"memos/(ic-memo|seller-cleanup-report|financing-pack|post-close-admin-plan)",
+    re.IGNORECASE,
+)
+E2E_RE = re.compile(r"/recoup-catalog-(deal|demo)\b", re.IGNORECASE)
+
+
</file context>



def text_of(ev):
content = ev.get("message", {}).get("content", "")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: ev.get("message", {}) returns None when an event explicitly contains "message": null — the default {} only applies when the key is missing entirely. The chained .get("content", "") will then raise AttributeError on None, causing the hook to exit non-zero with a traceback instead of staying silent. This violates the documented fail-open contract. Use (ev.get("message") or {}) to coalesce both missing and null cases.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/deal_completion_gate.py, line 67:

<comment>`ev.get("message", {})` returns `None` when an event explicitly contains `"message": null` — the default `{}` only applies when the key is missing entirely. The chained `.get("content", "")` will then raise `AttributeError` on `None`, causing the hook to exit non-zero with a traceback instead of staying silent. This violates the documented fail-open contract. Use `(ev.get("message") or {})` to coalesce both missing and null cases.</comment>

<file context>
@@ -0,0 +1,171 @@
+
+
+def text_of(ev):
+    content = ev.get("message", {}).get("content", "")
+    if isinstance(content, str):
+        return content
</file context>

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