Implement external-results case endpoints with test-case auto-upsert, transition enforcement, and traceability linking - #307
Conversation
…ests Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/1a97867b-9a96-45d2-9c4e-da61008b2fb5 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/1a97867b-9a96-45d2-9c4e-da61008b2fb5 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/82714922-40f0-4c88-82c0-a3f345ba338c Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
…eability-test Stabilize e2e traceability checks by isolating CRUD edit/delete targets from seeded rows
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/8873bd6c-5149-43d9-a4ee-00e976f25394 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/8873bd6c-5149-43d9-a4ee-00e976f25394 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/8873bd6c-5149-43d9-a4ee-00e976f25394 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
…test-failures Harden E2E seed loading for test_cases schema drift and add preflight seed integrity check
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/f9994ee2-dcb2-4c82-8b74-1b72a1d405d0 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
…-endpoints External Results audit closure: actor-aware audit model + runner-token write-path logging
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/1b41d391-8b03-4161-9c69-e2a89debf6af Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/1b41d391-8b03-4161-9c69-e2a89debf6af Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/1b41d391-8b03-4161-9c69-e2a89debf6af Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/1b41d391-8b03-4161-9c69-e2a89debf6af Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/1123ad8f-4de3-4e0a-bcb3-d52176bfe2f6 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/3ef84186-557c-465a-ada3-1b031de06c56 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/3ef84186-557c-465a-ada3-1b031de06c56 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/3ef84186-557c-465a-ada3-1b031de06c56 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
… update spec doc Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/2f7fc268-3b43-4776-81a8-8972049d3c89 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
…t test Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/2f7fc268-3b43-4776-81a8-8972049d3c89 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
…-test-coverage Add audit-emission test coverage for External Results write endpoints + spec doc update
…tream 413 abort Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/60342348-2c34-4552-836f-d1aab44ec210 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
…t-parsing feat(#320): convert upload_artifact to true streaming multipart parsing with mid-stream 413 abort
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/49d28a1b-9205-45d8-a3e4-a7a7466668a1 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
…-plugin-react-6 Upgrade frontend to Vite 8 + @vitejs/plugin-react 6 in one coupled update
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/bc1794fc-6e38-4ae9-960a-67d8fef8b317 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
…-to-6-0 chore(frontend): upgrade TypeScript to 6.0.x with no TS6 migration changes required
…system Groups patch and minor updates into a single weekly PR per ecosystem so the backlog doesn't accumulate dozens of individual bump PRs. Major-version bumps are intentionally excluded from every group so they still surface as individual PRs and get deliberate review (e.g. TypeScript 5→6, Vite 7→8). Applies to: pip (backend), npm (frontend), docker, github-actions.
bg-playground
left a comment
There was a problem hiding this comment.
Solid implementation — the auto-upsert logic, transition enforcement, and test coverage all look right. Two must-fix items before this can merge cleanly onto current main.
🛑 1. Multi-head migration
#319 (artifacts, already merged) added migration l1m2n3o4p5q6 chaining off the same ancestor as this PR's i8j9k0l1m2n3. The migration tree now has two heads — the same situation as #305.
Fix: after rebasing onto main, run:
cd backend && alembic merge heads -m "merge case-results and artifacts migrations"Commit the generated merge migration. The resulting revision should have:
down_revision = ("i8j9k0l1m2n3", "l1m2n3o4p5q6")🛑 2. caseoutcome ENUM missing create_type=False — same issue as #317 round 1
The model side:
# backend/app/models/external_results.py (current)
outcome = Column(Enum(CaseOutcome, values_callable=_enum_values), nullable=False)No name= and no create_type=False. SQLAlchemy will attempt CREATE TYPE caseoutcome on Base.metadata.create_all() (test setup, fresh deploys) which conflicts with the migration-created type.
Fix — model side:
outcome = Column(
Enum(CaseOutcome, name="caseoutcome", create_type=False, values_callable=_enum_values),
nullable=False,
)Fix — migration upgrade (before op.create_table):
caseoutcome_enum = sa.Enum(
"passed", "failed", "skipped", "flaky",
name="caseoutcome",
create_type=False,
)
caseoutcome_enum.create(op.get_bind(), checkfirst=True)
op.create_table(
"external_case_results",
...
sa.Column("outcome", caseoutcome_enum, nullable=False),
...
)Fix — migration downgrade:
caseoutcome_enum = sa.Enum(name="caseoutcome", create_type=False)
# drop_table first, then drop the type
op.drop_table("external_case_results")
op.drop_column("test_cases", "auto_registered")
caseoutcome_enum.drop(op.get_bind(), checkfirst=True)This is the exact pattern applied in #319 (artifact_kind enum) — see commit 1 of that PR for the full reference.
Non-blocking observations (don't hold up merge)
Unique constraint vs. idempotency key mismatch. UNIQUE(project_id, external_id) is on the table, but the idempotency lookup uses (session_id, external_id). Two different sessions in the same project submitting the same external_id will: pass the idempotency guard (different session_id) → hit the DB unique constraint → 500. Consider an explicit IntegrityError handler in the POST endpoint, or a follow-up issue.
auto_registered is write-once. It's correctly returned on the 201 response but not stored on the row — subsequent GETs always return False. Fine if intentional (only the creating caller needs to know), but worth a spec note or a follow-up to add the column if GETs should reflect it.
Missing started → passed transition test. The original prompt asked for it; the three covered cases (passed → flaky ✅, passed → failed ✅, flaky → passed ✅) don't include the happy-path started → passed. Low impact — the CRUD logic handles it correctly — but worth adding for completeness.
Once the rebase + merge migration + create_type=False are in, this is a 👍. The CRUD logic, auth wiring, and test coverage are all in good shape.
Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
…ests Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/1a97867b-9a96-45d2-9c4e-da61008b2fb5 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/1a97867b-9a96-45d2-9c4e-da61008b2fb5 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Agent-Logs-Url: https://github.com/bg-playground/BGSTM/sessions/03e9c564-39cd-4091-9b6d-3dfcff810fb2 Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
…base Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Addressed in f0b34d8. I rebased onto current |
bg-playground
left a comment
There was a problem hiding this comment.
Both must-fix items from the earlier review are resolved — this is a 👍 from me.
✅ Must-fix sign-off
Multi-head migration — m2n3o4p5q6r7 correctly merges the two divergent branches (down_revision = ("j9k0l1m2n3o4", "l1m2n3o4p5q6")). Single head confirmed.
case_outcome ENUM create_type=False — model uses Enum(CaseStatus, name="case_outcome", create_type=False) and the migration uses postgresql.ENUM(..., create_type=False) with checkfirst=True. Matches the pattern from #319 exactly.
✅ Bonus items resolved
- #343 (idempotency/constraint mismatch) — constraint is now a partial unique index on
(session_id, external_id) WHERE external_id IS NOT NULL, which correctly aligns with the CRUD lookup. Closing that issue. - #320 (real streaming enforcement) —
_SizeLimitedFileTarget.on_data_receivedraises_SizeLimitExceededmid-stream; therequest.stream()loop exits immediately. No full-body buffering. auto_registeredtransient on GET — column is now persisted on the row. GETs return the correct value.
Note on scope
This PR now covers more ground than #303 alone (artifact upload, audit log actor_kind refactoring, Projects API, requirement_external_ids, smoke workflow, Dependabot grouping). All of it is in a clean, passing state. Fine to merge as-is; just noting for the changelog.
One nit (no action needed)
flaky → flaky is allowed by _is_transition_allowed. Consistent with the spec's "same outcome is a no-op" rule — confirming it's intentional.
13/13 CI checks green, branch is clean against main. Ready to merge. 🚀
Original prompt
Implement the case-result endpoints in
external_results.py, per the merged spec (docs/specs/external_results_v1.md) and the Pydantic schemas already inbackend/app/schemas/external_results.py.Tracking: This PR closes BGSTM#303, parented under BGSTM#291.
Depends on (already merged):
get_current_runner_token,require_runner_scope)ExternalRunSessionmodel,external_results.pyrouter file)Scope (this PR only)
Add per-test-case result endpoints with auto-upsert behavior. Artifacts (#298) and audit log integration (#297) are explicitly out of scope.
Endpoints
POST /api/v1/external-results/case— submit a case result (auto-upserts test case if needed)PATCH /api/v1/external-results/case/{id}— update status / re-classifyGET /api/v1/external-results/case/{id}— readFiles to add
backend/app/models/external_results.py— extend withExternalCaseResultSQLAlchemy model. Columns:idUUID PK,session_idFK →external_run_sessions,test_case_idFK →test_cases(nullable for upsert path),external_id(nullable, unique withproject_id),title,outcomeenum,duration_ms,error_messagenullable,created_at,updated_at. Add appropriate indexes (session_id,(project_id, external_id)).backend/app/crud/external_results.py— extend withcreate_case_result,update_case_result,get_case_resulthelpers. Auto-upsert lookup logic lives here.backend/alembic/versions/<rev>_add_external_case_results.py— migration. CRITICAL: runcd backend && alembic headsfirst to find the actual current head and chaindown_revisionto it. Do NOT guess or copy from older migration comments — that caused the multi-head error in feat: machine-token auth flow for external runners (BGSTM#296) #305.backend/tests/integration/test_external_results_case.py— full integration test file.Files to modify
backend/app/api/external_results.py— add the 3 new endpoints alongside the existing session endpoints.backend/app/models/test_cases.py(or wherever theTestCasemodel lives) — addauto_registered: bool = Falsecolumn only if it does not already exist. If it exists, skip. If you need to add it, include the column in the same migration above.Required behavior
Auto-upsert rules (POST /case):
test_case_idis supplied and exists → link result to it.test_case_idis supplied but doesn't exist → 404 with codetest_case_not_found.external_idis supplied → lookup(project_id, external_id)intest_cases. Found → link. Not found → create a newTestCaserow withauto_registered=trueand link.missing_identifier.Traceability auto-link:
If
requirement_ids: list[UUID]is provided in the payload, write entries into the existingtraceabilitytable (seebackend/app/api/traceability.py) linking each requirement to the test case. Idempotent — duplicate inserts must not error or create duplicate rows. Use the existing CRUD/model patterns.Status transition rules (enforced on PATCH):
started→ any (no restriction)passed/failed/skipped→ onlyflaky(re-classification allowed once)aborted→ terminal, no transitions allowedinvalid_status_transitionand a message describing both the current and attempted statuses.Auth:
external_results:writescope viarequire_runner_scopefrom [Auth] Machine-token flow for external runners #296.Idempotency:
Per the spec, duplicate POSTs with the same
(session_id, external_id)collapse to the same case-result row (return existing row, not 409, not duplicate).Audit:
Do NOT wire audit writes. Leave clear
# TODO(BGSTM#297): write_audit(...)comments at every state-changing path so #297's audit pass can find them. Match the placement convention from #300.Tests (required)
In
backend/tests/integration/test_external_results_case.py:Auto-upsert (4 scenarios):
test_case_id→ links to existing test casetest_case_id→ 404test_case_not_foundexternal_idmatching existing test case → links, no new rowexternal_idnot matching → createsTestCasewithauto_registered=true, links resultmissing_identifierStatus transitions (3+ scenarios):
started→passedsucceedspassed→flakysucceeds (re-classification)passed→failedreturns 409invalid_status_transitionaborted→ anything returns 409invalid_status_transitionTraceability auto-link:
requirement_ids: [r1, r2]creates 2traceabilityrows linking the test case to each requirementrequirement_idsdoes not create duplicate rows (idempotent)Auth:
This pull request was created from Copilot chat.