diff --git a/README.md b/README.md index 1cdbab888..8e075212c 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,7 @@ Workstream turns that operating knowledge into reusable infrastructure. - [Chunk 7 Checker Runner And Registry](docs/spec_chunk_7_checker_runner_registry.md) - [Chunk 8 Evidence And Policy Checkers](docs/spec_chunk_8_evidence_policy_checkers.md) - [Chunk 9 Pre-Review Gate](docs/spec_chunk_9_pre_review_gate.md) +- [Chunk 10 Checker Trial](docs/spec_chunk_10_checker_trial.md) - [Day-by-Day Execution Plan](docs/roadmap_day_by_day_execution_plan.md) - [Implementation Backlog](docs/roadmap_implementation_backlog.md) - [Product Principles](docs/product_principles.md) diff --git a/backend/tests/test_checkers.py b/backend/tests/test_checkers.py index 678e6aa98..ae0107ad5 100644 --- a/backend/tests/test_checkers.py +++ b/backend/tests/test_checkers.py @@ -161,6 +161,52 @@ async def lock_submission_and_get_auto_run( return locked.json(), runs[0] +async def create_checker_trial_project( + client: AsyncClient, + slug: str, + required_checkers: list[str] | None = None, +) -> dict: + """Create and activate a project guide for one checker trial scenario. + + Args: + client: API client using the current project manager actor. + slug: Unique project slug for this scenario. + required_checkers: Optional locked required checker policy names. + + Returns: + Created project response payload. + """ + project_response = await client.post( + "/api/v1/projects", + headers=auth_headers(), + json={ + "name": slug.replace("-", " ").title(), + "slug": slug, + "description": "Project for the Chunk 10 checker trial.", + "base_amount": "25.00", + "currency": "USD", + }, + ) + assert project_response.status_code == 201, project_response.text + project = project_response.json() + + guide_payload = complete_guide_payload() + if required_checkers is not None: + guide_payload["checker_policy"]["required_checkers"] = required_checkers + guide_response = await client.post( + f"/api/v1/projects/{project['id']}/guides", + headers=auth_headers(), + json=guide_payload, + ) + assert guide_response.status_code == 201, guide_response.text + activation_response = await client.post( + f"/api/v1/projects/{project['id']}/guides/{guide_response.json()['id']}/activate", + headers=auth_headers(), + ) + assert activation_response.status_code == 200, activation_response.text + return project + + async def test_pre_submit_check_returns_feedback_without_durable_run( checker_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, @@ -713,6 +759,213 @@ async def test_chunk8_task_setup_blocked_takes_priority_over_worker_revision( assert task.status == "review_pending" +async def test_chunk10_checker_trial_runs_sample_submissions_through_real_api( + checker_client: AsyncClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + trial_cases = [ + { + "slug": "chunk10-clean-packet", + "worker_subject": "chunk10-worker-clean", + "payload": complete_submission_payload(), + "route": "allow_review", + "task_status": "review_pending", + "checker_name": "check_submission_packet", + "checker_status": "passed", + "worker_route": "allow_review", + }, + { + "slug": "chunk10-missing-required-file", + "worker_subject": "chunk10-worker-missing-file", + "payload": { + **complete_submission_payload(), + "artifact_hash_manifest": [ + { + "artifact": "other.md", + "hash": "sha256:other-v1", + "size_bytes": 128, + "notes": "wrong artifact", + } + ], + }, + "route": "needs_revision", + "task_status": "needs_revision", + "checker_name": "check_required_files", + "checker_status": "failed", + "worker_route": "needs_revision", + }, + { + "slug": "chunk10-forbidden-file-path", + "worker_subject": "chunk10-worker-forbidden-file", + "payload": { + **complete_submission_payload(), + "artifact_hash_manifest": [ + *complete_submission_payload()["artifact_hash_manifest"], + { + "artifact": "secrets/.env", + "hash": "sha256:env-v1", + "size_bytes": 64, + "notes": "must be removed", + }, + ], + }, + "route": "needs_revision", + "task_status": "needs_revision", + "checker_name": "check_forbidden_files", + "checker_status": "failed", + "worker_route": "needs_revision", + }, + { + "slug": "chunk10-weak-confidentiality", + "worker_subject": "chunk10-worker-attestation", + "payload": { + **complete_submission_payload(), + "worker_attestation": "ok", + }, + "route": "needs_revision", + "task_status": "needs_revision", + "checker_name": "check_confidentiality_attestation", + "checker_status": "failed", + "worker_route": "needs_revision", + }, + ] + + for case in trial_cases: + set_dev_actor(monkeypatch, roles="project_manager", subject="project-manager-subject") + project = await create_checker_trial_project(checker_client, case["slug"]) + started_task = await create_started_task( + checker_client, + project["id"], + monkeypatch, + subject=case["worker_subject"], + ) + created = await checker_client.post( + f"/api/v1/tasks/{started_task['id']}/submissions", + headers=auth_headers(), + json=case["payload"], + ) + assert created.status_code == 201, created.text + + set_dev_actor(monkeypatch, roles="project_manager", subject="project-manager-subject") + _, manager_run = await lock_submission_and_get_auto_run( + checker_client, + created.json()["id"], + ) + assert manager_run["routing_recommendation"] == case["route"] + target_result = next( + result + for result in manager_run["results"] + if result["checker_name"] == case["checker_name"] + ) + assert target_result["status"] == case["checker_status"] + + async with db_session.get_session_factory()() as session: + task = await session.get(WorkstreamTask, started_task["id"]) + assert task is not None + assert task.status == case["task_status"] + + set_dev_actor(monkeypatch, roles="worker", subject=case["worker_subject"]) + worker_read = await checker_client.get( + f"/api/v1/checker-runs/{manager_run['id']}", + headers=auth_headers(), + ) + assert worker_read.status_code == 200, worker_read.text + worker_body = worker_read.json() + assert worker_body["routing_recommendation"] == case["worker_route"] + worker_result = next( + result + for result in worker_body["results"] + if result["checker_name"] == case["checker_name"] + ) + assert worker_result["status"] == case["checker_status"] + assert worker_result["metadata"] == {} + if case["route"] == "needs_revision": + assert worker_result["worker_message"] + assert worker_result["worker_suggested_fix"] + if case["checker_name"] == "check_forbidden_files": + assert ".env" not in worker_read.text + assert "secrets/" not in worker_read.text + assert "local://" not in worker_read.text + + set_dev_actor(monkeypatch, roles="project_manager", subject="project-manager-subject") + project = await create_checker_trial_project( + checker_client, + "chunk10-task-setup-defect", + required_checkers=["check_acceptance_criteria_present"], + ) + started_task = await create_started_task( + checker_client, + project["id"], + monkeypatch, + subject="chunk10-worker-task-setup", + ) + created = await checker_client.post( + f"/api/v1/tasks/{started_task['id']}/submissions", + headers=auth_headers(), + json=complete_submission_payload(), + ) + assert created.status_code == 201, created.text + + async with db_session.get_session_factory()() as session: + task = await session.get(WorkstreamTask, started_task["id"]) + assert task is not None + task.acceptance_criteria = None + await session.commit() + + set_dev_actor(monkeypatch, roles="project_manager", subject="project-manager-subject") + _, blocked_run = await lock_submission_and_get_auto_run( + checker_client, + created.json()["id"], + ) + assert blocked_run["routing_recommendation"] == "task_setup_blocked" + setup_result = next( + result + for result in blocked_run["results"] + if result["checker_name"] == "check_acceptance_criteria_present" + ) + assert setup_result["status"] == "failed" + assert setup_result["worker_visible"] is False + + async with db_session.get_session_factory()() as session: + task = await session.get(WorkstreamTask, started_task["id"]) + assert task is not None + assert task.status == "auto_checking" + + set_dev_actor(monkeypatch, roles="worker", subject="chunk10-worker-task-setup") + worker_blocked_read = await checker_client.get( + f"/api/v1/checker-runs/{blocked_run['id']}", + headers=auth_headers(), + ) + assert worker_blocked_read.status_code == 200, worker_blocked_read.text + assert worker_blocked_read.json()["routing_recommendation"] == "not_evaluated" + assert worker_blocked_read.json()["results"] == [] + assert "task_setup_blocked" not in worker_blocked_read.text + assert "acceptance_criteria" not in worker_blocked_read.text + + async with db_session.get_session_factory()() as session: + task = await session.get(WorkstreamTask, started_task["id"]) + assert task is not None + task.acceptance_criteria = "Worker output must satisfy the project rubric." + await session.commit() + + set_dev_actor(monkeypatch, roles="project_manager", subject="project-manager-subject") + retry = await checker_client.post( + f"/api/v1/submissions/{created.json()['id']}/checker-runs", + headers=auth_headers(), + json={"trigger_reason": "task setup repaired during Chunk 10 trial"}, + ) + assert retry.status_code == 200, retry.text + retry_body = retry.json() + assert retry_body["attempt_number"] == 2 + assert retry_body["supersedes_checker_run_id"] == blocked_run["id"] + assert retry_body["routing_recommendation"] == "allow_review" + assert retry_body["trigger_source"] == "manual_checker_trigger" + async with db_session.get_session_factory()() as session: + task = await session.get(WorkstreamTask, started_task["id"]) + assert task is not None + assert task.status == "review_pending" + + async def test_worker_can_read_only_worker_visible_checker_result_fields( checker_client: AsyncClient, monkeypatch: pytest.MonkeyPatch, diff --git a/docs/checker_trial_failure_catalog.md b/docs/checker_trial_failure_catalog.md new file mode 100644 index 000000000..9ad4633e9 --- /dev/null +++ b/docs/checker_trial_failure_catalog.md @@ -0,0 +1,50 @@ +# Checker Trial Failure Catalog + +## Purpose + +This catalog records the Chunk 10 checker trial outcomes. It is the audit note for the first real sample submissions run through the Week 2 checker framework. + +The trial does not add a lifecycle state, a review decision, or a frontend surface. It proves the current backend contracts for clean submissions, worker-fixable checker failures, internal task setup failures, and trusted checker retry. + +## Trial Results + +| Scenario | Sample Packet Shape | Primary Checker | Routing Recommendation | Task Status | Worker Visibility | Project Manager/Admin API Visibility | +| --- | --- | --- | --- | --- | --- | --- | +| Clean packet | Valid summary, package hash, `answer.md`, evidence, and confidentiality attestation | `check_submission_packet` plus required structural checks | `allow_review` | `review_pending` | Passing worker-visible checker context | Full checker run and result rows through backend API | +| Missing required file | Artifact manifest omits `answer.md` | `check_required_files` | `needs_revision` | `needs_revision` | Required-file message and suggested fix | Full checker result with missing file metadata | +| Forbidden file path | Artifact manifest includes a forbidden path pattern | `check_forbidden_files` | `needs_revision` | `needs_revision` | Generic forbidden-file message without raw sensitive path leakage | Full checker result with forbidden category metadata | +| Weak confidentiality attestation | Attestation is too short and generic | `check_confidentiality_attestation` | `needs_revision` | `needs_revision` | Attestation fix message | Full checker result with failed attestation fields | +| Locked task setup defect | Task loses reviewable acceptance criteria after screening | `check_acceptance_criteria_present` | `task_setup_blocked` | `auto_checking` until repair | Hidden from worker as `not_evaluated` with no result rows | Full internal checker route and blocked audit event | + +## Routing Notes + +Worker-fixable submission failures use `needs_revision`. The worker can see the relevant checker result, worker message, and suggested fix. + +Locked task setup failures use `task_setup_blocked`. This is an internal checker routing recommendation for project-manager repair. It is not a task status, not a review decision, and not a worker-facing revision request. + +Trusted checker retry is allowed after internal repair. The repair path is: + +```text +task_setup_blocked +-> project manager repairs task setup +-> trusted checker retry +-> allow_review +-> review_pending +``` + +## False-Positive Notes + +- `check_low_quality_generated_artifacts` is warning-only in the current trial because simple placeholder wording can be legitimate during early task work. +- `check_forbidden_files` intentionally reports a generic worker message. A forbidden path hit could be a false positive if a project intentionally requires a file with a sensitive-looking name, but the safe default is to block and require project-manager clarification. +- `check_confidentiality_attestation` is deterministic and text-based. It can reject a sincere but short attestation; this is acceptable for v0.1 because the required wording is part of the submission contract. + +## Missing-Checker Notes + +- No semantic answer-quality checker exists yet. Human review still owns correctness. +- No external execution sandbox checker exists yet. Week 2 only records structural and policy-context checks. +- No reputation, ERC-8004 identity, x402, escrow, or payment settlement checker exists in v0.1. +- No project-specific custom checker execution worker exists yet. The current registry is in-process and deterministic. + +## Evidence + +The API trial is covered by `test_chunk10_checker_trial_runs_sample_submissions_through_real_api` in `backend/tests/test_checkers.py`. diff --git a/docs/internal_reviews/2026-06-12_chunk10_checker_trial.md b/docs/internal_reviews/2026-06-12_chunk10_checker_trial.md new file mode 100644 index 000000000..dccb3bd18 --- /dev/null +++ b/docs/internal_reviews/2026-06-12_chunk10_checker_trial.md @@ -0,0 +1,117 @@ +# Internal Review: Chunk 10 Checker Trial + +## Scope + +Chunk 10 adds the checker trial specification, failure catalog, and API-level trial coverage for five sample submission scenarios. + +Changed contract surfaces: + +- `docs/spec_chunk_10_checker_trial.md` +- `docs/checker_trial_failure_catalog.md` +- `docs/spec_week2_checker_framework.md` +- `docs/roadmap_day_by_day_execution_plan.md` +- `README.md` +- `backend/tests/test_checkers.py` + +## Verifier Results + +### Senior engineering + +Finding: the first draft overclaimed reviewer checker-output visibility even though current checker read access is limited to admin, project manager, and assigned worker. + +Resolution: corrected Chunk 10 wording to trusted internal and project-manager/admin API visibility. Reviewer visibility remains a Week 3 contract. + +Verdict after fix: no lifecycle expansion, no new review decision, worker-fixable failures route to `needs_revision`, and internal setup defects remain `task_setup_blocked`. + +### QA/test + +Finding: same reviewer-visibility wording issue; test coverage itself matched current backend contracts. + +Resolution: corrected the docs and kept the trial as real API coverage. + +Verified coverage: + +- clean packet reaches `review_pending` +- missing required file routes to `needs_revision` +- forbidden file path routes to `needs_revision` without worker path leakage +- weak confidentiality attestation routes to `needs_revision` +- locked task setup defect routes to internal `task_setup_blocked` +- trusted checker retry after repair reaches `review_pending` + +### Security/auth + +Finding: the original catalog visibility column implied broader internal checker access than the backend allows. + +Resolution: renamed the column to `Project Manager/Admin API Visibility` and kept worker redaction assertions in the trial test. + +Verified boundaries: + +- test auth remains dev-only fixture usage +- no production Flow auth behavior changed +- worker responses hide `task_setup_blocked` +- worker responses hide internal task setup field names and metadata +- forbidden path details do not leak through worker-visible messages + +### Product/ops + +Findings: + +- normalize `needs_revision` wording where Chunk 10 touched Week 2 docs +- use `trusted checker retry` as the canonical phrase +- commit the new Chunk 10 docs with the README link + +Resolution: normalized wording and kept the operator/worker mental model: workers see `needs_revision`; project-manager-owned setup defects stay internal. + +## Validation + +Passed: + +```bash +cd backend && .venv/bin/python -m ruff check app tests scripts +``` + +Passed: + +```bash +cd backend && WORKSTREAM_DATABASE_URL=postgresql+asyncpg://workstream:workstream@localhost:5433/workstream .venv/bin/python -m pytest tests/test_checkers.py::test_chunk10_checker_trial_runs_sample_submissions_through_real_api -q +``` + +Result: `1 passed in 23.20s`. + +Passed: + +```bash +cd backend && WORKSTREAM_DATABASE_URL=postgresql+asyncpg://workstream:workstream@localhost:5433/workstream .venv/bin/python -m pytest tests/test_checkers.py -q +``` + +Result: `23 passed in 150.27s`. + +Passed: + +```bash +cd backend && WORKSTREAM_DATABASE_URL=postgresql+asyncpg://workstream:workstream@localhost:5433/workstream .venv/bin/python -m pytest tests/test_tasks.py tests/test_checkers.py -q +``` + +Result: `62 passed in 406.92s`. + +Passed: + +```bash +cd backend && WORKSTREAM_DATABASE_URL=postgresql+asyncpg://workstream:workstream@localhost:5433/workstream .venv/bin/python -m pytest -q +``` + +Result: `112 passed in 695.21s`. + +Passed: + +```bash +cd backend && .venv/bin/docstr-coverage --config .docstr.yaml +``` + +Result: `100.0%`. + +## Closure + +Valid findings addressed. + +Open sub-agent sessions: none. diff --git a/docs/roadmap_day_by_day_execution_plan.md b/docs/roadmap_day_by_day_execution_plan.md index f7d5ef81a..dc1fffb08 100644 --- a/docs/roadmap_day_by_day_execution_plan.md +++ b/docs/roadmap_day_by_day_execution_plan.md @@ -134,9 +134,9 @@ Week 2 is backend-first checker infrastructure. Checker output is exposed throug The core invariant is: -`Draft packet -> Pre-submit checks -> Submit -> Lock -> Internal CheckerRun -> CheckerResults -> REVIEW_PENDING or NEEDS_REVISION` +`Draft packet -> Pre-submit checks -> Submit -> Lock -> Internal CheckerRun -> CheckerResults -> review_pending or needs_revision` -The checker framework does not accept or reject work. It may route worker-fixable checker failures to user-facing `NEEDS_REVISION`, but that does not create a human review decision. Internally the source is recorded as `auto_checker`. +The checker framework does not accept or reject work. It may route worker-fixable checker failures to user-facing `needs_revision`, but that does not create a human review decision. Internally the source is recorded as `auto_checker`. ### Day 6: Checker Interface @@ -169,9 +169,9 @@ Deliver: Exit criteria: - worker-fixable submission failures fail before review -- high severity failures block `REVIEW_PENDING` +- high severity failures block `review_pending` - checker runs bind to the exact submission id, submission version, package hash, and artifact hash manifest -- worker-fixable checker failures route to user-facing `NEEDS_REVISION` +- worker-fixable checker failures route to user-facing `needs_revision` ### Day 8: Evidence And Acceptance Checkers @@ -211,7 +211,7 @@ Exit criteria: Deliver: - 5 sample submissions -- checker failure catalog +- [checker failure catalog](checker_trial_failure_catalog.md) - false-positive notes - missing-checker list @@ -259,7 +259,7 @@ Exit criteria: Deliver: -- `REVIEW_PENDING -> NEEDS_REVISION` +- `review_pending -> needs_revision` - feedback history - task unlock for worker - resubmission requirements diff --git a/docs/spec_chunk_10_checker_trial.md b/docs/spec_chunk_10_checker_trial.md new file mode 100644 index 000000000..b62c7ed15 --- /dev/null +++ b/docs/spec_chunk_10_checker_trial.md @@ -0,0 +1,78 @@ +# Chunk 10 Checker Trial + +## Purpose + +Chunk 10 proves the Week 2 checker framework against real sample submission flows. + +This chunk does not add a new lifecycle state or review decision. It exercises the existing API contracts from project guide activation through task submission, lock, automatic checker run, task routing, worker-visible feedback, and trusted checker retry from an internal blocked gate. + +## Scope + +- sample submissions that pass and fail through the backend API +- failure catalog for checker outcomes +- false-positive notes +- missing-checker notes +- trusted checker retry documentation for internal blocked gate repair +- tests that prove trusted internal and worker-visible checker output boundaries + +## Non-Scope + +- product frontend implementation +- human review decision records +- revision replay enforcement +- contribution records +- payment records +- reputation records +- external checker worker infrastructure +- new checker names unless the trial proves a missing checker is required + +## Trial Matrix + +The trial must include at least five sample submissions: + +| Scenario | Expected Routing | Expected Task Status | Worker Visible | +| --- | --- | --- | --- | +| Clean packet | `allow_review` | `review_pending` | Passing checker context | +| Missing required file | `needs_revision` | `needs_revision` | Required-file fix message | +| Forbidden file path | `needs_revision` | `needs_revision` | Safe forbidden-file fix message | +| Weak confidentiality attestation | `needs_revision` | `needs_revision` | Attestation fix message | +| Locked task setup defect | `task_setup_blocked` | `auto_checking` | Internal route hidden | + +The worker-facing output must keep the same public language as the rest of Workstream. Worker-fixable checker failures are `needs_revision`. Internal setup defects stay hidden from workers and are repaired by a project manager before a trusted checker retry. + +The locked task setup defect must also prove trusted repair: + +```text +task_setup_blocked +-> project manager repairs task setup +-> trusted checker retry +-> allow_review +-> review_pending +``` + +## Conditions Of Satisfaction + +- at least one clean submission reaches `review_pending` +- at least one worker-fixable submission failure reaches `needs_revision` +- locked task setup failures and worker-fixable failures use distinct routing recommendations +- worker-visible responses do not expose internal `task_setup_blocked` routing +- trusted checker retry from an internal blocked gate is covered +- false-positive notes are written down +- missing-checker notes are written down +- failure catalog links every trial scenario to the checker that produced the route + +## Evidence + +Trial evidence is stored in: + +- [Checker Trial Failure Catalog](checker_trial_failure_catalog.md) +- backend API integration tests for the sample matrix + +The trial test must use real backend API calls for project creation, guide activation, task screening/release, worker claim/start, submission creation, submission locking, checker run reads, and trusted checker retry. Direct database setup is allowed only to create the controlled locked task setup defect that normal lifecycle guards are designed to prevent. + +## Verifier Agents + +- senior engineering +- QA/test +- security/auth +- product/ops diff --git a/docs/spec_chunk_9_pre_review_gate.md b/docs/spec_chunk_9_pre_review_gate.md index e1ad8c094..541c2fe05 100644 --- a/docs/spec_chunk_9_pre_review_gate.md +++ b/docs/spec_chunk_9_pre_review_gate.md @@ -49,7 +49,7 @@ Route outcomes: - `allow_review`: task moves to `review_pending` - `needs_revision`: task moves to user-facing `needs_revision` - `task_setup_blocked`: task remains in `auto_checking` for project-manager repair -- `checker_retry`: task remains in `auto_checking` until a trusted retry or repair happens +- `checker_retry`: task remains in `auto_checking` until a trusted checker retry or repair happens `task_setup_blocked` and `checker_retry` are internal checker routing recommendations. They are not review decisions and they are not worker-facing task outcomes. diff --git a/docs/spec_week2_checker_framework.md b/docs/spec_week2_checker_framework.md index 27110c62f..3cb3a4bc1 100644 --- a/docs/spec_week2_checker_framework.md +++ b/docs/spec_week2_checker_framework.md @@ -179,7 +179,7 @@ Conditions of satisfaction: ### Chunk 9: Pre-Review Gate -Automatically triggers internal post-submit checks after submission locking and calculates whether a checked submission moves to `REVIEW_PENDING`, user-facing `NEEDS_REVISION`, or an internal `task_setup_blocked` repair route. +Automatically triggers internal post-submit checks after submission locking and calculates whether a checked submission moves to `REVIEW_PENDING`, user-facing `needs_revision`, or an internal `task_setup_blocked` repair route. Detailed spec: [Chunk 9 Pre-Review Gate](spec_chunk_9_pre_review_gate.md). @@ -195,6 +195,8 @@ Conditions of satisfaction: Runs real sample submissions through the checker framework. +Detailed spec: [Chunk 10 Checker Trial](spec_chunk_10_checker_trial.md). + Conditions of satisfaction: - at least one clean submission reaches `REVIEW_PENDING`