-
Notifications
You must be signed in to change notification settings - Fork 7
WS-ART-001-03C: cut over verified guide sources #249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Abiorh001
wants to merge
23
commits into
main
Choose a base branch
from
codex/ws-art-001-03c-guide-source-cutover
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
14c2b5d
feat(artifacts): cut over verified guide sources
Abiorh001 d9e5ab0
docs: clarify celery beat operation
Abiorh001 6a353cc
test(projects): reconcile verified setup fixtures
Abiorh001 58886d0
fix(artifacts): resolve guide cutover review findings
Abiorh001 1c951f7
Merge remote-tracking branch 'origin/main' into codex/ws-art-001-03c-…
Abiorh001 670b952
test(projects): complete verified lifecycle fixtures
Abiorh001 2cda9aa
Merge remote-tracking branch 'origin/main' into codex/ws-art-001-03c-…
Abiorh001 4f1ca20
test(db): bind combined auth artifact schema
Abiorh001 98b8348
test(projects): preserve exact setup generation
Abiorh001 312e57c
test(tasks): preserve artifact-bound setup lineage
Abiorh001 ea913a0
Merge branch 'main' into codex/ws-art-001-03c-guide-source-cutover
abiorh-claw de2ae8b
test(ci): reconcile shared artifact foundations
Abiorh001 6f9aab2
Merge remote-tracking branch 'origin/codex/ws-art-001-03c-guide-sourc…
Abiorh001 45329fd
Merge remote-tracking branch 'origin/main' into codex/ws-art-001-03c-…
Abiorh001 eb82a59
fix(artifacts): close guide cutover review findings
Abiorh001 d6862d5
test(artifacts): scope guide authority migration proof
Abiorh001 8213af5
Merge remote-tracking branch 'origin/main' into codex/ws-art-001-03c-…
Abiorh001 79d2fdf
docs(artifacts): record distributed lane repair
Abiorh001 65d1cd7
docs(artifacts): refresh reconciled review evidence
Abiorh001 feb40f9
test(artifacts): use locally authorized guide uploader
Abiorh001 098da08
docs(artifacts): close final external wording note
Abiorh001 d611f26
ci: validate reconciled artifact head
Abiorh001 3533799
ci: wire isolated MinIO into API fan-in
Abiorh001 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
111 changes: 111 additions & 0 deletions
111
...1-immutable-artifact-storage/reviews/WS-ART-001-03C-external-review-response.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| # WS-ART-001-03C External Review Response | ||
|
|
||
| ## Comments addressed | ||
|
|
||
| - Agent Gates initially rejected restricted human/worker vocabulary in the | ||
| README Celery Beat note. The note now uses execution-process terminology; | ||
| the stale authorization documentation check passes and the rerun succeeded. | ||
| - Backend semantic lanes exposed project/task fixtures that assumed setup runs | ||
| existed even when test configuration disabled autostart. Verified fixtures | ||
| now create one production-shaped generation run only when absent, isolated | ||
| worker tests explicitly create their run, and downstream task fixtures bind | ||
| the verified report to that same run. | ||
| - The queued-before-verified-material assertion now uses the persisted | ||
| `current_step="queued"` contract. | ||
| - Backend run `30781770775` then exposed eight remaining project-lifecycle | ||
| fixtures that still selected diagnostic reports or omitted verified-report | ||
| setup-run linkage. Those fixtures now use exact verified reports, and the | ||
| policy-derivation route composes the canonical verified-material adapter. | ||
| - Backend run `30783400382` reduced the remaining failures to five exact test | ||
| seams: warning tests now acknowledge the diagnostic record used by manual | ||
| policy creation and the verified record used by activation; direct service | ||
| tests compose the verified-material adapter. It also exposed that the worker | ||
| used traceback logging for an unexpected parser/runtime failure; production | ||
| now emits only a fixed message and setup-run ID, and the test proves raw | ||
| secrets and paths do not enter the log payload. | ||
| - CodeRabbit inline findings were verified and resolved: migration constraint | ||
| operations use the physical PostgreSQL name in both directions, guide | ||
| continuation recovery publishes the continuation directly with an | ||
| independent bound, and the two documentation claims now match implementation. | ||
| - CodeRabbit's latest incremental fixture-architecture comment was valid: the | ||
| complete verified-guide lineage helper now lives in a shared test utility, | ||
| and project and task tests consume that single implementation without | ||
| importing one product test module from another. | ||
| - The remaining valid review cleanups are also applied: stale contracts reject | ||
| all three removed v1 identity fields; verified agent-input projection is | ||
| shared; extraction slot naming and reader typing are explicit; the dispatch | ||
| predicate is typed and documented; the migration records its empty-table | ||
| downgrade dependency; the eligibility probe avoids a pointless row lock; | ||
| repository layout is conventional; and the unused source-label threat | ||
| categories were removed from the parametrized test. | ||
| - After AUTH PR #248 merged, ART was rebased as the single successor migration | ||
| `0049_guide_source_v2`. Hosted run `30784652926` proved the exact combined | ||
| AUTH+ART public-schema fingerprint; the fail-closed test constant now records | ||
| that observed value. | ||
| - Hosted run `30786024204` reduced the suite to one stale synthetic fixture: a | ||
| policy-context test created setup generation 2 while all verified ART lineage | ||
| remained generation 1. The test now reuses the exact source setup generation | ||
| and varies only the effective-policy context it is intended to isolate. | ||
| - Hosted run `30786751487` passed the complete project lifecycle and exposed | ||
| four task corruption tests whose shared helper deleted an immutable, | ||
| ART-bound setup run. The helper now clears only its mutable post-submit | ||
| output pointer before removing the generated policy and preserves all | ||
| verified guide-binding lineage. | ||
| - Hosted run `30787408677` passed both project and task lifecycle lanes. The | ||
| remaining shared-foundations failures were reconciliation-only: the OpenAPI | ||
| inventory now records the exact merged AUTH surface, the ART-admission | ||
| migration fixture uses the columns that existed at revision `0028`, and | ||
| downgrade tests expect the outer `0049` clean-cut guard that necessarily | ||
| protects populated guide-source lineage before older migration guards can | ||
| run. Direct migration-function tests continue proving the superseded `0039`, | ||
| `0040`, and `0042` populated-evidence guards independently. | ||
| - Distributed Backend run `30790674519` passed project lifecycle, task | ||
| lifecycle, shared foundations, and schema contracts B. Schema contracts A | ||
| exposed one migration-scope test that seeded pre-0045 rows and then upgraded | ||
| through the intentional `0049` clean-cut refusal. The historical-preservation | ||
| test now stops at revision `0045`, which is the migration it proves; separate | ||
| tests continue proving the current-head `0049` refusal. | ||
| - Distributed Backend run `30800292363` passed all five semantic lanes and | ||
| exposed one real-API helper authority mismatch in the final fan-in job. The | ||
| helper created the guide-source snapshot with the token carrying the local | ||
| Project Manager grant, but attempted the hidden artifact upload with a token | ||
| carrying only a legacy Flow role claim. The API correctly returned the | ||
| resource-hiding 404. The upload now uses the same locally authorized token as | ||
| snapshot creation; no production authorization behavior changed. | ||
| - CodeRabbit's review of repair head `feb40f9a` found no code defect and one | ||
| trivial compound-modifier wording issue in the trust bundle; the wording is | ||
| now hyphenated without changing evidence meaning. | ||
|
|
||
| ## Comments deferred | ||
|
|
||
| - Holding guide/read row locks through the authorized provider read remains | ||
| intentional for this L1 boundary: splitting the transaction would violate | ||
| the reviewed transaction-bound AUTH/read contract. | ||
| - Moving verified-report queries from `ProjectService` into the repository is | ||
| a non-functional ownership refactor outside this clean-cut chunk. | ||
| - A dedicated continuation-scan interval is explicitly optional for v0.1; the | ||
| shared bounded scan interval remains the approved operational surface. | ||
| - The pre-submit worker wrapper retains the stable Celery/test seam and is not | ||
| an independent execution path. | ||
|
|
||
| ## Human decisions needed | ||
|
|
||
| - None. Human merge approval remains required after hosted checks pass. | ||
|
|
||
| ## Commands rerun | ||
|
|
||
| - Ruff over backend application, tests, backend scripts, and repository scripts. | ||
| - Python compilation for backend application and tests. | ||
| - Stale authorization and artifact documentation checks. | ||
| - Lightweight agent gates and Markdown link validation. | ||
|
|
||
| ## Required next evidence | ||
|
|
||
| - Hosted Backend and Agent Gates rerun on the exact E2E repair head. | ||
|
|
||
| ## Remaining risks | ||
|
|
||
| - All five distributed semantic lanes passed on run `30800292363`; the final | ||
| real-API fan-in proof remains pending on the repaired helper. | ||
| - CodeRabbit's latest incremental review reported no new actionable findings; | ||
| all earlier inline findings were checked against the final diff. |
59 changes: 59 additions & 0 deletions
59
...1-immutable-artifact-storage/reviews/WS-ART-001-03C-internal-review-evidence.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| # WS-ART-001-03C Internal Review Evidence | ||
|
|
||
| ## Scope | ||
|
|
||
| Guide-source v2 clean cut, verified ART-only guide material, and automatic | ||
| same-generation setup continuation. | ||
|
|
||
| ## Final reviewer results | ||
|
|
||
| Earlier reviewers evaluated the implementation through `312e57c5` and the | ||
| focused shared-foundations reconciliation at `de2ae8b8`. After merging current | ||
| `main`, the focused migration-scope repair and updated trust evidence were | ||
| reviewed through `79d2fdfd`; the follow-up evidence-only correction records | ||
| those results. The review tracks used the changed ART/project/task tests, Ruff, | ||
| Python compilation, `git diff --check`, stale ART contracts, Markdown links, | ||
| and the lightweight agent gates identified below. | ||
|
|
||
| - Architecture: pass; project continuation retains a closed ART capability. | ||
| - Security/auth: pass; exact authorization facts and provider reads remain in | ||
| the same transaction-held lock window and candidate changes fail closed. | ||
| - Product/ops: pass; continuation evidence is operator-visible. | ||
| - Senior engineering: pass with low risk after durable stale-dispatch claiming. | ||
| - CI integrity: internal pass; the 78% global floor remains and focused 90% | ||
| gates were added. Hosted Backend run `30767889658` completed with failure and | ||
| initiated the fixture-reconciliation sequence recorded in the external review | ||
| response. Agent Gates pass; final hosted Backend proof remains pending after | ||
| reconciliation with the distributed lanes merged from `main`. | ||
| - Docs: pass after guide-source v2 and diagnostic/verified report corrections. | ||
| - Reuse/dedup: pass with low risk after candidate, AUTH-fact, and read-path reuse. | ||
| - Test delta: pass with low risk after verified route, visibility, and dispatch | ||
| retry replacement coverage. | ||
| - QA: pass after downstream project/task fixtures were reconciled with verified | ||
| activation and existing setup-generation semantics. | ||
|
|
||
| ## Resolved findings | ||
|
|
||
| - Removed the legacy caller-controlled guide byte identity and excerpt path. | ||
| - Separated diagnostic and verified sufficiency report slots. | ||
| - Replaced fabricated test provenance with the full constrained ART lineage. | ||
| - Reused the existing setup run instead of creating a duplicate generation. | ||
| - Centralized verified replica selection and binding authorization facts. | ||
| - Shared authorized provider-read preparation between classification and extraction. | ||
| - Added a committed `dispatch_pending` claim, deterministic task id, 60-second | ||
| stale cutoff, and explicit claim timestamp advancement before retry publish. | ||
|
|
||
| ## Local evidence | ||
|
|
||
| - Ruff and Python compilation: passed for changed backend code/tests. | ||
| - `git diff --check`: passed. | ||
| - Stale artifact contract scan: passed at `guide_source_cutover`. | ||
| - Lightweight agent gates: 10 passed after the distributed-lane CI merge. | ||
| - Distributed lane evidence/merge validators: 44 passed. | ||
| - Markdown link check: passed for changed Markdown files. | ||
| - Non-database focused project tests: 4 passed. | ||
| - The focused database-backed migration test was attempted through the | ||
| canonical isolated runner after current-main reconciliation. The local runner | ||
| reached migration `0049` and then failed in its database-operation wrapper | ||
| before executing the test assertion; hosted Backend/Agent Gates remain | ||
| required. | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.