Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ they cross multiple L1 boundaries.
| Chunk | Goal | Risk | Entry gate/status |
|---|---|---:|---|
| `WS-ART-001-PLAN3` | Reconcile the complete remaining v0.1 custody chain and AUTH/REV/CON handoffs. | L1 | Planning only; proposed |
| `WS-ART-001-03C` | Clean-cut legacy guide identity/excerpts and make the verified same-generation pipeline live. | L1 | Proposed; AUTH-04B entry gate satisfied by PR #245 |
| `WS-ART-001-04A1` | Remove legacy multi-step contributor intake reachability and schema without adding the replacement route. | L1 | Proposed after 03C |
| `WS-ART-001-03C` | Clean-cut legacy guide identity/excerpts and make the verified same-generation pipeline live. | L1 | Merged PR #249 |
| `WS-ART-001-04A1` | Remove legacy multi-step contributor intake reachability and schema without adding the replacement route. | L1 | Implemented; internal review passed; external PR gates pending |
| `WS-ART-001-04A2` | Add bounded one-outer-ZIP intake and archive-safety inspection in private scratch. | L1 | Proposed after 04A1 |
| `WS-ART-001-04A3` | Add canonical semantic manifest, executable normalization, and unchanged-work gate. | L1 | Proposed after 04A2 |
| `WS-ART-001-04B` | Run non-bypassable platform and locked-guide prechecks against that exact scratch tree and persist bounded evidence. | L1 | Proposed after 04A3 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ capability, browser-to-provider path, or client provider credential.

## D6 - Verification Before Binding

Provider acknowledgement sets the upload item to
`stored_pending_verification` and creates a pending replica, never a binding.
Provider acknowledgement sets the durable put attempt to `object_confirmed`
and creates a pending replica, never a binding.
Celery independently reads and hashes the complete object. Only a matching
object becomes bindable.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,3 +184,62 @@ binding, checker materialization/output custody, reviewer packet byte access,
and accepted-contribution identity projection. REV owns review lifecycle and
notes/findings; CON owns ContributionRecord; client delivery remains a future
owner and is not silently implemented by ART v0.1.

## 2026-08-04 ART-04A1 Legacy Contributor Intake Discovery

Observations on merged `main` at `2feaf47d`:

- No HTTP route currently exposes upload-session or upload-item creation, but
`ArtifactUploadSession` and `ArtifactUploadItem` remain active SQLAlchemy
metadata in `app/modules/artifacts/models.py` and `app/db/models.py`.
- `ContributorArtifactAdmissionRequest` in
`app/modules/artifacts/schemas.py` remains an internal command that accepts
caller authorization plus an upload-item id. `ArtifactAdmissionService`
still dispatches that type through `_contributor_facts`, so the obsolete
contributor intake remains reachable to internal callers even without an
HTTP route.
- `ArtifactRepository` still exposes upload-item/session locks, contributor
relationship lookup, and upload-item receipt lookup. These methods are used
only by the retired contributor path and compatibility state projection.
- Shared put recovery and verification still conditionally mutate an upload
item when `ArtifactPutAttempt.upload_item_id` is present. Removing the two
ledgers therefore also requires removing those compatibility mutations while
preserving guide and checker-output recovery.
- `ArtifactPutAttempt.upload_item_id` and
`ArtifactOperationReceipt.upload_item_id` still foreign-key the legacy item
table. Receipt contract version 1 identifies historical acknowledgements only
through `upload_item_id`; version 2 may also carry it for contributor puts.
Historical audit values cannot remain readable if the column is dropped.
- Existing migration tests already seed populated legacy sessions/items,
attempts, and receipts and exercise database invariants. ART-04A1 needs a new
head migration that refuses unsafe populated cutover instead of fabricating a
replacement identity or silently deleting evidence.
- AUTH has already removed the obsolete action identifiers from the active
catalogue and service matrix; `tests/test_authorization.py` retains the
deterministic historical-only proof. ART must not edit AUTH availability or
create aliases.

Implementation constraints derived from the current code:

- Remove the contributor admission request variant, service dispatch, repository
relationship methods, upload-ledger models, and conditional upload-item state
projection from shared recovery/verification.
- Do not detach or preserve nullable `upload_item_id` compatibility columns.
Refuse any populated historical reference before dropping the columns and
ledgers, leaving that deployment unchanged for a separate maintenance decision.
- The migration must fail closed when rows exist whose deletion would discard
non-represented contributor state. Upgrade/downgrade behavior and the exact
safe-empty condition require plan-review approval before implementation.
- No replacement route, ZIP parser, scratch orchestration, provider write, AUTH
action activation, Submission, checker, or review behavior belongs to 04A1.

Plan-review resolution:

- 04A1 is a complete safe-empty clean cut. It takes exclusive locks and refuses
atomically when any session/item row, contributor put attempt, contract-v1
receipt, or non-null upload-item reference exists. Refusal preserves the old
schema and all historical identifiers for a separately approved maintenance
decision. A successful upgrade therefore removes the contributor columns and
ledger tables completely; it does not retain detached compatibility fields.
- Downgrade recreates only the exact empty legacy schema proven by the upgrade
precondition. It never fabricates a session/item lineage from newer facts.
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,7 @@ durable acknowledgement-unknown attempt rather than committing a terminal fact.
7. Transaction B records provider acknowledgement, completes the provisional
admission charges, sets the `ArtifactPutAttempt` to `object_confirmed`, and
creates the replica with pending verification and unknown
availability/integrity; while a legacy contributor upload item exists, it
alone moves to `stored_pending_verification`. No binding exists.
availability/integrity. No binding exists.
8. A durable verification job is committed in PostgreSQL and published to
Celery after commit. A periodic scanner republishes pending work within the
configured SLA.
Expand All @@ -218,8 +217,7 @@ charges provisional. A PostgreSQL scanner publishes ambiguous and expired
in-flight attempts; a fixed service principal runs read-only
`observe_put_result` plus a complete hash. Matching bytes complete Transaction B
once, authoritative absence releases charges and moves the put attempt to
`absent_replay_required`; while a legacy contributor upload item exists, it
alone moves to `replay_required`. Mismatched bytes quarantine the key. No background
`absent_replay_required`. Mismatched bytes quarantine the key. No background
resolver repeats a provider write. Exact replay after absence must atomically
reacquire capacity before another provider call.
Workstream never stores upload bytes in Postgres, Redis, or Celery payloads.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Chunk Contract: WS-ART-001-04A1 — Legacy Contributor Intake Removal

Parent initiative: `WS-ART-001` | Risk: L1 | Status: Proposed after 03C
Parent initiative: `WS-ART-001` | Risk: L1 | Status: Implemented; external PR gates pending

## Goal

Expand All @@ -9,24 +9,107 @@ unused schema before building the one-ZIP replacement.

## Allowed Files

ART upload models/migration/repository/routes/schemas, static architecture and
migration tests, stale-contract/docs, and scoped CI evidence.
- `backend/app/modules/artifacts/models.py`
- `backend/app/db/models.py`
- `backend/app/modules/artifacts/schemas.py`
- `backend/app/modules/artifacts/service.py`
- `backend/app/modules/artifacts/repository.py`
- one new linear `0051` Alembic revision after `0050_guide_source_v2`
- `backend/tests/test_alembic.py`
- `backend/tests/test_artifact_architecture.py`
- contributor-only removals plus guide/checker regression coverage in
`backend/tests/test_artifact_admission.py`,
`backend/tests/test_artifact_recovery.py`, and
`backend/tests/test_artifact_verification.py`
- `backend/tests/conftest.py`
- contributor-only cleanup in `backend/tests/test_artifact_authorization.py`
- stale-contract/spec/glossary text and this chunk's loop evidence

No runtime route file is allowed because no upload-session/item HTTP route is
currently composed and 04A1 adds no replacement.

## Not Allowed Changes

Replacement upload route, ZIP parsing, provider I/O, Submission/checker/review,
AUTH catalogue/availability, compatibility aliases, or fabricated backfill.
Old Alembic revisions are immutable. No detached historical columns, legacy
tables, compatibility models, or write aliases remain after the safe-empty
cutover.

## Locked Migration Policy

The new migration obtains exclusive locks on the legacy ledgers, put attempts,
and operation receipts before checking or changing schema. Upgrade refuses in
the same transaction if any of these facts exist:

- any `artifact_upload_sessions` row;
- any `artifact_upload_items` row;
- any `artifact_put_attempts.producer_request_type = 'contributor'` row;
- any non-null `artifact_put_attempts.upload_item_id`;
- any `artifact_operation_receipts.contract_version = 1` row; or
- any non-null `artifact_operation_receipts.upload_item_id`.

Refusal preserves the prior revision, schema, rows, foreign keys, and readable
historical identifiers without deletion or fabricated translation. Such a
deployment requires a separately approved maintenance/audit migration; 04A1
does not migrate populated legacy intake.

On a safe-empty deployment, upgrade removes contributor branches from put and
receipt constraints, removes their upload-item foreign keys/indexes/columns,
then drops `artifact_upload_items` and `artifact_upload_sessions`. Runtime code
simultaneously loses every contributor request, dispatch, lookup, and state
projection path.

Downgrade recreates the exact empty legacy columns, constraints, indexes, and
tables because a successful upgrade proved that no legacy facts were deleted.
It refuses atomically if the post-cutover database contains a contributor
producer fact that cannot be represented truthfully. No downgrade invents
sessions, items, actors, roles, or state.

## Acceptance Criteria

No route, command, service matrix lookup, model, or schema can create/use the
old intake; historical audit values remain readable where required; populated
unsafe downgrade/upgrade refuses honestly; no new intake becomes reachable.
- No route, request command, admission union member, service dispatch,
repository lookup, ORM model/import, SQL constraint, or recovery/verification
mutation can create or use the old intake.
- Architecture/OpenAPI proof shows 04A1 exposes neither the retired surface nor
the later submission-bundle replacement.
- Safe-empty upgrade removes both ledgers and every writable contributor
reference; safe-empty downgrade recreates the exact empty prior schema.
- Every populated legacy condition above refuses atomically and leaves revision,
schema, data, and historical identifier readability unchanged.
- Direct SQL cannot create contributor put attempts or upload-item-backed
receipts after cutover.
- Guide and checker-output admission, put confirmation, missing-object recovery,
integrity mismatch, and verification terminalization continue without any
upload-item mutation branch.
- AUTH's historical-only deletion proof continues to pass without catalogue,
matrix, availability, grant, or alias edits.

## Verification Commands

Focused Alembic/architecture/route tests, Ruff, stale scans, hosted Backend and
Agent Gates, repository 78% and changed subsystem 90% coverage.
Exact minimum:

```bash
(cd backend && .venv/bin/python -m pytest -q \
tests/test_artifact_architecture.py tests/test_alembic.py)
(cd backend && .venv/bin/python -m pytest -q \
tests/test_artifact_admission.py tests/test_artifact_recovery.py \
tests/test_artifact_verification.py)
(cd backend && .venv/bin/python -m ruff check app tests scripts)
python3 scripts/check_stale_artifact_contracts.py
python3 scripts/check_stale_authorization_docs.py
python3 scripts/check_stale_workstream_wording.py
python3 scripts/check_markdown_links.py
git diff --check
```

Alembic tests must prove empty upgrade/downgrade, every populated-upgrade
refusal condition, atomic unchanged state after refusal, exact schema removal
and recreation, and post-cutover direct-SQL denial. Architecture tests must
prove runtime/import/metadata and route/OpenAPI absence. Focused guide/checker
tests must cover the shared recovery/verification outcomes named above. Hosted
Backend and Agent Gates remain authoritative for repository 78% and ART 90%
coverage; no threshold or gate may be weakened.

## Required Reviewers

Expand All @@ -35,3 +118,7 @@ Architecture, security/auth, QA, product/ops, senior, CI, docs, reuse, test delt
## Human Review Focus And Stop Conditions

Prove deletion without opening a replacement or losing historical evidence.
Stop if populated legacy rows require migration, an AUTH/catalogue change seems
necessary, a replacement route/ZIP/scratch/provider/Submission/checker/review
behavior is needed, an old Alembic revision would need editing, or any required
test/coverage gate would need weakening.
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# WS-ART-001-04A1 External Review Response

## CodeRabbit

CodeRabbit completed its review on PR #264 without actionable comments.

CodeRabbit's earlier detailed review still contained one major finding and two
nitpicks, so the absence of a new summary comment was not treated as closure:

- **Major — resolved:** recovery tests no longer attach or read the unmapped
`recovery_submission_id` attribute on `ArtifactVerificationJob`. The fixture
now derives the real immutable `Submission.id` from the persisted
`CheckerRun` lineage and passes it explicitly in every task-scoped recovery
request, including retries.
- **Receipt nullability — resolved:** migration 0052 now makes
`artifact_operation_receipts.put_attempt_id` non-null after its locked
populated-legacy preflight. The ORM matches that v2-only invariant, the
downgrade restores legacy nullability before recreating the v1 shape, and a
model contract assertion protects the mapping.
- **Test-helper import — documented, no code move:** moving the shared
checker-output helper would also move its large project/guide/task/submission
relationship fixture across domain test modules. That broad fixture
refactor is outside this removal chunk and would increase this PR's coupling
and review surface. The import remains test-only and has no runtime effect.
- **Docstring heuristic — no change:** the repository's hosted docstring gate
passed. No repository standard was weakened and no unrelated docstrings were
added solely for a standalone reviewer heuristic.

Local correction evidence:

- Ruff on all changed Python files: passed;
- `git diff --check`: passed;
- no `recovery_submission_id` references remain under `backend/tests`;
- focused PostgreSQL rerun was interrupted by the known local Python exit 139
before pytest produced a result; hosted sharded Backend and Agent Gates are
the authoritative execution evidence for this correction.

The first hosted correction run then exposed the expected schema-custody delta:
making `put_attempt_id` non-null changed the canonical public-schema
fingerprint. The database migrations completed successfully, but fixture reset
failed closed because `EXPECTED_PUBLIC_SCHEMA_SHA256` still named the prior
nullable schema. The constant now records the hosted schema digest
`8acef1c1d96ced0a4d4723ce71aa2e675ab841ec4305d9421ed0584313b98b55`;
no reset guard was removed or weakened. After REV-03A1 merged its own migration
as revision 0051, ART was rebased as revision 0052 with
`0051_review_queue_foundation` as its parent; REV's schema and tests were
preserved. A fresh merged-head migration produced the combined canonical schema
digest `f30127e0acd66d6e2e0cc4d56741ddc8b7cd09175d8c3251749624a0fc353aee`,
which replaces both branch-local fingerprints.

The first reconciled hosted run passed project lifecycle, shared foundations,
and schema contracts B, then found two REV downgrade assertions that still
named REV's former branch-local head. Both downgrade refusals behaved correctly
and atomically retained the merged current head. Their expected revision is now
`0052_legacy_intake_removal`; no REV downgrade guard, model, or product behavior
changed.

## Hosted CI correction

The first Backend sharded run failed one `shared_foundations` test. Replacing
the deleted contributor fixture with a current checker-output fixture made the
recovery resource submission-scoped, but the operator HTTP test requests still
omitted the canonical `submission_id`. Production correctly failed closed with
`409 artifact recovery resource facts changed`.

The test now carries the exact submission lineage for denied, stale, successful,
replayed, altered, ineligible, and cross-project recovery requests. No production
authorization or recovery guard was weakened.

Focused correction evidence:

- the formerly failing operator HTTP test: `1 passed`;
- complete operator API and recovery files: `15 passed`;
- Ruff on the corrected test file: passed.

The correction is pushed for a fresh hosted Backend and Agent Gates run.

The fresh run proved `shared_foundations`, `project_lifecycle`,
`task_lifecycle`, and `schema_contracts_a`, then exposed one stale assertion in
`schema_contracts_b`: the broad current-schema contract still required the two
tables this chunk intentionally removes. The assertion now classifies
`artifact_upload_sessions.id` and `artifact_upload_items.id` as discarded
columns. The exact formerly failing schema test passes locally.

All five shards then passed, while the aggregate ART subsystem coverage gate
reported `89.52%`. The deleted contributor tests had also carried ambiguous-put
terminal coverage. That proof is now restored on the current task-scoped
checker-output producer for mismatch, provider conflict, and collision with an
already verified replica. All three cases pass. Combining those exact tests
with the authenticated hosted shard coverage reaches `90.01%`; the hosted gate
must confirm the final commit.
Loading
Loading