fix(worker,shiftengine): a killed run reports its own death, and does not spend the agent budget #39

Merged

ryangr0 merged 3 commits from fix/infra-kills-dont-park-the-ticket into development 2026-08-26 05:08:24 +00:00

Conversation 0 Commits 3 Files changed 16 +636 -30

ryangr0 commented 2026-08-26 04:46:08 +00:00

Owner

Copy link

What

Shift 73 parked work item 98 at needs_human on 2026-08-13 after three pods were killed mid-run. The agents were working correctly in all three — LiteLLM's ledger records 19, 17 and 1 completed model calls, context growing 18k → 53k tokens, each pod dying within seconds of a call that had just succeeded, for $0.0237 of an $8.00 pool. Nothing about the work was ever tried, and the close reason sent a person to read agent logs.

Two defects, one behind the other.

1. A killed worker reported nothing

ploeg-worker installed no signal handler at all, so SIGTERM killed it on the default disposition: no outcome, no revoked credential, and the Lease left to the sweeper a full 15 minutes later, attributed to nothing.

RunContext now takes a cancellable parent; cmd/ploeg-worker hands it SIGINT/SIGTERM. The run context is deliberately not derived from that parent — the harness must die with it, but revoke, settle and report have to survive it, or the shutdown reports nothing and we are back where we started. context.WithCancelCause carries why: errTerminated maps to failed/infra_node, errLeaseLost keeps the existing stuck/lease_lost.

terminationGracePeriodSeconds (new value, default 90) is load-bearing: the 30s default SIGKILLed the pod mid-report and left the handler inert.

2. Infrastructure spent the agent's retry budget

store.ExpireLeases has always counted infrastructure apart on the pre-Shift path — refunding the attempt, tracking infra_failures, capping at MaxInfraFailures. The Shift path inherited none of it, and for a Round whose only producer of failed is the sweeper, the ticket's three attempts were being spent entirely on the cluster.

FailedRunsInRound now returns InfraAttempts beside Attempts, partitioned in SQL by work.InfraFailureReasons() so query and engine cannot drift. retryFailedWriter bounds them separately and closes with writing_run_killed_repeatedly when infrastructure is at fault — a close reason that points at evictions and node pressure rather than at the ticket.

Applied to item 98: three kills would have left all three agent attempts intact.

Also

settleSpend ran on the run context, so a cancelled run lost its cost settlement — which is why shift 73 reads spent=0.0000 against real gateway spend. It runs detached now.

Not fixed here

No backoff between infra retries on this path, where ExpireLeases has 1/5/15/60-minute steps. Gating a reopened Run on a time would change ClaimRole and the KEDA trigger predicate together, and those must stay byte-identical. Recorded as an accepted cost and a re-evaluation trigger in ADR-0021.

Open question for review: the infra budget reuses MaxInfraFailures (10). Without backoff that is 10 rapid pods on a sick cluster, where it used to stop at 3. A smaller dedicated constant is a two-line change if you'd rather.

Records

• ADR-0021 (new) — refines ADR-0019, whose first re-evaluation trigger fired here: "a writing Run reaches MaxRunAttempts in production more than once in a month — the failure is not transient and retrying is the wrong response." It did, three times. The re-evaluation reaches the opposite conclusion: the failure was transient, it just could not be told apart from a considered one.
• ADR-0019 — dated history + "Refined by" appended; body untouched, per the append-only rule.
• openspec/specs/shift-orchestration — requirement and scenarios updated. Edited directly rather than via a change directory; say so if you would rather it went through propose to apply to archive.

Verification

go build, go vet, full suite and helm goldens green. New tests: TestFailedWriter_InfraKillsDoNotSpendTheAgentBudget, TestFailedWriter_ParksAtTheInfraCapAndSaysSo, TestAbortOnTermination_*, and TestInfraFailureReasons_MatchesIsInfra (fails if the SQL list and the Go predicate ever drift).

🤖 Generated with Claude Code

## What Shift 73 parked work item 98 at `needs_human` on 2026-08-13 after three pods were killed mid-run. The agents were working correctly in all three — LiteLLM's ledger records **19, 17 and 1 completed model calls**, context growing 18k → 53k tokens, each pod dying within seconds of a call that had just succeeded, for **$0.0237 of an $8.00 pool**. Nothing about the work was ever tried, and the close reason sent a person to read agent logs. Two defects, one behind the other. ### 1. A killed worker reported nothing `ploeg-worker` installed **no signal handler at all**, so SIGTERM killed it on the default disposition: no outcome, no revoked credential, and the Lease left to the sweeper a full 15 minutes later, attributed to nothing. `RunContext` now takes a cancellable parent; `cmd/ploeg-worker` hands it SIGINT/SIGTERM. The run context is deliberately **not** derived from that parent — the harness must die with it, but revoke, settle and report have to survive it, or the shutdown reports nothing and we are back where we started. `context.WithCancelCause` carries *why*: `errTerminated` maps to `failed`/`infra_node`, `errLeaseLost` keeps the existing `stuck`/`lease_lost`. `terminationGracePeriodSeconds` (new value, default 90) is load-bearing: the 30s default SIGKILLed the pod mid-report and left the handler inert. ### 2. Infrastructure spent the agent's retry budget `store.ExpireLeases` has always counted infrastructure apart on the pre-Shift path — refunding the attempt, tracking `infra_failures`, capping at `MaxInfraFailures`. The Shift path inherited none of it, and for a Round whose only producer of `failed` is the sweeper, the ticket's three attempts were being spent entirely on the cluster. `FailedRunsInRound` now returns `InfraAttempts` beside `Attempts`, partitioned in SQL by `work.InfraFailureReasons()` so query and engine cannot drift. `retryFailedWriter` bounds them separately and closes with **`writing_run_killed_repeatedly`** when infrastructure is at fault — a close reason that points at evictions and node pressure rather than at the ticket. Applied to item 98: three kills would have left all three agent attempts intact. ### Also `settleSpend` ran on the run context, so a cancelled run lost its cost settlement — which is why shift 73 reads `spent=0.0000` against real gateway spend. It runs detached now. ## Not fixed here No backoff between infra retries on this path, where `ExpireLeases` has 1/5/15/60-minute steps. Gating a reopened Run on a time would change `ClaimRole` and the KEDA trigger predicate together, and those must stay byte-identical. Recorded as an accepted cost and a re-evaluation trigger in ADR-0021. **Open question for review:** the infra budget reuses `MaxInfraFailures` (10). Without backoff that is 10 rapid pods on a sick cluster, where it used to stop at 3. A smaller dedicated constant is a two-line change if you'd rather. ## Records - **ADR-0021** (new) — refines ADR-0019, **whose first re-evaluation trigger fired here**: *"a writing Run reaches MaxRunAttempts in production more than once in a month — the failure is not transient and retrying is the wrong response."* It did, three times. The re-evaluation reaches the opposite conclusion: the failure *was* transient, it just could not be told apart from a considered one. - ADR-0019 — dated history + "Refined by" appended; body untouched, per the append-only rule. - `openspec/specs/shift-orchestration` — requirement and scenarios updated. Edited directly rather than via a change directory; say so if you would rather it went through propose to apply to archive. ## Verification `go build`, `go vet`, full suite and helm goldens green. New tests: `TestFailedWriter_InfraKillsDoNotSpendTheAgentBudget`, `TestFailedWriter_ParksAtTheInfraCapAndSaysSo`, `TestAbortOnTermination_*`, and `TestInfraFailureReasons_MatchesIsInfra` (fails if the SQL list and the Go predicate ever drift). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

ryangr0 added 1 commit 2026-08-26 04:46:08 +00:00

fix(worker,shiftengine): a killed run reports its own death, and does not spend the agent's budget ... df5dc9f3e3

Shift 73 parked work item 98 at needs_human on 2026-08-13 after three pods
were killed mid-run. The agents were working correctly in all three —
LiteLLM's ledger records 19, 17 and 1 completed model calls with context
growing 18k -> 53k tokens, each pod dying within seconds of a call that had
just succeeded, for $0.0237 of a $8.00 pool. Nothing about the work was ever
tried, and the close reason sent a person to read agent logs.

Two defects, one behind the other.

ploeg-worker installed no signal handler at all, so SIGTERM killed it on the
default disposition: no outcome reported, no per-run credential revoked, and
the Lease left to the sweeper a full 15 minutes later, attributed to nothing.
RunContext now takes a cancellable parent and cmd/ploeg-worker hands it
SIGINT/SIGTERM. The run context is deliberately NOT derived from that parent
— the harness must die with it, but revoke, settle and report have to survive
it, or the shutdown reports nothing and we are back where we started.
context.WithCancelCause carries WHY: errTerminated maps to failed/infra_node,
errLeaseLost keeps the existing stuck/lease_lost, and the two now mean
opposite things to the Round.

Then the budget. store.ExpireLeases has always counted infrastructure apart
on the pre-Shift path — refunding the attempt, tracking infra_failures,
capping at MaxInfraFailures. The Shift path inherited none of it and charged
every `failed` Run to the same three attempts, which for a Round whose only
producer of `failed` is the sweeper means the ticket's budget was being spent
entirely on the cluster. FailedRunsInRound now returns InfraAttempts beside
Attempts, partitioned in SQL by work.InfraFailureReasons() so the query and
the engine cannot drift; retryFailedWriter bounds them separately and closes
with `writing_run_killed_repeatedly` when the infrastructure is at fault —
a close reason that points at evictions and node pressure rather than at the
ticket. An unset or unknown failure_reason counts against the agent: a reason
nobody set must not buy unlimited retries.

Also fixed on the way: settleSpend ran on the run context, so a cancelled run
lost its cost settlement entirely — which is why shift 73 reads spent=0.0000
against real gateway spend. It runs detached now.

terminationGracePeriodSeconds (new value, default 90) is load-bearing: the
30s default SIGKILLed the pod mid-report and left the handler inert.

Not fixed here: there is still no backoff between infra retries on the Shift
path, where ExpireLeases has 1/5/15/60-minute steps. Gating a reopened Run on
a time would change ClaimRole and the KEDA trigger predicate together, and
those must stay byte-identical. Recorded as an accepted cost and a
re-evaluation trigger in ADR-0021.

Refs: ADR-0021 (refines ADR-0019, whose first re-evaluation trigger fired
here), openspec shift-orchestration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

ryangr0 added 1 commit 2026-08-26 04:48:24 +00:00

style(worker): gofmt the appended regression tests ... 63e5101d05

A stray double blank line from appending the tests, which `gofmt -l`
in the checks workflow rejects. No behaviour change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

ryangr0 added 1 commit 2026-08-26 04:55:01 +00:00

fix(adrs): revert the ADR-0017 index edit — upstream had already resolved it ... 721630de56

The earlier commit set 0017's Records row to `accepted` to match its
frontmatter. That reading came from a checkout eight commits behind
development, where the file said `accepted` and the row said `proposed`.
Upstream had already fixed the same drift the other way, in 26364d6: the
file is `proposed` and the row is `proposed`, consistently.

Rebasing carried my edit onto the resolved state and broke parity in the
opposite direction, which is what TestADRIndexParity was failing on. It also
would have flipped a decision's recorded status from proposed to accepted —
a governance change, not the clerical fix I took it for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

ryangr0 merged commit a49a61efbb into development 2026-08-26 05:08:24 +00:00

ryangr0 referenced this pull request from a commit 2026-08-26 05:08:24 +00:00

Merge pull request 'fix(worker,shiftengine): a killed run reports its own death, and does not spend the agent budget' (#39) from fix/infra-kills-dont-park-the-ticket into development

Commenting is not possible because the repository is archived.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

webgrip/ploeg!39

No description provided.