fix: an app answers for itself, and an item belongs to its project - #137
Merged
Conversation
Three API-contract defects, all found by reading the surface rather than running it. **One app's wiring is no longer another's (#106).** `create_api` copied its fleet, model client, session host and probes into a module-level dictionary, so a second app in the same process silently took over the first's readiness gate: app A could report app B's worker pool and probe results while its own top-level fields still said otherwise. Preflight now reads the calling app's `app.state`. A gate an unrelated app can change is not a gate. **Latest events are scoped to their project (#109).** `(project_id, item_id)` is an item's identity and two projects each having a `T1` is supported, but the projection keyed on the item id alone, so one project's newest event was reported as the other's -- including the session deep link, which would put a human in the wrong project's terminal. **The inception routes publish a schema (#111).** Both answered `response_model=dict`, which generates `additionalProperties: true` and documents nothing. `InceptionDraft` and `InceptionPlan` name the fields a client actually needs. Also pins #108, already fixed by #120 but never covered: an `exhausted` item is the one an operator most needs to look at, and both work routes used to 500 on it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #106. Closes #109. Closes #111. Closes #108.
#106 — preflight wiring leaked across
create_apiinstancescreate_apicopied fleet, model client, session host and probes into amodule-level
_APP_STATE. Every preflight/readiness call read that global, sobuilding a second app overwrote the first's wiring — app A could report app
B's worker pool, session host and probe results, while its own top-level
readiness fields still came from its own
app.state. A response could saymode: monitoring-onlywhile every project claimedready_to_start: true._preflightnow takes the calling app'sapp.state. The global is gone.Test builds two apps deliberately — A with no fleet and healthy probes, B with
a fleet and failing probes, B constructed second because that is the one
that used to win — and asserts A answers for itself.
#109 — latest events collided across projects
(project_id, item_id)is an item's identity, and two projects each having aT1is explicitly supported._latest_by_itemkeyed on the item id alone, sowhichever project wrote most recently supplied
latestfor both. Worst partis the deep link:
session_idpointed at the other project's terminal.Now scoped, in the audit query and the ingest-store fallback alike.
#111 — inception routes documented nothing
POST /api/inceptionandGET /api/inception/{id}/planboth declaredresponse_model=dict, which emitsadditionalProperties: true. A generatedclient could not discover
state,project_idormarkdown. NowInceptionDraftandInceptionPlan, every field described.The test asserts against
/openapi.jsonrather than the handler, since theschema is the artefact that was wrong.
#108 — already fixed, never covered
WorkStategainedexhaustedin #120, so the 500s are gone. There was notest, which is how it was missed in the first place. Added one: claim to the
attempt ceiling, then assert both
GET /api/workandGET /api/work/{id}answer 200 and report the state.
All four regression tests fail without their respective fix. Suite, ruff and
mypy .green.