feat(sdlc): daemon-independent escape grant shim (NEW-2) + deprecate HAPAX_*_OFF#3805
Conversation
…cate HAPAX_*_OFF Closes the audit's central safety correction (NEW-2/INV-4). The EscapeGrant substrate shipped with ZERO non-test callers, so the only working escape from an irreversible-harm gate was the deprecated, silent, unconditional HAPAX_*_OFF off-switch — the exact inversion of "obligatory but never stuck via an authorized scoped grant." - hooks/scripts/escape-grant.sh (new): shared sourced helper. `escape_grant_allows <gate>` scans /var/lib/hapax/coord/grants for a signed EscapeGrant covering the gate and verifies it via `python3 -m shared.governance.coord_capabilities verify-grant` — a PURE FILE READ, never an RPC, so it works with the kernel down. Degrades closed (missing dir/key/python ⇒ no escape). Ledgers each grant. - hooks/scripts/cc-task-gate.sh: every BLOCK now routes through _emit_block, which honors a covering grant before failing closed. HAPAX_CC_TASK_GATE_OFF (formerly silent) is ledgered (cc_task_gate_off_bypass) + deprecation-warned; HAPAX_METHODOLOGY_EMERGENCY records a pending retro-grant obligation (1h deadline) on both bypass paths. - hooks/scripts/no-stale-branches.sh: same helper wired into all 4 block points (destructive x2, worktree-cap, stale-branches) — the exact gate that hard-walls a stale-worktree lane from creating a branch. - scripts/coord-grant-mint (new): operator/lane CLI — writes a signed, scoped, time-boxed grant to /var/lib/hapax/coord/grants/<id>.grant; auto-creates the signing key (0600, O_EXCL) on first mint. - scripts/coord-retro-grant-watch (new): closes the deprecation loop — fulfils a retro-grant obligation when a covering grant lands, ntfy-escalates when the 1h deadline passes with none. (Deploy via a timer — out of this task's scope.) Threat model: single-user (axiom single_user). The HMAC key shares the operator's uid, so this enforces a DELIBERATE, SCOPED, TIME-BOXED, AUDITED escape, not adversarial isolation between the operator's own lanes. TDD: chaos/INV-4 acceptance (a hand-written grant unblocks with NO daemon present — strictly stronger than "daemon killed"), scope/expiry/forged-key negatives, gate-off ledgering, retro-grant obligation + 1h deadline, watcher fulfil/escalate/idempotent, minter key+scope. 26 new tests; full tests/hooks/ + tests/scripts/ + test_coord_capabilities green; ruff + shellcheck clean. AuthorityCase: CASE-FORMAL-GOVERNANCE-001 Task: reform-fix-grant-escape-shim-20260531 Parent spec: coordination-reform-master-design-2026-05-30.md (NEW-2 sec 4.4, INV-4) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds daemon-independent, signed, scoped, time-limited "escape grants" and tooling: a Bash gate checker ( ChangesEscape grant system for HAPAX gates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 323cf5a57c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if _has_covering_grant(grant_dir, key, gate, ts_s, now): | ||
| out.append({**rec, "status": "fulfilled", "resolved_s": int(now)}) |
There was a problem hiding this comment.
When the watcher runs after an obligation is already overdue, this check accepts any covering grant minted after the bypass, even if it was minted after deadline_s. That means an emergency HAPAX_*_OFF use can be marked fulfilled instead of escalated by creating a grant late, defeating the documented 1h retro-grant requirement; include the deadline in the covering-grant predicate (e.g. require issued_at <= deadline_s) before marking the record fulfilled.
Useful? React with 👍 / 👎.
| grant = read_grant_file(path) | ||
| if grant is None or grant.issued_at < min_issued_at: | ||
| continue | ||
| if verify_escape_grant(grant, key=key, now=now, gate=gate): |
There was a problem hiding this comment.
If the watcher is delayed until after an otherwise timely retro grant has expired, passing the current watcher time into verify_escape_grant makes that historical grant look invalid and the obligation escalates anyway. With the default 1h grant TTL, a grant minted within the required hour can expire before this script next runs, so fulfillment depends on watcher cadence rather than whether the signed grant landed on time; verify the signature/scope and evaluate expiry against the obligation window instead of now.
Useful? React with 👍 / 👎.
| printf '{"ts":"%s","kind":"cc_task_gate_off_bypass","role":"%s","tool":"%s"}\n' \ | ||
| "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "$_gate_off_role" "$tool_name" \ | ||
| >>"$_gate_off_ledger" 2>/dev/null || true | ||
| echo "cc-task-gate: HAPAX_CC_TASK_GATE_OFF bypass used — LEDGERED. This switch is DEPRECATED" >&2 | ||
| echo " (incident-only). Prefer a scoped signed escape: scripts/coord-grant-mint --scope $GATE_NAME." >&2 | ||
| exit 0 |
There was a problem hiding this comment.
When HAPAX_CC_TASK_GATE_OFF=1 is used, this path only writes a ledger line and exits, so coord-retro-grant-watch never receives a pending obligation for this deprecated unconditional bypass. In the incident path that still relies on the old off-switch, no one is forced or reminded to mint the scoped signed grant within the 1h window, leaving the original HAPAX_*_OFF escape effectively outside the new retro-grant enforcement loop.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/hooks/test_cc_task_gate.py (1)
1159-1167: 💤 Low valueMinor inefficiency:
json.loadscalled twice per line.Each line is parsed twice—once to check
"kind" in json.loads(line)and again to extract the value. Consider parsing once:♻️ Suggested refactor
def _ledger_kinds(home: Path) -> list[str]: ledger = home / ".cache" / "hapax" / "methodology-emergency-ledger.jsonl" if not ledger.exists(): return [] + kinds = [] + for line in ledger.read_text().splitlines(): + if not line.strip(): + continue + obj = json.loads(line) + if "kind" in obj: + kinds.append(obj["kind"]) + return kinds - return [ - json.loads(line)["kind"] - for line in ledger.read_text().splitlines() - if line.strip() and "kind" in json.loads(line) - ]🤖 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/test_cc_task_gate.py` around lines 1159 - 1167, The _ledger_kinds function currently calls json.loads(line) twice per non-empty line; change it to parse each non-blank line once into a local variable (e.g., obj = json.loads(line)) then check "kind" in obj and append obj["kind"]; you can implement this by replacing the current list comprehension with a simple for loop or a single-expression comprehension that binds the parsed object once so each line is only json.loads()ed one time.scripts/coord-retro-grant-watch (2)
123-166: ⚡ Quick winConsider resilient JSONL parsing for corrupted ledger lines.
Line 153 uses
json.loads(line)in a list comprehension. If the obligations ledger contains a malformed JSON line, the watcher will crash. Adding error handling to skip invalid lines would improve resilience.♻️ Proposed resilient parsing
- records = [json.loads(line) for line in obligations.read_text().splitlines() if line.strip()] + records = [] + for line in obligations.read_text().splitlines(): + if not line.strip(): + continue + try: + records.append(json.loads(line)) + except json.JSONDecodeError: + pass # skip malformed line key = _load_key(key_path)🤖 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 `@scripts/coord-retro-grant-watch` around lines 123 - 166, The list comprehension in main that builds records via json.loads(line) will crash on any malformed JSON; change it to iterate lines from obligations.read_text().splitlines(), attempt json.loads(line) inside a try/except catching json.JSONDecodeError (and ValueError for safety), skip invalid lines and optionally record/log the failure (e.g., include the line index or content) so the watcher continues; update the variable construction around "records" in main and ensure any skipped-line accounting is reflected in the summary passed to process.
71-96: ⚡ Quick winConsider defensive handling for malformed obligation records.
Lines 83-84 use
float()directly on record fields. If the obligations ledger contains malformed entries (non-numericts_sordeadline_s), the watcher will crash withValueError. Since the ledger is operator-controlled and best-effort, this may be acceptable, but adding a try/except to skip malformed records would improve resilience.🛡️ Proposed defensive skip for malformed records
for rec in records: if rec.get("status") != "pending": out.append(rec) continue - gate = str(rec.get("gate", "*")) - ts_s = float(rec.get("ts_s", 0)) - deadline_s = float(rec.get("deadline_s", 0)) + try: + gate = str(rec.get("gate", "*")) + ts_s = float(rec.get("ts_s", 0)) + deadline_s = float(rec.get("deadline_s", 0)) + except (ValueError, TypeError): + out.append(rec) # preserve malformed record as-is + continue if _has_covering_grant(grant_dir, key, gate, ts_s, now):🤖 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 `@scripts/coord-retro-grant-watch` around lines 71 - 96, The process function currently calls float() on rec["ts_s"] and rec["deadline_s"] unguarded, which will raise ValueError/TypeError for malformed ledger entries; wrap the parsing of ts_s and deadline_s in a try/except (catch ValueError and TypeError) inside process (next to where ts_s = float(...) and deadline_s = float(...)) and on parse failure skip or mark the record as malformed: append the original rec (or a copy with status "invalid") to out, increment a new summary["invalid"] counter (initialize it alongside total/pending/fulfilled/escalated), do not change to_notify for that record, and continue the loop so the watcher doesn't crash; keep all other logic (including _has_covering_grant calls) unchanged.
🤖 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 `@scripts/coord-retro-grant-watch`:
- Around line 123-166: The list comprehension in main that builds records via
json.loads(line) will crash on any malformed JSON; change it to iterate lines
from obligations.read_text().splitlines(), attempt json.loads(line) inside a
try/except catching json.JSONDecodeError (and ValueError for safety), skip
invalid lines and optionally record/log the failure (e.g., include the line
index or content) so the watcher continues; update the variable construction
around "records" in main and ensure any skipped-line accounting is reflected in
the summary passed to process.
- Around line 71-96: The process function currently calls float() on rec["ts_s"]
and rec["deadline_s"] unguarded, which will raise ValueError/TypeError for
malformed ledger entries; wrap the parsing of ts_s and deadline_s in a
try/except (catch ValueError and TypeError) inside process (next to where ts_s =
float(...) and deadline_s = float(...)) and on parse failure skip or mark the
record as malformed: append the original rec (or a copy with status "invalid")
to out, increment a new summary["invalid"] counter (initialize it alongside
total/pending/fulfilled/escalated), do not change to_notify for that record, and
continue the loop so the watcher doesn't crash; keep all other logic (including
_has_covering_grant calls) unchanged.
In `@tests/hooks/test_cc_task_gate.py`:
- Around line 1159-1167: The _ledger_kinds function currently calls
json.loads(line) twice per non-empty line; change it to parse each non-blank
line once into a local variable (e.g., obj = json.loads(line)) then check "kind"
in obj and append obj["kind"]; you can implement this by replacing the current
list comprehension with a simple for loop or a single-expression comprehension
that binds the parsed object once so each line is only json.loads()ed one time.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 78c537fa-033e-4702-a7e1-726520300c59
📒 Files selected for processing (9)
hooks/scripts/cc-task-gate.shhooks/scripts/escape-grant.shhooks/scripts/no-stale-branches.shscripts/coord-grant-mintscripts/coord-retro-grant-watchtests/hooks/test_cc_task_gate.pytests/hooks/test_no_stale_branches.pytests/scripts/test_coord_grant_mint.pytests/scripts/test_coord_retro_grant_watch.py
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
…into beta/reform-fix-grant-escape-shim-20260531 # Conflicts: # tests/hooks/test_cc_task_gate.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9df13ba34
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _record_retro_grant_obligation() { | ||
| local role="${1:-unknown}" task="${2:-unknown}" case_id="${3:-unknown}" | ||
| local tool="${4:-unknown}" trigger="${5:-HAPAX_METHODOLOGY_EMERGENCY}" | ||
| local obligations="${HOME}/.cache/hapax/coord-retro-grant-obligations.jsonl" |
There was a problem hiding this comment.
When coord-retro-grant-watch is run with --obligations/HAPAX_COORD_RETRO_OBLIGATIONS (the new watcher explicitly supports this for non-default deployments or timers), the gate still appends emergency obligations to the hard-coded $HOME/.cache/... file here. In that configuration the watcher reads a different ledger, reports zero pending records, and the mandatory retro-grant escalation loop is silently bypassed; use the same env/configured path when recording obligations.
Useful? React with 👍 / 👎.
| "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "$role" "$task_id" "$authority_case" "$tool_name" \ | ||
| >> "$_emergency_ledger" | ||
| echo "cc-task-gate: EMERGENCY BYPASS — logged to $_emergency_ledger" >&2 | ||
| _record_retro_grant_obligation "$role" "$task_id" "$authority_case" "$tool_name" "HAPAX_METHODOLOGY_EMERGENCY" |
There was a problem hiding this comment.
For a normal claimed task with HAPAX_METHODOLOGY_EMERGENCY=1, control exits in the earlier emergency block before role/task/case parsing, so this contextual retro-grant obligation is unreachable and every recorded obligation/notification uses unknown for the task and AuthorityCase. That defeats the new audit trail exactly on the incident bypass path; defer the early exit until after context is available, falling back to the early record only when the infrastructure lookup really fails.
Useful? React with 👍 / 👎.
| if to_notify and not args.dry_run: | ||
| _escalate(to_notify) | ||
|
|
||
| _atomic_write(obligations, updated) |
There was a problem hiding this comment.
When the watcher runs concurrently with a gate invocation that appends a new emergency obligation, this read/process/replace cycle can drop the newly appended line: the watcher reads the old file, the gate appends, then _atomic_write replaces the whole ledger with only the old snapshot. In the timer/manual-run context this leaves a real HAPAX_METHODOLOGY_EMERGENCY use with no pending record to fulfill or escalate; take a lock shared with the appender or merge newly appended records before replacing.
Useful? React with 👍 / 👎.
| print(json.dumps({"total": 0, "pending": 0, "fulfilled": 0, "escalated": 0})) | ||
| return 0 | ||
|
|
||
| records = [json.loads(line) for line in obligations.read_text().splitlines() if line.strip()] |
There was a problem hiding this comment.
A single malformed or partially written JSONL line makes json.loads raise before any pending records are processed, so one corrupt append/manual edit permanently prevents later overdue obligations from being fulfilled or escalated. Because the ledger is append-only and written by shell hooks, the watcher should tolerate bad lines the way the grant reader tolerates malformed grants, e.g. skip and report them rather than exiting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hooks/scripts/cc-task-gate.sh (1)
677-686:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing error suppression on section 10 emergency bypass.
Unlike the early emergency path (lines 223, 226), this path lacks
2>/dev/null || true. Withset -euo pipefail, a mkdir or write failure would cause the script to exit before the bypass completes — defeating the purpose of an emergency override that should work when infrastructure is broken.Proposed fix to match the early emergency path
if [[ "${HAPAX_METHODOLOGY_EMERGENCY:-0}" == "1" ]]; then _emergency_ledger="$HOME/.cache/hapax/methodology-emergency-ledger.jsonl" - mkdir -p "$(dirname "$_emergency_ledger")" + mkdir -p "$(dirname "$_emergency_ledger")" 2>/dev/null || true printf '{"ts":"%s","role":"%s","task":"%s","case":"%s","tool":"%s"}\n' \ "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "$role" "$task_id" "$authority_case" "$tool_name" \ - >> "$_emergency_ledger" + >> "$_emergency_ledger" 2>/dev/null || true echo "cc-task-gate: EMERGENCY BYPASS — logged to $_emergency_ledger" >&2 _record_retro_grant_obligation "$role" "$task_id" "$authority_case" "$tool_name" "HAPAX_METHODOLOGY_EMERGENCY" exit 0 fi🤖 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/scripts/cc-task-gate.sh` around lines 677 - 686, The emergency bypass branch guarded by HAPAX_METHODOLOGY_EMERGENCY sets _emergency_ledger and performs mkdir/printf/echo/_record_retro_grant_obligation but lacks error suppression, so failures under set -euo pipefail can abort the bypass; update the mkdir, file write/append and echo calls in that block (the mkdir -p "$(dirname "$_emergency_ledger")", the printf >> "$_emergency_ledger" append, and the echo to stderr) to ignore errors by appending redirection and a harmless true fallback (e.g., add 2>/dev/null || true to the mkdir and the write/echo commands) so the emergency path always continues and still calls _record_retro_grant_obligation and exit 0 even if filesystem ops fail.
🤖 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.
Outside diff comments:
In `@hooks/scripts/cc-task-gate.sh`:
- Around line 677-686: The emergency bypass branch guarded by
HAPAX_METHODOLOGY_EMERGENCY sets _emergency_ledger and performs
mkdir/printf/echo/_record_retro_grant_obligation but lacks error suppression, so
failures under set -euo pipefail can abort the bypass; update the mkdir, file
write/append and echo calls in that block (the mkdir -p "$(dirname
"$_emergency_ledger")", the printf >> "$_emergency_ledger" append, and the echo
to stderr) to ignore errors by appending redirection and a harmless true
fallback (e.g., add 2>/dev/null || true to the mkdir and the write/echo
commands) so the emergency path always continues and still calls
_record_retro_grant_obligation and exit 0 even if filesystem ops fail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c31f1ca9-6e28-4f10-aa86-1f9fa9ad4274
📒 Files selected for processing (2)
hooks/scripts/cc-task-gate.shtests/hooks/test_cc_task_gate.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/hooks/test_cc_task_gate.py
1 participant
Summary
Wires the daemon-independent escape grant shim (reform Phase 4, NEW-2/INV-4) and deprecates the
HAPAX_*_OFFoff-switch to incident-only. Closes the audit's central safety correction: theEscapeGrantsubstrate (shared/governance/coord_capabilities.py) shipped with zero non-test callers, so the only working escape from an irreversible-harm gate was the deprecated, silent, unconditionalHAPAX_*_OFF— the exact inversion of "obligatory but never stuck via an authorized scoped grant."reform-fix-grant-escape-shim-20260531coordination-reform-master-design-2026-05-30.md§4.4 (NEW-2), INV-4What changed
hooks/scripts/escape-grant.sh(new) — shared sourced helper.escape_grant_allows <gate>scans/var/lib/hapax/coord/grantsfor a signedEscapeGrantcovering the gate and verifies it viapython3 -m shared.governance.coord_capabilities verify-grant— a pure file read, never an RPC, so it works with the kernel down. Degrades closed (missing dir/key/python ⇒ no escape); ledgers each honored grant.hooks/scripts/cc-task-gate.sh— every BLOCK now routes through_emit_block, which honors a covering grant before failing closed.HAPAX_CC_TASK_GATE_OFF(formerly silent) is ledgered (cc_task_gate_off_bypass) + deprecation-warned;HAPAX_METHODOLOGY_EMERGENCYrecords a pending retro-grant obligation (1h deadline) on both bypass paths.hooks/scripts/no-stale-branches.sh— same helper wired into all 4 block points (destructive ×2, worktree-cap, stale-branches) — the exact gate that hard-walls a stale-worktree lane from creating a branch.scripts/coord-grant-mint(new) — operator/lane CLI: writes a signed, scoped, time-boxed grant to/var/lib/hapax/coord/grants/<id>.grant; auto-creates the signing key (0600,O_EXCL) on first mint.scripts/coord-retro-grant-watch(new) — closes the deprecation loop: fulfils a retro-grant obligation when a covering grant lands, ntfy-escalates when the 1h deadline passes with none.Acceptance criteria
coord-grant-mint/ CLI writes a signed grant file to/var/lib/hapax/coord/grants/<id>.grantwith a real signing key.HAPAX_CC_TASK_GATE_OFFandHAPAX_METHODOLOGY_EMERGENCYare ledgered; emergency records a retro-grant obligation.Test evidence (TDD — red→green for every behavior)
tests/hooks/test_cc_task_gate.py— 129 passed (16 new: grant honor / scope / expiry / forged-key / ledger, chaos/INV-4, gate-off ledger + warning, emergency retro-grant + 1h deadline)tests/hooks/test_no_stale_branches.py— 17 passed (4 new: grant override on branch-create + destructive reset, wrong-scope control)tests/scripts/test_coord_grant_mint.py— 4 passedtests/scripts/test_coord_retro_grant_watch.py— 6 passedtests/test_coord_capabilities.py— green (substrate unmodified)ruff check+ruff formatclean;shellcheck -S warningclean on all three shims.Notes
single_user). The HMAC key shares the operator's uid, so this enforces a deliberate, scoped, time-boxed, audited escape — not adversarial isolation between the operator's own lanes. It replaces the blunt, silent, unconditional off-switch.coord-retro-grant-watch(systemd/is outside the task'smutation_scope_refs); the watcher is runnable now.coord.grant.mintverb lives in the constitution repo (outside council's mutation scope); AC feat(dev-story): add git bundle and archived conversation indexing #1 is satisfied via the/CLIpath per the task note.test_session_context_*timeouts seen locally are pre-existingsession-context.sh(>10s) load flakiness on this box — zero coupling to this change (that script references none of these surfaces).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Tests