fix: resolve acceptance criteria before handing a deploy to QA#94
Merged
Conversation
The 2026-07-16 mega deployed successfully and its QA run, qa-88a6cc0c, died with qa_no_acceptance_criteria. Repository.acceptance_criteria was only ever written by the architect's update_acceptance_criteria tool, so a story that never went through the architect reached QA with nothing to test — and QA only discovered that after the story had already moved to TESTING and the run existed, leaving partially applied state behind a run that could only error. Make the repository the one source of truth and seed it. POST /api/repositories/ now writes BASELINE_ACCEPTANCE_CRITERIA, so every repository has the one check that holds for any deployed service from the moment it exists; the architect still extends the list as stories add functionality. Story and task criteria describe work to be done and are not what QA runs. Resolve the criteria in the producer, before any state changes. The deploy → QA handoff reads them and carries them on QAMessage, and the consumer no longer looks them up itself — that producer/consumer split is what lost them. Missing criteria now route to the same visible failure as a missing deployed_url or application_id: story failed, admins notified, no TESTING transition and no QA run. Admin run-e2e answers 422 instead of creating a run it cannot finish. The contract rejects blank criteria, which would otherwise parse to "no checks" and reach the agent with nothing to test. Criteria whose every line is a plain "GET <path> returns <status>" are decided over HTTP by the QA consumer, retrying while the service comes up. A health-only story therefore completes with outcome passed and no LLM, and no coding agent is paid for a check that is one request. The mega now gates on the QA run the pipeline produced rather than a health request it issues itself, which proved the service answers but not that QA had concluded anything. run_non_llm_qa moves to pipeline_helpers, where the run observation rules it needs already live: it reads /api/runs/ as an internal service with no user header, and looks the run up by story, since a project can carry QA runs of other stories.
Review found two defects in the non-LLM QA path, both real. The HTTP branch was chosen after _resolve_server_info had already read the application, its server, and the server's SSH private key. A criteria block of plain GET expectations is answered by the deployed URL alone, so a temporarily missing SSH key turned a service answering 200 into a terminal qa_outcome=error without QA ever issuing the request. The same held for bot_username: a tg_bot project's first story carries the seeded health check, so it failed before the architect had written any Telegram criteria. Parse the criteria first and resolve the server, its key, and bot_username only in the agent branch, which is what actually needs them. The health client also followed redirects while the grammar accepts any three-digit status, so the DSL promised an exact status check and broke it in both directions: "GET /old returns 301" could never pass because the client reported the destination's 200, and "returns 200" passed on a path that only redirected to a 200. Check the status the path itself answers.
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
1 participant
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.
Problem
The 2026-07-16 mega deployed successfully and its QA run (
qa-88a6cc0c, storystory-3de09248, reporepo-181548a0) died withqa_no_acceptance_criteria.Repository.acceptance_criteriawas only ever written by the architect'supdate_acceptance_criteriatool, so a story that never went through the architect reached QA with nothing to test. Worse, QA discovered this only after the story had moved to TESTING and the run existed — partially applied state behind a run that could only ever error.What changed
One source of truth, seeded.
POST /api/repositories/now writesBASELINE_ACCEPTANCE_CRITERIA(- GET /health returns 200), the one check that holds for any deployed service. Both repo-creation paths (PO agent,_repo_setup) go through that router, so every repository has criteria from the moment it exists. The architect still extends the list as stories add functionality. Story/task criteria describe work to be done and are not what QA runs.Criteria resolved by the producer, before any mutation. The deploy → QA handoff resolves them and carries them on
QAMessage; the consumer no longer looks them up. That producer/consumer split is what lost them. Missing criteria now route to the same visible failure as a missingdeployed_url/application_id(fail story + notify admins, no TESTING transition, no QA run) — consistent with the existing precondition guard, which this does not regress. Adminrun-e2eanswers 422 rather than creating a run it cannot finish. The contract rejects blank criteria, which would otherwise parse to "no checks" and reach the agent with nothing to test.Health-only criteria decided over HTTP. Criteria whose every line is a plain
GET <path> returns <status>are run by the QA consumer against the deployed URL, retrying while the service comes up. A health-only story completes with outcomepassedand no LLM. One prose line sends the whole block to the agent as before.The mega gates on the pipeline's own QA run. It previously issued its own health request, which proved the service answers, not that QA concluded anything.
run_non_llm_qamoves topipeline_helpers, where the run-observation rules it needs already live — it reads/api/runs/as an internal service with no user header (per #93's ownership narrowing) and looks the run up by story, since a project can carry QA runs of other stories.Testing
shared/tests/test_acceptance.py— parser: baseline is health-only, prose/POST/plain-text criteria fall back to the agent, empty is not "zero checks that pass".test_qa_runner.py—run_health_checksagainst real HTTP semantics via respx: 200 passes, wrong status fails with detail, retries while a service returns 503/connection-refused then 200, unreachable fails after the attempt budget, one failing check fails the run.test_qa_consumer.py— health-only criteria pass without the agent, failing check stores FAILED, prose criteria still reach the agent, criteria come from the message (no repo lookup).test_supervisor_run_routing.py— criteria travel on the message; no repo / null / blank criteria each fail the story with no partial state; the existingapplication_idguard still holds.test_qa_contract.py— criteria required and non-blank.test_admin_actions.py(service test, real DB + Redis) — seeded criteria reachqa:queue; cleared criteria give 422 and create no run.make test-unit(8/8 suites),make test-service SERVICE=api(105 passed),pytest tests/live/test_harness_contract.py(69 passed, the CI step),make lintandruff format --checkclean.Live mega not run from this branch — it needs the real VPS/GitHub path.
Out of scope
Mega deploy-run lookup, LLM in happy-path QA/deploy, repository/story/task model redesign.