Skip to content

DRU-989 -- A run waiting for the usage window shows as running - #1075

Merged
czpython merged 1 commit into
czpython:mainfrom
p-r-a-v-i-n:fix/org-spend-limit-retry
Nov 3, 2026
Merged

czpython merged 1 commit into
czpython:mainfrom
p-r-a-v-i-n:fix/org-spend-limit-retry

Conversation

@p-r-a-v-i-n

@p-r-a-v-i-n p-r-a-v-i-n commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

Fixes #981.

Claude's monthly spend-limit message also contains session limit resets....

Druks was matching session limit and treating the error like a normal quota window. It shut down the VM, waited for the next window, and retried. But the monthly spend limit was still there, so the retry only wasted another usage window.

Now:

  • monthly spend limit is checked before session limit, and the call fails directly.
  • Normal quota-window errors still wait and retry as before.
  • While waiting, the dashboard shows that it is waiting for quota and includes the real reset time when available.
  • The reset time comes from the latest usage scrape.
  • Failed Claude calls save their cost and tokens before raising the error.
  • Existing runs keep their old DBOS operation order when this change is deployed.

I kept the existing phase string and added phaseResetsAt separately, so the existing field does not change shape.

No database migration, dependency, or lockfile change.

This does not preserve uncommitted files from the VM. That is a separate problem.

@p-r-a-v-i-n
p-r-a-v-i-n requested a review from czpython as a code owner November 3, 2026 06:56
@p-r-a-v-i-n
p-r-a-v-i-n marked this pull request as draft November 3, 2026 06:58
@p-r-a-v-i-n
p-r-a-v-i-n force-pushed the fix/org-spend-limit-retry branch from 79a4fbd to 8ceaae6 Compare November 3, 2026 08:28
@p-r-a-v-i-n
p-r-a-v-i-n marked this pull request as ready for review November 3, 2026 08:30
@p-r-a-v-i-n
p-r-a-v-i-n force-pushed the fix/org-spend-limit-retry branch from 8ceaae6 to 4da0b93 Compare November 3, 2026 08:35
Comment thread backend/druks/harnesses/claude.py Resolved
Comment thread backend/druks/agents.py Resolved
Comment thread backend/druks/agents.py Resolved
Comment thread backend/druks/durable/activity.py Resolved
Comment thread backend/druks/durable/reads.py Resolved
Comment thread backend/druks/durable/schemas.py Resolved

@czpython czpython left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core fix is right. The spend-limit marker now comes before session limit, and write_cost runs before check_returncode. Please make these changes before merge:

  1. Every phase push now records two DBOS operations (blocker). set_run_phase always sends phase_resets_at before phase. DBOS records each set_event as an operation and checks its name on replay. A run that is active at deploy time, for example a build parked at a gate, replays with shifted operation ids and fails with DBOSUnexpectedStepError. reads.py already ignores the reset outside waiting_for_quota, so the clearing write is not necessary. Send phase_resets_at only when resets_at is given.

  2. Remove the ISO round trip. Druks uses the DBOS pickle serializer, so a datetime passes through step results and events unchanged. Return (wait_seconds, reset) from _quota_retry_wait. Then remove the dict, float(...), str(...) and the isoformat/fromisoformat calls in agents.py and activity.py. get_run_phase_resets_at can return the event directly, and the test does not need clock.fromisoformat.side_effect.

  3. agents.py: quota_resets_at = None runs on every HarnessError, and Retry.QUOTA is checked a second time after the chain. Move set_run_phase(...) into the elif Retry.QUOTA branch, after the max-wait raise.

  4. harnesses/base.py: the comment change is not part of the fix. Revert it.

  5. The marker and the description do not agree. The description says spend limit and the code says monthly spend limit. Use one of the two in both places.

  6. Tests:

    • Delete test_run_phase.py. It tests a fake DBOS dict.
    • Remove the cost assertions in test_a_carried_failure_is_raised_with_its_code. They test behavior that already exists: sandbox/host.py reads the cost after a failure, and AgentCall.finish records it.
    • In test_failed_claude_call_writes_cost_before_raising, keep only the cost assertions. code and retry are class constants, and test_sandboxed_harness.py already covers the classification.
  7. Frontend (minor): phaseResetsAt: string | null | undefined = undefined has a default that is not necessary. ?: is sufficient. You can put waiting_for_quota in PHASE_LINES and add a special case only when a reset time is present.

@p-r-a-v-i-n
p-r-a-v-i-n force-pushed the fix/org-spend-limit-retry branch 2 times, most recently from 91c789c to 2b21b9c Compare November 3, 2026 10:40
@p-r-a-v-i-n
p-r-a-v-i-n requested a review from czpython November 3, 2026 10:41
Comment thread backend/druks/agents.py Resolved
@p-r-a-v-i-n
p-r-a-v-i-n force-pushed the fix/org-spend-limit-retry branch from 2b21b9c to 23816c0 Compare November 3, 2026 12:37

@czpython czpython left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the patch is gone. Please make three more changes before merge:

  1. Rename phase_resets_at to phase_ends_at and store the wake time. phase is a generic field. The page needs the time when the run tries again, not the time when the provider resets the window. Set the value after the jitter, to datetime.now(UTC) + timedelta(seconds=wait_seconds). Then:

    • The 30-minute fallback also gets a time. Today it sends no reset.
    • _quota_retry_wait can return only the wait again, as on main.
    • Do not use expires_at. In Druks it means that a token, grant, secret or lease stops being valid.
  2. An old reset shows on a later wait. quota_retried resets for each agent call, so one run can wait for quota more than once. If the second wait has no reset, only phase is sent. phase_resets_at keeps the first value, which is already in the past, and the page shows that time. With point 1, every quota wait writes an end time, and an old end time is always in the past. So reads.py can read phase_ends_at while the run is running and return it only when it is later than now. The phase == "waiting_for_quota" check is then not necessary.

  3. A failed call still loses its cost in Codex, Pi and OpenCode. The issue expects every failed call to record its cost. The PR fixes only Claude. Each of the other harnesses raises before write_cost:

    • Codex: check_returncode runs first, and the cost is read after output.json, which a failed run may not have. Read the cost from session.jsonl and write it first. The reader already returns None when the file is missing.
    • Pi: check_returncode runs first. A provider error that exits with zero raises inside the message loop, before the usage sum. Add up the usage and write the cost, then raise. A broken stream needs check_returncode in the except, as in Claude.
    • OpenCode: info.error raises before the cost sum, and check_returncode runs before write_cost. Use the same fix as for Pi.

    Add to the base parse docstring that the cost sidecar is written before a failure raises. Add one cost-before-raise test for each harness, next to the Claude test in test_cost_capture.py.

@p-r-a-v-i-n

This comment was marked as low quality.

@czpython

This comment was marked as resolved.

@p-r-a-v-i-n
p-r-a-v-i-n force-pushed the fix/org-spend-limit-retry branch 2 times, most recently from c0864fb to 54d201d Compare November 3, 2026 14:27

@czpython czpython left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, all three points are fixed. Please make two small changes before merge:

  1. Pi: the usage guard serves only the tests. if message.get("usage") and the two ... if usage else None exist because two error fixtures in test_pi.py send no usage. A real Pi assistant message always has usage. Add usage to those two fixtures, read message["usage"] directly, and remove both conditionals.

  2. reads.py (minor): the second latest and latest.is_running can be phase. You can also remove the reassignment to None and put the check in the SubjectResponse(...) call: phase_ends_at=ends_at if ends_at and ends_at > datetime.now(UTC) else None.

@p-r-a-v-i-n
p-r-a-v-i-n force-pushed the fix/org-spend-limit-retry branch from 54d201d to d32e980 Compare November 3, 2026 15:24
@czpython
czpython self-requested a review November 3, 2026 15:42

@czpython czpython left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good one!

@czpython
czpython merged commit e957528 into czpython:main Nov 3, 2026
6 checks passed
@p-r-a-v-i-n
p-r-a-v-i-n deleted the fix/org-spend-limit-retry branch November 3, 2026 15:44
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.

An org spend limit is retried as a usage window, shows as running, and records no cost

2 participants