-
Notifications
You must be signed in to change notification settings - Fork 1.1k
docs(devlog): record why the root workflow budget expires a long session (#4546) #4653
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| # 260915 — the root workflow budget outlives the task it was sized for | ||
|
|
||
| ## What happened | ||
|
|
||
| A Codex session spent several hours dispatching subagents. Every dispatch failed, | ||
| across three unrelated providers, with a 429 that reads as a provider rate limit. | ||
| The obvious readings were all wrong: not the model, not the account, not the | ||
| upstream. The refusal came from this proxy. | ||
|
|
||
| The reproduction is one line. With the same request body, a probe carrying the | ||
| long-running session id in `x-codex-parent-thread-id` was refused, and a probe | ||
| carrying a freshly invented root id was served. Restarting the proxy served both. | ||
| That is the whole diagnosis: the ceiling is per root, held in process memory, and | ||
| the session had reached it. | ||
|
|
||
| ## Why the ceiling fired | ||
|
|
||
| `DEFAULT_WORKFLOW_BUDGET_POLICY` (`src/lib/workflow-budget.ts`) caps a root at 256 | ||
| physical sends and 64 distinct children. `workflowSendCeilingReached` compares | ||
| `state.sends >= policy.maxPhysicalSends`, and `state.sends` is **cumulative for the | ||
| life of the process**. The root id is `x-codex-parent-thread-id`, which for Codex is | ||
| the session. So the cap is not a fan-out guard on a long session; it is an expiry. | ||
|
|
||
| The comment that justifies it says a per-request cap "cannot bound a fan-out that | ||
| sends once per child seven hundred times". That is a **burst** concern, and a burst | ||
| is bounded by a rate. A lifetime total cannot tell seven hundred sends in a minute | ||
| from two hundred and fifty-six sends spread over four hours, and it refuses both. | ||
| The second one is ordinary work. | ||
|
|
||
| Two things made it expensive to diagnose rather than merely annoying. The refusal | ||
| is a 429 that an operator reads as an upstream rate limit, so the first hours went | ||
| to providers and accounts. And there is no way out except restarting the proxy: | ||
| `resetWorkflowBudgetsForTest` exists, the name says who it is for, and | ||
| `workflowBudgetSnapshot` is never exposed, so the state that decided the refusal is | ||
| invisible from outside the process. | ||
|
|
||
| ## The rule | ||
|
|
||
| > A root budget bounds a **rate**, and says so. A ceiling that fires is a local | ||
| > decision an operator can see, name and clear without restarting the proxy. | ||
|
|
||
| ## Roadmap | ||
|
|
||
| | Doc | Work phase | Outcome | | ||
| | --- | --- | --- | | ||
| | `010_windowed_ceilings.md` | wfb | Sends and distinct children are counted over a bounded window, so a long session is never refused for work it did hours ago while a burst inside one window still is | | ||
| | `020_legible_refusal.md` | wfc | The refusal names the ceiling that fired, is marked as a proxy decision rather than an upstream one, and the root budget can be read and cleared through the management API | | ||
|
|
||
| ## Write scope | ||
|
|
||
| Permitted: `src/lib/workflow-budget.ts`, the workflow call sites in | ||
| `src/server/responses/core.ts` and `src/server/index.ts`, the management read and | ||
| mutation surface under `src/server/management/`, `src/server/request-log.ts` for the | ||
| refusal provenance, their tests, and this unit. | ||
|
|
||
| ## Verification posture | ||
|
|
||
| Local suite, typecheck, install and GUI build are **not run** for this unit by | ||
| explicit instruction. Proof is hosted CI at the exact final head SHA and nothing | ||
| else. Pushes use `--no-verify`. | ||
|
|
||
| ## What would make this fail | ||
|
|
||
| Raising the numbers instead of fixing the shape. A bigger lifetime total is the | ||
| same defect further away: it still refuses a session for work it finished hours | ||
| ago, and it still cannot be seen or cleared. The window is the change; the numbers | ||
| are a consequence of it. | ||
|
|
||
|
|
||
| ## Reproducing it | ||
|
|
||
| Both probes carry the same body and differ only in the root id. Against a proxy | ||
| whose process has been up long enough for a session to reach the ceiling: | ||
|
|
||
| ```bash | ||
| BODY='{"model":"gpt-5.6-terra","input":[{"role":"user","content":[{"type":"input_text","text":"ok"}]}],"max_output_tokens":16,"stream":true}' | ||
|
|
||
| # the long-running session's own root: refused | ||
| curl -s -o /dev/null -w '%{http_code}\n' -N -X POST http://127.0.0.1:10100/v1/responses \ | ||
| -H 'Content-Type: application/json' -H 'Accept: text/event-stream' \ | ||
| -H "x-codex-parent-thread-id: <that session id>" -d "$BODY" | ||
|
|
||
| # any root the process has not seen: served | ||
| curl -s -o /dev/null -w '%{http_code}\n' -N -X POST http://127.0.0.1:10100/v1/responses \ | ||
| -H 'Content-Type: application/json' -H 'Accept: text/event-stream' \ | ||
| -H "x-codex-parent-thread-id: probe-$(date +%s)" -d "$BODY" | ||
| ``` | ||
|
|
||
| Two answers from one proxy, one body and one upstream, separated only by which | ||
| root the request claims. That is what rules out the provider, the account and the | ||
| model in a single step, and it is the check to run first the next time a fan-out | ||
| starts failing for no visible reason. | ||
|
Comment on lines
+89
to
+92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In a Codex Pool installation with multiple stored accounts, these probes do not hold the upstream constant: Useful? React with 👍 / 👎. |
||
|
|
||
| After a restart both return 200, which is the other half of the diagnosis: the | ||
| ceiling is process-memory only, so the evidence disappears the moment anyone tries | ||
| the obvious remedy. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| # 010 — wfb: count sends and children over a window, not over a lifetime | ||
|
|
||
| ## Today | ||
|
|
||
| ```ts | ||
| export function workflowSendCeilingReached(rootId, policy) { | ||
| const state = roots.get(rootId); | ||
| return state !== undefined && state.sends >= policy.maxPhysicalSends; | ||
| } | ||
| ``` | ||
|
|
||
| `state.sends` only ever grows. `state.children` is a Set that only ever gains | ||
| members. Neither has a clock. A root that made 256 sends in its first hour is | ||
| refused for the rest of the process even if it sends nothing for a day. | ||
|
|
||
| ## The change | ||
|
|
||
| Keep the counters, add a window. A root records its sends as timestamped buckets | ||
| and the ceiling compares the count **inside the window** against | ||
| `maxPhysicalSends`. Distinct children get the same treatment: a child seen once, | ||
| hours ago, and never again should not hold a slot forever. | ||
|
|
||
| The default window has to be argued for rather than picked. 256 sends is the | ||
| number already in the tree and it was chosen against a fan-out, so the window is | ||
| the interval over which that fan-out would be abusive. A ten-minute window keeps | ||
| the original intent — seven hundred sends in a minute is still refused several | ||
| times over — while an ordinary session that averages well under a send every two | ||
| seconds never approaches it. | ||
|
|
||
| `maxConcurrentChildren` stays as it is. Concurrency is already instantaneous; it | ||
| has no lifetime problem to fix. | ||
|
|
||
| ## What must not change | ||
|
|
||
| An unconfigured install must not see a refusal it would not have seen before. | ||
| Windowing only ever admits more, never less, for the same traffic — the count | ||
| inside a window is bounded by the lifetime count — so this direction is safe by | ||
| construction. Say so in a test rather than trusting the argument. | ||
|
|
||
| The eviction rules from #4546 stay: a root is evicted only when it is both | ||
| inactive and not exhausted, and a full table refuses rather than laundering a | ||
| fan-out into a fresh allowance. A windowed root that has aged out of its window is | ||
| no longer exhausted, which is exactly the state that makes it evictable again. | ||
|
|
||
| ## Acceptance | ||
|
|
||
| 1. A root at the ceiling is admitted once its window rolls, without a restart. | ||
| 2. A burst inside one window is still refused at the same count as before. | ||
| 3. Distinct children age out of the window the same way sends do. | ||
| 4. Bucket storage per root is bounded; a root that sends forever does not grow | ||
| forever. | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # 020 — wfc: a refusal an operator can read, name and clear | ||
|
|
||
| ## Today | ||
|
|
||
| The refusal is a 429 with `workflow_budget_exhausted` and a sentence about the | ||
| task's send budget. Two problems, and the first one cost hours. | ||
|
|
||
| A 429 from a proxy that also forwards provider 429s is ambiguous. There is nothing | ||
| on the record that says which one this was, so the first move is always to go look | ||
| at the provider. This is the same defect #4639 fixed for the synthetic 503: a | ||
| locally generated refusal presented under a field an operator reads as upstream. | ||
| The fix there was provenance on the record, and it applies unchanged here. | ||
|
|
||
| The second is that there is no way out. `resetWorkflowBudgetsForTest` is named for | ||
| its audience and `workflowBudgetSnapshot` has no caller outside the module, so the | ||
| state that decided the refusal cannot be read and cannot be cleared except by | ||
| restarting the proxy — which drops every other root's accounting with it. | ||
|
|
||
| ## The change | ||
|
|
||
| Name the ceiling. The denial type already distinguishes | ||
| `workflow-sends-exhausted` from `workflow-children-exhausted` and the rest; carry | ||
| that through to the error body and onto the request log instead of collapsing it | ||
| into one sentence. | ||
|
|
||
| Mark it local. The request log gains the same origin treatment #4639 introduced, so | ||
| a proxy refusal and an upstream 429 are distinguishable on the record and on the | ||
| management read surface. | ||
|
|
||
| Expose and allow clearing. `GET` the root's budget through the management API so an | ||
| operator can see a ceiling approaching rather than discovering it, and allow a | ||
| bounded, recorded clear of one root. Clearing one root is not the same as | ||
| restarting: it is scoped, it is logged, and it leaves every other root's accounting | ||
| intact. | ||
|
|
||
| ## What must not change | ||
|
|
||
| The clear is an operator action on the operator's own proxy, not a path a request | ||
| can take. It goes through the management surface, which already requires a | ||
| dashboard session or the admin token, and it must not be reachable from the data | ||
| plane. A fan-out cannot be allowed to clear its own ceiling — that would make the | ||
| budget a suggestion, which is the failure #4546 spent a release removing. | ||
|
|
||
| ## Acceptance | ||
|
|
||
| 1. The refusal body and the request log name which ceiling fired. | ||
| 2. The record marks the refusal as proxy-origin, distinguishable from an upstream 429. | ||
| 3. An operator can read one root's budget and clear it through the management API. | ||
| 4. The clear is scoped to one root, is recorded, and is not reachable from the data plane. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Publishing this
_planunit exposes an unfixed spend-guard bypass: the command tells a data-plane client how to evade an exhausted root by minting a newx-codex-parent-thread-id, while the planned windowing and management fixes have not shipped. Repository policy explicitly places bypass reasoning and reproduction steps for unfixed defects in scratch rather than trackeddevlog; remove this unit from the commit and keep it under.tmp/until the fix is public.AGENTS.md reference: AGENTS.md:L124-L130
Useful? React with 👍 / 👎.