-
Notifications
You must be signed in to change notification settings - Fork 6
WS-ART-001-03B3B1: approve guide parser dependencies #230
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
Merged
abiorh-claw
merged 4 commits into
main
from
codex/ws-art-001-03b3b1-parser-dependency-approval
Jul 31, 2026
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
66ac70e
ci(artifacts): approve guide parser dependencies
Abiorh001 92cceee
ci(artifacts): register dependency gate test lane
Abiorh001 a5f8683
fix(artifacts): harden dependency approval review gate
Abiorh001 6e5e697
Merge branch 'main' of https://github.com/Flow-Research/workstream 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
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
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
44 changes: 44 additions & 0 deletions
44
...mmutable-artifact-storage/reviews/WS-ART-001-03B3B1-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,44 @@ | ||
| # WS-ART-001-03B3B1 External Review Response | ||
|
|
||
| ## Comments addressed | ||
|
|
||
| - GitHub Backend run `30600808957` passed the exact-head approval gate, then | ||
| failed canonical collection with | ||
| `missing_lane_modules:tests/test_guide_extractor_dependencies.py`. | ||
| - The focused test module is now assigned to the existing | ||
| `shared_foundations` semantic lane. No new lane or coverage exception was | ||
| introduced. | ||
|
|
||
| ## Comments deferred | ||
|
|
||
| - None. | ||
|
|
||
| ## Human decisions needed | ||
|
|
||
| - A fresh independent approval of the exact final PR head is pending. | ||
| `abiorh-claw` approved `66ac70e6` before the CI repair, so that approval was | ||
| correctly dismissed when the head changed. | ||
|
|
||
| ## CodeRabbit | ||
|
|
||
| - The initial check returned `pass` with `Review rate limited` and no comments. | ||
| - The repaired-head review posted four findings. Three valid findings are | ||
| addressed: exact-final-head wording, stable type-validation failures, and | ||
| preserving an active approval across a later `COMMENTED` review. | ||
| - The workflow-checkout finding is rejected with direct hosted evidence: review | ||
| event run `30600808957` passed exact-head approval against `66ac70e6`, executed | ||
| the PR's new code, and reached canonical lane collection. The checkout also | ||
| uses `fetch-depth: 0`, so both PR parents required by the diff were present. | ||
|
|
||
| ## Commands rerun | ||
|
|
||
| - Focused dependency gate and tests. | ||
| - Stable malformed-value and approval-then-comment regression tests. | ||
| - Canonical semantic-lane collect-only validation. | ||
| - Ruff, markdown links, stale-contract checks, lightweight agent gates, and | ||
| diff checks. | ||
|
|
||
| ## Remaining risks | ||
|
|
||
| - Hosted Backend and Agent Gates must pass on the repaired exact head after a | ||
| fresh independent approval. |
131 changes: 131 additions & 0 deletions
131
...ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B3B1-pr-trust-bundle.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,131 @@ | ||
| # WS-ART-001-03B3B1 PR Trust Bundle | ||
|
|
||
| ## Chunk | ||
|
|
||
| `WS-ART-001-03B3B1` — Parser Dependency Approval. | ||
|
|
||
| ## Goal | ||
|
|
||
| Approve and enforce the smallest exact PDF, OOXML-safety, and image-metadata | ||
| dependency set without installing packages or changing runtime behavior. | ||
|
|
||
| ## Human-approved intent | ||
|
|
||
| The merged 03B3B split contract requires dependency approval before any PDF, | ||
| OOXML, or image parser implementation. The human owner explicitly started this | ||
| chunk after PR #228 merged. | ||
|
|
||
| ## What changed | ||
|
|
||
| - Added a canonical allowlist for `pypdf==6.14.2`, `defusedxml==0.7.1`, and | ||
| `Pillow==12.3.0`, including exact approved wheel URLs and SHA-256 hashes. | ||
| - Added a deterministic checker for manifest shape, wheel identity/platform, | ||
| hash-bound future declarations, prohibited packages, format-scoped imports, | ||
| and independent exact-head GitHub approval. | ||
| - Added focused failure tests and wired the gate before backend installation. | ||
| - Registered the focused test module in the canonical `shared_foundations` | ||
| semantic lane after hosted inventory validation exposed the missing entry. | ||
| - Updated the ART specification, chunk command, status, and review evidence. | ||
|
|
||
| ## Why it changed | ||
|
|
||
| Untrusted document parsers are a supply-chain boundary. A package name or | ||
| version alone cannot prove which bytes will later be installed, and a | ||
| repository-authored approval file cannot independently authorize itself. | ||
|
|
||
| ## Design chosen | ||
|
|
||
| - PDF: pure-Python `pypdf`. | ||
| - OOXML safety: pure-Python `defusedxml` plus later ART-owned bounded container | ||
| and format adapters. | ||
| - Image metadata: native `Pillow`, restricted to two named Python 3.11/3.12 | ||
| manylinux x86_64 wheels. | ||
| - Future runtime declarations must use the exact approved wheel URL with its | ||
| SHA-256 fragment. Optional/dependency-group parser declarations are denied. | ||
| - Allowlist changes require a current approving GitHub review from an | ||
| independent human owner/member/collaborator on the exact PR head. | ||
|
|
||
| ## Alternatives rejected | ||
|
|
||
| - `python-docx`, `python-pptx`, `openpyxl`, `lxml`, and `XlsxWriter`: larger, | ||
| unnecessary graphs for the bounded v0.1 OOXML extraction design. | ||
| - Version-only pins: do not bind installed artifact bytes. | ||
| - Repository-local approval as authority: forgeable by the proposing PR. | ||
| - `pull_request_target`: an unnecessary privileged workflow boundary. | ||
|
|
||
| ## Scope control | ||
|
|
||
| No `pyproject.toml`, lockfile, dependency install, application import, parser | ||
| adapter, AUTH, Celery, submission, or product lifecycle change is included. | ||
|
|
||
| ## Product behavior | ||
|
|
||
| None. Project Manager guide-source handling remains distinct from contributor | ||
| one-ZIP submission handling. All parser behavior remains unimplemented. | ||
|
|
||
| ## Acceptance criteria proof | ||
|
|
||
| - Canonical schema covers version, license, maintenance, advisories, | ||
| transitive graph, native-code, malformed-input, network, cancellation, | ||
| import, format, source, wheel URL, and hash facts. | ||
| - Three closed scopes are present exactly once. | ||
| - Plain pins, wrong hashes/URLs/wheels/tags/scopes, prohibited packages, | ||
| optional-group bypasses, cross-format imports, self/bot/stale/dismissed | ||
| approvals, and GitHub API failure all reject in focused tests. | ||
|
|
||
| ## Tests/checks run | ||
|
|
||
| - Manifest checker: pass. | ||
| - Focused pytest: 34 pass; checker coverage 92.94 percent. | ||
| - Ruff lint and format: pass. | ||
| - Compile, markdown links, stale artifact/wording, lightweight agent gates, | ||
| and `git diff --check`: pass. | ||
|
|
||
| ## Test delta | ||
|
|
||
| One new focused test module; no tests removed, skipped, or weakened. | ||
|
|
||
| ## CI integrity | ||
|
|
||
| The gate runs before package installation. Existing backend tests, semantic | ||
| lanes, provider proof, and 78/90 percent coverage floors are unchanged. Token | ||
| permissions remain read-only: `contents: read` and `pull-requests: read`. | ||
|
|
||
| ## Reviewer results | ||
|
|
||
| Architecture, security, QA, product/ops, senior engineering, CI integrity, | ||
| reuse/dedup, and test delta pass after valid findings were repaired. Docs | ||
| review is rerun on the staged evidence. | ||
|
|
||
| ## External review | ||
|
|
||
| CodeRabbit's later substantive review produced three valid repaired findings | ||
| and one workflow-checkout finding rejected by direct hosted evidence. The first approval-authorized | ||
| Backend run found one missing semantic-lane inventory entry, now repaired. | ||
| Exact repaired-head hosted results remain required external evidence. | ||
|
|
||
| ## Remaining risks | ||
|
|
||
| - Live GitHub review-event/check semantics require hosted proof. | ||
| - Review submitted/dismissed events rerun the full Backend job; this is an | ||
| accepted low operational cost for the v0.1 fail-closed required check. | ||
| - `defusedxml` is a stable, slow-release security utility; its isolation and | ||
| bounded OOXML use remain mandatory in later chunks. | ||
|
|
||
| ## Follow-up work | ||
|
|
||
| After this chunk merges, `WS-ART-001-03B3B2` may install only the approved PDF | ||
| wheel and implement bounded PDF extraction. Later OOXML and image chunks remain | ||
| separate and require explicit starts. | ||
|
|
||
| ## Human review focus | ||
|
|
||
| Confirm the three-package minimum, wheel URL/hash selection, native Pillow | ||
| platform bounds, prohibited package set, and independent exact-head approval | ||
| semantics. | ||
|
|
||
| ## Human merge ownership | ||
|
|
||
| Only the human owner may approve and merge this PR. The PR must receive an | ||
| independent qualifying GitHub approval on its exact final head; this agent will | ||
| not merge it. |
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: Flow-Research/workstream
Length of output: 3403
🏁 Script executed:
Repository: Flow-Research/workstream
Length of output: 50047
On
pull_request_review, the dependency approval gate checks out the default branch while comparing against the pull-request head.pull_request_reviewsets${{ github.sha }}to the default-branch ref, but this job enables--require-pr-approvalfor${{ github.event.pull_request.head.sha }}and the script runsgit diff ${base_sha}...${head_sha} -- guide_extractor_dependencies.json. The approvedhead_shacan pass while the checked-out workflow actually validates default-branch artifacts.For a fork PR, the head commit is missing from the clone, so
git diff base...head -- guide_extractor_dependencies.jsonexits non-zero and the script fails withguide_dependency_git_diff_failed.Use the pull-request head checkout for review events, or restrict this step to
pull_requestevents and enforce approval by head separately.🤖 Prompt for AI Agents