Skip to content

Expire timed-out confirmation requests without executing actions - #108

Draft
ThinkOffApp wants to merge 1 commit into
mainfrom
claudemm/gate-expire-endpoint
Draft

ThinkOffApp wants to merge 1 commit into
mainfrom
claudemm/gate-expire-endpoint

Conversation

@ThinkOffApp

Copy link
Copy Markdown
Owner

Timed-out PreToolUse gates call an expiry endpoint that the daemon did not implement, leaving completed requests visible as pending. Add explicit expiry without executing actions or overwriting existing human decisions, and return an expired result to active waiters.

Review in progress. The existing 29 confirmation tests pass independently, but review found remaining cases before rollout: mirror expiry to central action status, return immediately to waiters started after expiry, reject non-object request bodies, and prove late-click/typed-action nonexecution. The current patch's late waiter returns timeout and its mirror emits only pending; reproductions confirmed locally.

This queue lifecycle fix is separate from the user-requested dynamic team-lead approval and restricted scratch cleanup policy. No approval authority or live service configuration is changed by this PR.

The PreToolUse gate waits IAK_GATE_TIMEOUT for a human, then proceeds under
IAK_GATE_DEFAULT=allow and POSTs /intent/<id>/expire so the request stops
sitting in the queue. The daemon has never implemented that route, so every
timed-out intent stayed `pending` for ever: on 2026-09-18 there were 31 of them,
the oldest created three minutes after the running daemon started, each one a
command that had already executed and a button that could no longer change
anything.

expireIntent is deliberately not decideIntent:
  - a human approve/deny that arrived first is left untouched, and the call
    reports the existing decision rather than overwriting it;
  - it never runs an action. decideIntent executes an action bound to the
    intent, and a timeout is not consent to do that.

waitForDecision also reported an expiry as {status:'decided', decision:null},
which is how a caller ends up believing someone approved something. It now
reports {status:'expired'}.

Tests cover: settles without deciding, leaves the pending queue, idempotent,
never overrides a human decision, releases waiters, unknown id, and the HTTP
route including its 404.

This makes the buttons honest. It does NOT address the larger point that the
gate notifies a human about commands it will allow regardless — that is a
policy decision, raised separately.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ThinkOffApp
ThinkOffApp force-pushed the claudemm/gate-expire-endpoint branch from 5916ebd to 75b41f4 Compare September 21, 2026 07:00
@ThinkOffApp

Copy link
Copy Markdown
Owner Author

Rebased onto main (was at 5916ebd, main had moved to ec7e1e8).

Stacking check: compared commit ancestry against the other open PRs. #108's commit is not an ancestor of, and shares no commits with, #115 (claudemm/team-lead-approvals) or #106 (codex/coalesce-intent-publishing); neither of those is an ancestor of #108 either. #108 is not stacked on or under any other open PR. #115 does not contain #108's commit, so #115 is not stacked on #108. Not superseded on main (the commit was absent from main before this rebase).

Conflicts and resolutions:

  1. test/confirmations.test.mjs import list — both sides added a new named import (getIntent on main, expireIntent on this branch) at the same line. Kept both imports.

  2. test/confirmations.test.mjs, large hunk — both sides appended new test blocks immediately after the same anchor test (defaultCallbackBase: a bound IPv6 host is preserved and bracketed): main added the "choice intents" and "HTML queue renders the server wording" tests, this branch added the five expireIntent//intent/:id/expire tests. These are independent additions with no shared logic, so the resolution keeps both blocks concatenated (main's additions first, then this branch's), each retaining its own closing tags. Rebuilt from the pristine source files rather than hand-patching the markers, since git's line diff had matched an unrelated trailing }); from each side as a false "common" line.

  3. src/confirmations.mjs — auto-merged cleanly by git, no manual resolution needed. Verified afterward: the new /intent/:id/expire route regex (^\/intent\/([^/]+)\/expire$) doesn't shadow or get shadowed by any other route (/intent/:id/decision, /intent, /intents, /actions/*, /ide-chat/:id) — checked route registration order.

Verification (worktree npm ci, Node's built-in test runner; compared against a control worktree at unmodified origin/main):

control (main @ ec7e1e8) this branch (rebased)
tests 659 pass, 0 fail 664 pass, 0 fail

comm on the sorted test-name lists from both runs: the branch adds exactly the 5 expected new tests (expireIntent settles a pending intent without deciding it, expireIntent is idempotent and never overrides a human decision, expireIntent releases waiters instead of leaving them hanging, expireIntent rejects an unknown id, HTTP POST /intent/:id/expire settles it, 404s an unknown id) and drops none of main's 543. Zero introduced failures, zero regressions.

Not verified: typecheck and lint. This repo has no tsconfig.json, no ESLint config, and no typecheck/lint script in package.json — there is nothing of that kind to run here, on either branch or control.

Force-pushed the rebased branch (old tip 5916ebd → new tip 75b41f4) with --force-with-lease. Left as draft, not merged.

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