Skip to content

fix: report setup admission capacity errors clearly - #195

Draft
evalstate wants to merge 3 commits into
mainfrom
fix/setup-admission-errors
Draft

evalstate wants to merge 3 commits into
mainfrom
fix/setup-admission-errors

Conversation

@evalstate

Copy link
Copy Markdown
Collaborator

Summary

  • Return 503 capacity_exhausted with a clear manual-retry message when the existing setup active-Job admission limit is reached, before runJob is called.
  • Preserve the typed capacity rejection through the Workbench wrapper. Other setup start failures remain sanitized 500 internal_error, without forwarding raw provider exceptions or guessing that a provider failure means capacity exhaustion.
  • Include a service-generated request ID in both error responses and fixed stage/code/request-ID diagnostics. The existing browser API/ErrorNotice path already presents these safely; no UI implementation change is needed.
  • Document the existing accounting inconsistency: setup counts all active namespace Jobs, while parent scheduling counts active parent Jobs. Limits, defaults, accounting, adoption, retries, cancellation, and execution are unchanged.

Harbor boundary review

This is Harbor-HF authenticated admission and HF Job lifecycle error reporting, not Harbor execution behavior.

Checked at pinned Harbor dcd0a7ac74b7bd417780d9cb27cd819c7ec82e4e:

  • src/harbor/models/job/config.py: native JobConfig, trial/agent concurrency and validation remain authoritative and untouched.
  • src/harbor/environments/hf_sandbox.py: HFSandboxEnvironment.start delegates to Sandbox.create; Harbor owns environment lifecycle within a trial, not this control-service setup admission rejection.

Reviewed upstream history through 1f84b4c007788702d9d9b06cfada3c4840103693, including native config validation/dry-run changes in 9a2e3b13 and the pinned-to-head diff for these files. No upstream feature replaces this control-layer error mapping, so no pin change or upstream patch is needed. The existing public Harbor execution/configuration/output boundary remains unchanged; this PR adds no Harbor imports, persisted fields, mirrored configuration, parser, or execution logic.

Validation

  • 562 unit tests passed.
  • 26 browser tests passed, including setup capacity message, code, HTTP status, request ID, and no run submission.
  • Regression coverage includes the configured/default exact boundary, rejection above it, admission below it, unrelated active Jobs, terminal Jobs, adoption at capacity, wrapper identity preservation, rejected-state cleanup, and sanitization of unknown failures.
  • Changed executable statements: 13/13 covered; changed branch outcomes: 12/12 covered.
  • Passed root formatting, lint, typecheck, typecheck:space, build, generated-contract check, and dependency audit (zero vulnerabilities).
  • Passed normal Slophammer check and DRY; privacy checker and full diff reviewed before each commit.

Existing/environmental blockers — no gates weakened

  • Global coverage remains below 85%: lines 79.12%, statements 76.71%, functions 76.68%, branches 70.05%.
  • slophammer-py check . --baseline cannot run: slophammer-baseline.json is absent.
  • The required mutation command cannot run: scripts/check_mutation.py is absent.
  • Root Playwright startup stalled at the IPv4 server probe. The complete suite passed using an isolated IPv6 server and an untracked temporary Playwright harness. No environmental workaround is committed.

No Python behavior changed. No image build/publication, deployment, new setup/benchmark Job, inference, credential transfer, configuration change, or remote runtime mutation was performed. Publication and merge are separate from this local validation.

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.

1 participant