docs(art): define default pre-submit checker catalogue plan - #271
Conversation
📝 WalkthroughWalkthroughThe PR updates WS-ART-001 planning and architecture documents. It splits ART-04B into bounded phases, defines a versioned pre-submit checker catalogue, removes the standalone precheck route, and documents server-owned bundle preparation, evidence, admission, and immutable submission creation. ChangesWS-ART-001 planning and sequencing
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
docs/architecture_checker_framework.md (1)
39-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the catalogue as a dispatch authority.
PreSubmissionCheckerCatalogueis not a registry. Replace “neither registry” with “neither the durable registry nor the pre-submission catalogue”. State that the catalogue owns pre-submit dispatch and the durable registry owns post-submit dispatch.As per coding guidelines, use terminology consistent with the canonical architecture names.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/architecture_checker_framework.md` around lines 39 - 46, Update the “Durable/Post-Submit Checker Registry” section to identify PreSubmissionCheckerCatalogue as the pre-submit dispatch authority, while the durable checker registry owns post-submit dispatch. Replace the ambiguous “neither registry” wording with explicit references to the durable registry and pre-submission catalogue, preserving the existing no-duplicate-IDs and primitive-map constraint.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-04B1-default-checker-catalogue.md:
- Around line 76-83: Add an explicit mapping in the catalogue specification from
each stable catalogue ID to its public policy checker name and implementation
primitive, including the aliases such as check_submission_packet →
validate_submission_packet and check_required_files → require_file. State that
the public policy name is persisted in the effective plan and result, while
dispatch uses the mapped implementation primitive, and preserve deterministic
compiled rule instance IDs.
In
@.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-04B3-effective-pre-submit-evidence.md:
- Around line 33-38: Update the shared Checker Result Contract and CheckerResult
model to explicitly type catalogue ID/version, source, bounded code/message, and
policy trace for every result. Define their exact nesting and persistence rules
in the canonical schema, and ensure serialization/storage preserves them rather
than relying on untyped metadata.
In @.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/PLAN.md:
- Around line 335-336: Update the checker failure behavior statement in the plan
to distinguish completed checker findings from infrastructure failures. State
that only completed checker results create structured contributor findings,
while infrastructure failures, disabled mandatory checks, and resource
exhaustion produce a stable retryable infrastructure outcome without creating
findings or durable artifact, Submission, or review state.
In `@docs/architecture_lockdown.md`:
- Around line 124-131: Update the pre_submission_check_failed audit behavior
described in the continuous submission-bundle preparation flow to persist only a
bounded, path-redacted audit projection rather than the full structured checker
result. Define and document the projection’s explicitly allowed fields,
excluding filenames, scratch/provider references, credentials, raw output, and
unbounded messages, while preserving the existing failure status and audit-event
semantics.
In `@docs/glossary.md`:
- Around line 228-232: The pre-submit task lock must record the immutable
default catalogue snapshot, including its version, digest or entry hashes, and
enabled/disabled state. Update the pre-submit glossary definition in
docs/glossary.md and the corresponding plan text in
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/DISCOVERY.md to
state that the pre-submit plan hash or catalogue manifest binds each locked task
to that snapshot; keep the default catalogue immutable.
In `@docs/spec_chunk_8_submission_artifact_policy_checkers.md`:
- Around line 6-12: Update the submission-bundle preparation section to remove
all references to the obsolete PreSubmitCheckResponse contract. Describe
preflight failures using the canonical same-request
pre_submission_checker_failed response, define the bounded submission-bundle
preparation response, and explicitly state that no independent precheck route or
standalone registry exists, using terminology from the architecture and lockdown
documents.
In `@docs/template_submission_packet.md`:
- Around line 52-58: Align the contributor documentation with the single packet
contract: in docs/template_submission_packet.md lines 52-58, remove or reframe
client-supplied Evidence fields, references, and hashes so the contract uses one
outer ZIP plus summary and attestation with server-derived metadata; in
docs/operations_project_operating_manual.md lines 248-249, replace “output
package or reference” and separate evidence entries with the canonical one-ZIP
fields. Preserve the documented pre-submission behavior.
---
Nitpick comments:
In `@docs/architecture_checker_framework.md`:
- Around line 39-46: Update the “Durable/Post-Submit Checker Registry” section
to identify PreSubmissionCheckerCatalogue as the pre-submit dispatch authority,
while the durable checker registry owns post-submit dispatch. Replace the
ambiguous “neither registry” wording with explicit references to the durable
registry and pre-submission catalogue, preserving the existing no-duplicate-IDs
and primitive-map constraint.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 175f91f7-b9e9-4157-b774-1d001110928e
📒 Files selected for processing (27)
.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/AUTH_HANDOFF.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/CHUNK_MAP.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/DECISIONS.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/DISCOVERY.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/PLAN.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/REVIEW_LOG.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/RISKS.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-04A4-legacy-precheck-removal.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-04B-pre-submit-admission.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-04B1-default-checker-catalogue.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-04B2-default-checker-execution.md.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-04B3-effective-pre-submit-evidence.mddocs/architecture_checker_framework.mddocs/architecture_data_model.mddocs/architecture_lockdown.mddocs/current_system_data_flow.htmldocs/decision_0011_submission_artifact_policy_drives_pre_submit.mddocs/glossary.mddocs/operations_project_operating_manual.mddocs/product_first_user_flows.mddocs/spec_artifact_storage_service.mddocs/spec_authorization_service.mddocs/spec_chunk_8_submission_artifact_policy_checkers.mddocs/template_checker_policy.mddocs/template_submission_artifact_policy.mddocs/template_submission_packet.md
2cad8df to
0afdf7d
Compare
PR Trust Bundle: WS-ART-001-PLAN4
Chunk
WS-ART-001-PLAN4— planning-only correction for default pre-submission checks.Goal
Define one discoverable, versioned, disable-aware catalogue and one effective
pre-submission execution while preserving the exact contributor ZIP custody
chain and locked Project Guide rules.
Human-Approved Intent
Every submission is one outer ZIP. Workstream default checks and project-specific
checks run as one effective pre-submission operation against that exact ZIP in
bounded private scratch. All platform defaults are named centrally and expose
enabled/disabled state without allowing a mandatory check to be bypassed.
What Changed
documents with one-ZIP preparation, verified admission, and immutable binding;
Why It Changed
The former plan left default checks scattered across constants, compiler
primitives, a legacy registry, and documents; did not safely define
disabled;and could leave a caller-owned standalone precheck beside the authoritative ZIP
flow. The combined 04B also crossed too many L1 boundaries for one PR.
Design Chosen
The task-locked compiled-bundle hash commits to an immutable catalogue snapshot
and locked project policy. Results use typed provenance; mandatory unavailable
checks yield retryable infrastructure failure rather than findings or success.
Alternatives Rejected
Scope Control
Planning and canonical documentation only. No route, runtime code, database
migration, provider I/O, AUTH activation/grant, Submission lifecycle, checker
execution, Review, contribution, compensation, or reputation behavior changes.
Product Behavior
No product behavior changes in this PR. The approved target is one outer ZIP
plus summary/attestation, server-derived manifest/evidence facts, one effective
pre-submission result, verified admission, then atomic immutable Submission
binding under fresh authority.
Acceptance Criteria Proof
specified for every platform default and constrained project rule;
Tests And Checks Run
git diff --check— pass.Test Delta
No runtime tests change in this planning PR. The contracts require route/OpenAPI
removal proof, catalogue compiler tests, sealed materialization/executable parity,
crossed-state invalidation, full repository 78 percent coverage, and owned
subsystem 90 percent coverage in their implementation PRs.
CI Integrity
No workflow, lane, package, lint, or coverage configuration is changed. Existing
hosted gates remain intact; each implementation contract names the exact full
and scoped coverage commands it must preserve.
Reviewer Results
Architecture, security, QA, product/ops, senior engineering, CI integrity,
documentation, reuse/dedup, and test-delta reviewers pass after all valid
findings were repaired.
External Review
CodeRabbit posted seven actionable findings and one description warning. All
seven are addressed; the PR description is replaced with this trust-bundle
structure. Detailed disposition is in
WS-ART-001-PLAN4-external-review-response.md.Remaining Risks
Follow-Up Work
After human merge: implement only 04A4. Do not start 04B1 automatically.
Human Review Focus
Human Merge Ownership
Only the human owner may approve and merge PR #271. This planning merge does
not authorize any implementation chunk.