Repository navigation
fix(agents): stop charging mutex queue time to the implementer's work budget - #1282
Conversation
… budget `activeDeadlineSeconds` sat at spec level, and a spec-level deadline runs from submit — not from the moment the `mctl-agents-proposal-claims` mutex is acquired. That mutex has capacity 1 and one proposal takes 30-70 min, so only the first two or three submissions of a batch could start inside the 2h budget. Everything behind them expired Pending, having never created a pod. The lost attempt was not the worst of it. A spec-level deadline is workflow-wide, so once `implement` had consumed it, `implement-fallback` and `commit-and-push` were both failed instantly with "Step exceeded its deadline" — and `commit-and-push` is the only step that writes `.status.yaml` back. The proposal stayed `accepted` with no branch, no PR and no failure recorded anywhere, which is the exact state the `assert-produced` truthful-status design exists to prevent. On the night of 2026-09-19 six of nine implement runs were lost this way: mctl-agents#333 #334, mctl-telegram#438 #569, mctl-gitops#903 #1178. The controller log is unambiguous — "...isn't at the front" lockType=mutex name=argo-workflows/Mutex/mctl-agents-proposal-claims "Could not acquire lock named: .../mctl-agents-proposal-claims" "Workflow pod is missing" nodePhase=Pending recentlyStarted=false and the lock timeline matches the outcomes exactly: `bcc54a61` held it until 01:58, `0eaa9853` (telegram#438) acquired at 01:58 against its own 02:12:31 deadline, got 14 of the 40-70 min it needed, and died. The 7200s budget moves to the `run-implementer` template, where a deadline is applied to the pod and therefore runs from the pod's start, after the mutex is acquired. It covers `implement` and `implement-fallback` alike — both are that one template — so a primary that burns its full budget no longer leaves the fallback nothing to run in. Spec level keeps a ceiling, but as a runaway guard rather than a work budget: 8h covers worst-case work (7200 + 7200 + a bounded commit) plus room to queue behind two or three other runs. `commit-and-push` gets its own 900s so that, without the tight spec ceiling, the step that records the outcome cannot be the one that hangs. A deep enough queue can still expire at the spec ceiling — but it now expires having attempted nothing, rather than discarding a finished attempt. Narrowing the mutex to the discovery+claim critical section it actually protects is the real fix for the queue, and is tracked separately. The precedent is in this same file: `mctl-gitops-main-writes` was moved off the workflow spec onto `commit-and-push` for exactly this reason, after holding it across the whole implementer run starved other main-writers. Verified with `kubectl apply --dry-run=server`. Claude-Session: https://claude.ai/code/session_01NFS84XSBZ6vMKsd7CQr6Bf
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Claude finished @mashkovd's task in 2m 13s —— View job Review complete ✅
Verdict: APPROVE — no P1/P2 findings, 0 P3. Analysis:
No test suite applies to this repo beyond |
Antigravity (agy) review — informationalModel: gemini-3.8-flash-medium · commit Finding 1: Invalid
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
No P1/P2 findings (0 P3). Deadline placement is correct: run-implementer's new 7200s activeDeadlineSeconds is a POD-level deadline that starts after the mctl-agents-proposal-claims mutex is acquired, matching the intent of excluding queue time. The 8h spec ceiling is consistent with worst case (7200 primary + 7200 fallback + up to ~3×900s+backoff commit-and-push retries ≈ 4.75h) plus queue headroom. Verified this doesn't break scripts/validate-agent-platform.py's profile/CWFT timeout cross-check: cwft-mctl-agents-implement.yaml still declares IMPLEMENTER_TIMEOUT_SECONDS=5400, so that check derives its effective timeout from the env var (unchanged) rather than spec.activeDeadlineSeconds, so raising the spec ceiling to 28800 doesn't drift it out of sync with implementer-default profile.yaml's timeoutSeconds: 5400 or policy.yaml's maxTimeoutSeconds: 7200 ceiling. Good to merge.
Response to the agy review on
|
activeDeadlineSecondssat at spec level, and a spec-level deadline runsfrom submit — not from the moment the
mctl-agents-proposal-claimsmutexis acquired. That mutex has capacity 1 and one proposal takes 30-70 min,
so only the first two or three submissions of a batch could start inside
the 2h budget. Everything behind them expired Pending, having never
created a pod.
The lost attempt was not the worst of it. A spec-level deadline is
workflow-wide, so once
implementhad consumed it,implement-fallbackand
commit-and-pushwere both failed instantly with "Step exceeded itsdeadline" — and
commit-and-pushis the only step that writes.status.yamlback. The proposal stayedacceptedwith no branch, no PRand no failure recorded anywhere, which is the exact state the
assert-producedtruthful-status design exists to prevent.On the night of 2026-09-19 six of nine implement runs were lost this way:
mctl-agents#333 #334, mctl-telegram#438 #569, mctl-gitops#903 #1178. The
controller log is unambiguous —
and the lock timeline matches the outcomes exactly:
bcc54a61held ituntil 01:58,
0eaa9853(telegram#438) acquired at 01:58 against its own02:12:31 deadline, got 14 of the 40-70 min it needed, and died.
The 7200s budget moves to the
run-implementertemplate, where adeadline is applied to the pod and therefore runs from the pod's start,
after the mutex is acquired. It covers
implementandimplement-fallbackalike — both are that one template — so a primary that burns its full
budget no longer leaves the fallback nothing to run in.
Spec level keeps a ceiling, but as a runaway guard rather than a work
budget: 8h covers worst-case work (7200 + 7200 + a bounded commit) plus
room to queue behind two or three other runs.
commit-and-pushgets itsown 900s so that, without the tight spec ceiling, the step that records
the outcome cannot be the one that hangs.
A deep enough queue can still expire at the spec ceiling — but it now
expires having attempted nothing, rather than discarding a finished
attempt. Narrowing the mutex to the discovery+claim critical section it
actually protects is the real fix for the queue, and is tracked
separately. The precedent is in this same file:
mctl-gitops-main-writeswas moved off the workflow spec onto
commit-and-pushfor exactly thisreason, after holding it across the whole implementer run starved other
main-writers.
Verified with
kubectl apply --dry-run=server.Effect
Not in this PR, on purpose
mctl-agents-proposal-claimsto discovery+claim. That is the real fix for the queue and deserves its own change, not a hotfix.🤖 Generated with Claude Code
https://claude.ai/code/session_01NFS84XSBZ6vMKsd7CQr6Bf