Commit Graph

3 Commits

Author SHA1 Message Date
kshitij ea0d54db1d refactor: fold /simplify-code findings
- Single source for the approval-derived bound: public human_wait_ceiling()
  in tools/approval.py; the gate's lock-timeout helper delegates to it
  instead of re-deriving timeout + margin (was duplicated in two modules
  and reached for a private _get_approval_timeout).
- Shared _clamped_window_seconds() for the close-time accrual and the
  open-window read, so the two clamps are identical by construction.
- Gate __init__ grows session_key kwarg; tests construct via the real
  constructor instead of mutating privates post-hoc.
- Gateway test resolves its pending approval via resolve_gateway_approval()
  (the production /deny path) instead of hand-rolling queue-entry internals.
- Docstring accuracy: human_wait_seconds monotonicity caveat under cap
  eviction; s/pre_tool_block/pre_tool_call/ hook name.
2026-08-06 17:03:10 +05:30
kshitij 10fb01e725 fix: harden human-wait tracker from review findings
Review-driven follow-up to the #79719 fix:

- Clamp the CLOSE-side accrual too: a wedged window that eventually closed
  used to inject its full unclamped overstay into completed_seconds,
  retroactively extending a running batch's deadline by hours. Both clamps
  now share one ceiling helper (_human_wait_ceiling = approvals.timeout +
  HUMAN_WAIT_MARGIN_S), and the gate's lock-timeout uses the same margin
  constant so the bounds cannot drift apart.

- Evict idle sessions until the table is under the cap (was: at most one
  per insert, so churn could outgrow _HUMAN_WAIT_MAX_SESSIONS). Entries
  with an open window are still never evicted.

- Log (debug) instead of silently swallowing a failed session-key snapshot
  in the gate constructor.

Tests: close-side clamp regression + table-cap assertion added; suite at
17 passed.
2026-08-06 17:03:10 +05:30
kshitij 3305cfd2bb fix(agent): measure batch-deadline exclusion at the human wait, not authorization-gate residency
A tool wedged inside _ConcurrentToolAuthorizationGate hung the whole turn
forever (#79719): excluded_seconds() measured residency in gate.run() —
arbitrary code — so an open window grew 1:1 with wall clock and the batch
deadline's remaining was constant (remaining = deadline - window_started;
now cancels out). A hanging pre_tool_call plugin or an approval round-trip
to a dead client defeated the deadline entirely. The serialization lock was
also an unbounded acquire, so every other worker needing authorization
parked behind the wedged holder forever.

Fix, in two halves:

- tools/approval.py grows per-session human-wait accounting
  (human_wait_window / human_wait_seconds). The two places that are
  verifiably blocked on a HUMAN — the CLI approval prompt and the gateway
  approval poll loop — mark their own windows. Both are intrinsically
  bounded by approvals.timeout; the open-window read is additionally
  clamped to that timeout plus a margin as belt-and-braces.

- _ConcurrentToolAuthorizationGate keeps only serialization, with a bounded
  acquire (approvals.timeout + 60s; on expiry the prompt runs unserialized —
  the same degradation the start-order gate accepted in #79705).
  excluded_seconds() becomes a baseline-delta read of the session's
  human-wait total.

A wedged plugin now contributes nothing to the exclusion, so the batch
times out at the normal deadline with correctly labeled results, while a
genuine approval wait — which can legitimately exceed any fixed bound —
still extends the deadline in full. E2E (real AIAgent, worktree imports):
wedged-plugin batch on main never ends (>30s observed, 3s deadline); with
the fix it ends at 3.0s. A 4s simulated approval over a 2s deadline
completes without a timeout label.

Closes #79719
2026-08-06 17:03:10 +05:30