Kjell Tore Guttormsen
d577cadcf4
Re-review scope a44e37b..85a8ee3. BLOCK (1+6+3) → WARN (2 new MAJOR surfaced in 30-fix-references.mjs) → ALLOW (both remediated, re-verified by the independent code-correctness reviewer). 0 BLOCKER / 0 MAJOR / 0 MINOR / 0 SUGGESTION. Operator window (RUNBOOK.md) no longer gated by a review BLOCK. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
94 lines
7.1 KiB
Markdown
94 lines
7.1 KiB
Markdown
---
|
||
type: trekreview
|
||
review_version: "1.0"
|
||
created: 2026-06-17
|
||
task: "Marketplace polyrepo-migration tooling — 11-step extraction/validation/cutover harness"
|
||
slug: marketplace-polyrepo-migration
|
||
project_dir: docs/marketplace-polyrepo-migration
|
||
brief_path: docs/marketplace-polyrepo-migration/brief.md
|
||
scope_sha_start: a44e37b
|
||
scope_sha_end: 85a8ee3
|
||
reviewed_files_count: 29
|
||
findings: []
|
||
---
|
||
|
||
# Review: Marketplace polyrepo-migration tooling — 11-step extraction/validation/cutover harness
|
||
|
||
## Executive Summary
|
||
|
||
Verdict: **ALLOW**. This is the final pass of a three-step remediation arc. The first review (commit 3065930) returned **BLOCK** (1 BLOCKER + 6 MAJOR + 3 MINOR). The second pass (after commits 86208da + fef4b33) returned **WARN** — all 10 original findings resolved (the BLOCKER's flat external-source shape replaced by the nested `{ source: { source:'url', url, ref } }` form per the official Claude Code marketplace schema, with `validate()` + asserting tests; the SC6/SC2 regression detectors extracted into `sc6-check.sh`/`sc2-regression.sh` and negatively tested; the single-tag assertion, `$dest`-sourced regression capture, mktemp guard, `sc2_gate` routing, `git filter-repo` re-assertion, path-hygiene gate, and `blob_strip_safe: null` all confirmed) — but the high-effort deep read surfaced 2 NEW MAJOR findings in `migration/30-fix-references.mjs` (an untested `package.json` rewrite branch + a stale monorepo-relative `repository.directory` surviving into standalone repos). Both were remediated: the rewriter now drops `repository.directory` in both the plugin.json and package.json branches (idempotency preserved), and a new test drives the previously-unguarded branch against a synthetic `llm-security` extract. The independent code-correctness reviewer re-verified both as RESOLVED with no new correctness issue. Every Success Criterion traces to delivered code; every Non-Goal (the window-only Forgejo create / push / HTTPS-source resolution) remains correctly unbuilt and operator-gated by D8/NULL-push. The full local dry-run is green (11/11 targets, 0 pushes). Two standing disclosures, unchanged: the brief has no YAML frontmatter (FM_MISSING — a soft warning, not a finding, because it is a hand-written RATIFIED brief), and high-effort normalization was applied per the operator's maximal-discipline standing instruction (Pass 3 Cloudflare reasonableness filtering skipped).
|
||
|
||
## Coverage
|
||
|
||
| File | Treatment | Reason |
|
||
|------|-----------|--------|
|
||
| `migration/00-preflight.sh` | summary-only | Preflight; path-hygiene + filter-repo/python3 gates confirmed asserting |
|
||
| `migration/00-preflight.test.mjs` | summary-only | Preflight guard test; single-path lock added (836b8e9 alignment) |
|
||
| `migration/10-extract.sh` | summary-only | Extraction driver; single-path per 836b8e9; re-asserts git-filter-repo |
|
||
| `migration/10-extract.test.mjs` | summary-only | Extraction test; confirmed asserting |
|
||
| `migration/20-rehome-config.sh` | summary-only | Config rehome; covered by paired test |
|
||
| `migration/20-rehome-config.test.mjs` | summary-only | Rehome test; confirmed asserting |
|
||
| `migration/30-fix-references.mjs` | summary-only | Reference rewriter; 2 prior MAJOR (deep read) now RESOLVED |
|
||
| `migration/30-fix-references.test.mjs` | summary-only | Now covers the package.json + plugin.json branch (synthetic llm-security) |
|
||
| `migration/40-validate-standalone.sh` | summary-only | Standalone validator; sc2_gate routing added |
|
||
| `migration/40-validate-standalone.test.mjs` | summary-only | Validator test; confirmed asserting |
|
||
| `migration/templates/validate-plugin.generic.sh` | summary-only | Generic validation template; no dynamic logic regression |
|
||
| `migration/templates/gitignore.plugin.tmpl` | summary-only | Static template; no executable regression surface |
|
||
| `migration/50-config-audit-sc2.sh` | summary-only | SC2 audit gate; covered by paired test |
|
||
| `migration/50-config-audit-sc2.test.mjs` | summary-only | SC2 audit test; confirmed asserting |
|
||
| `migration/60-rewrite-marketplace.mjs` | summary-only | Prior BLOCKER fixed at :92 — nested source-object emitted + tested |
|
||
| `migration/60-rewrite-marketplace.test.mjs` | summary-only | Asserts the nested `{ source: {...} }` schema shape |
|
||
| `migration/70-thin-catalog.sh` | summary-only | Catalog thinning (3a5f558); covered by paired test |
|
||
| `migration/70-thin-catalog.test.mjs` | summary-only | Catalog thinning test; confirmed asserting |
|
||
| `migration/sc6-check.sh` | summary-only | SC6 DROP detector; negatively tested via sc-checks.test.mjs |
|
||
| `migration/sc2-regression.sh` | summary-only | SC2 regression detector; negatively tested |
|
||
| `migration/99-dryrun.sh` | summary-only | Dry-run harness (5e00f92); SC6/SC2 block confirmed correct |
|
||
| `migration/99-dryrun.test.mjs` | summary-only | Dry-run integration test; confirmed asserting |
|
||
| `migration/sc-checks.test.mjs` | summary-only | Negative tests for the sc6/sc2 detectors (both branches) |
|
||
| `migration/plugin-map.json` | summary-only | Extraction map; ms-ai-architect blob_strip_safe reset to null (preflight-computed) |
|
||
| `migration/RUNBOOK.md` | summary-only | Operator-window runbook; git-filter-repo + python3 preconditions added |
|
||
| `scripts/sync-design-system.mjs` | summary-only | Design-system sync; no migration-contract regression surface |
|
||
| `scripts/sync-design-system.test.mjs` | summary-only | DS-sync test; confirmed asserting |
|
||
| `docs/marketplace-polyrepo-migration/review.md` | skip | Review artifact itself — excluded from being reviewed as delivered code |
|
||
|
||
Note: the brief §6 nested-shape criterion is FULLY met. 0 files silently dropped; 0 deep-review treatments (all summary-only); 1 skip (the review artifact), recorded above.
|
||
|
||
## Findings (BLOCKER)
|
||
|
||
_None._
|
||
|
||
## Findings (MAJOR)
|
||
|
||
_None._
|
||
|
||
## Findings (MINOR)
|
||
|
||
_None._
|
||
|
||
## Findings (SUGGESTION)
|
||
|
||
_None._
|
||
|
||
## Remediation Summary
|
||
|
||
- BLOCKER: 0
|
||
- MAJOR: 0
|
||
- MINOR: 0
|
||
- SUGGESTION: 0
|
||
|
||
All 12 findings across the remediation arc are resolved: the 1 BLOCKER + 6 MAJOR + 3 MINOR from the first review (commits 86208da, fef4b33), plus the 2 MAJOR surfaced by the re-review's deep read of `30-fix-references.mjs` (this commit). The delivered Claude-run local/reversible half (Steps 1–11) is verified — full local dry-run 11/11 targets, 0 pushes; every paired unit suite green; the externalised marketplace.json emits the schema-correct nested source-object. The window-only steps (Forgejo `auto_init:false` create, post-strip history push, HTTPS `url`+`ref` resolution) remain correctly absent from this code — operator-gated by D8 / the RUNBOOK. The operator window (RUNBOOK.md) is no longer gated by a review BLOCK.
|
||
|
||
```json
|
||
{
|
||
"verdict": "ALLOW",
|
||
"counts": { "BLOCKER": 0, "MAJOR": 0, "MINOR": 0, "SUGGESTION": 0 },
|
||
"normalization": {
|
||
"mode": "default (high-effort)",
|
||
"pass3_skipped": true,
|
||
"pass3_skip_reason": "high-effort maximal-discipline standing instruction; Cloudflare reasonableness filter bypassed per v5.1.1",
|
||
"rule_key_substitutions": 0,
|
||
"note": "Final pass after full remediation. All 12 arc findings resolved; both 30-fix-references.mjs MAJORs re-verified RESOLVED by the independent code-correctness reviewer with no new correctness issue."
|
||
},
|
||
"findings": []
|
||
}
|
||
```
|