Record increment 1 approval after contamination fix (#1499)

2026-09-10 15:23:58 -05:00
parent e5a9a05ba2
commit 5446097483
3 changed files with 56 additions and 0 deletions
@@ -178,3 +178,5 @@ are never rewritten or removed; corrections are new entries.
 - 2026-09-10 — darkwing — GATE7-FINAL-FILBERT-1 APPROVED; gate 7 closed with owner mechanism (a). Six units committed at 86e009dd, pushed, remote verified. Registry plan fully resolved; no implementation authorized without a separate charter.

 - 2026-09-10 — darkwing — #1499 opened; M20 increment-1 charter drafted (docs/plans/2026-09-10_m20-increment1-charter.md): packages/mosaic schema/validation/CLI slice, pi 0.85.1 pin, no materializer/refresh/deployment. Awaiting owner approval; no implementation started.
+
+- 2026-09-10 — darkwing for Filbert — MOSAIC-I1-FILBERT-1 REQUEST CHANGES (sage contamination, dangling export) fixed at e5a9a05b; follow-up APPROVED after clean-archive reruns (package 15/15, launcher 5/5). I1-F3 baseUrl hardening deferred to a later increment. Push and owner acceptance next.
@@ -0,0 +1,50 @@
+# MOSAIC-I1-FILBERT-1 (#1499) — increment 1 implementation verdict
+
+Reviewer: Filbert (independent). Author: Darkwing. Candidate: `557aba0f9eba6cc413850e62196c20349ce4c24d` on refactor (not pushed). All work in disposable `git archive` copies of the candidate and baseline `54c12312`; no pushes, no live auth, no shared-log edits.
+
+## Verdict: REQUEST CHANGES
+
+The package itself is solid: 15/15 tests, a clean secret-free CLI, an exact 0.85.1 pin whose lockfile integrity matches the PI-REFRESH-ROCKO-1 record. But one change in the candidate breaks the charter's "existing suites remain green" obligation on the committed tree, and the review request's scope description did not disclose it.
+
+## Blocking finding I1-F1: test-darkwing-launch.mjs depends on uncommitted state
+
+`scripts/test-darkwing-launch.mjs` adds a fourth agent, `sage` (provider `zai`, model `glm-5.3`, thinking `high`), to the per-agent loop. `agents/sage` does not exist in git — never committed on any branch — it exists only as uncommitted live-worktree state. On the clean candidate tree (git archive of `557aba0f`), the suite fails: 4 pass, 1 fail at the sage iteration (ENOENT copying `agents/sage`). The suite passes only inside the dirty live worktree where the coordinator ran it. The stated scope ("0.84.4→0.85.1 fixture updates… Nothing else") is therefore both inaccurate and violated: this hunk is not a version bump, and it makes the commit's own suite red on the commit's own tree.
+
+Required fix: remove the three sage hunks (restore the three-agent enumeration), or land `agents/sage` through its own reviewed change first — which is outside this charter's allowed paths. Then re-freeze; I will rerun the darkwing suite and the package tests only.
+
+## Non-blocking findings
+
+- **I1-F2:** `packages/mosaic/package.json` exports advertise `./ids.mjs`, but `src/ids.mjs` does not exist. Lazy resolution hides it from the tests; any consumer importing that subpath fails. Remove the export or add the module in the fix-up commit.
+- **I1-F3 (hardening note):** a `custom-endpoint` provider with no `harnesses.pi` entry skips baseUrl/insecure-transport validation entirely. Uncovered shape; belongs in a later schema increment, recorded here so it is not forgotten.
+
+## Charter criteria, as verified
+
+1. **Package suites:** 15/15 pass (`node --test tests/` in the disposable candidate tree). Coverage maps to the plan's registry-schema list: unknown keys, ID regex including traversal shapes, path/name match, kind-specific requireds (native/custom-endpoint, insecure-transport rule, native rejecting `allowInsecureTransport`), reference integrity, duplicate IDs, missing provider/account, credential-type support, fork-pin field.
+2. **CLI:** `validate` exits 0 with `valid` on the fixture tree; refuses invalid trees exit 1 with precise `path: code: detail` errors, no stack spew, no secret echo (broken-JSON, duplicate, missing-reference cases all tested); usage errors exit 2; `list` prints only id/kind, ref/name/type, id projections — no credential-shaped fields, verified including a planted `credential.json` marker that is never read or echoed.
+3. **Pin and suites:** `package.json` pins `0.85.1` exactly; the lockfile entry carries the same tarball URL and `sha512-FGRN+…` integrity that PI-REFRESH-ROCKO-1 recorded from the npm registry — the pin resolves to exactly the investigated artifact. In the disposable candidate tree: config, auth, release green; conductor 17/0 green (my first conductor run failed only because a `git archive` tree lacks `.git` — rerun with a scratch git init passed; environment artifact, not a candidate defect). Task suite shows exactly one failure, `no agent identity on headless run`, identical at stashed baseline `54c12312` — parity confirmed, pre-existing, not counted against the candidate. The once-observed mock-adapter failure did not reproduce in any of my runs either; flaky-unreproduced stands.
+4. **No hidden paths:** the loader uses only `readFile`/`readdir`/`stat` on the supplied root; the CLI writes only stdout/stderr; tests use fixtures and `mkdtemp` temp roots; no network anywhere; `credential.json` is never read (code review plus the planted-marker test). `scripts/auth.sh` untouched; no writes to `~/.pi` or dataRoot.
+
+## Effect
+
+REQUEST CHANGES blocks the push. With I1-F1 fixed (and ideally I1-F2), a re-frozen candidate needs only the darkwing-suite and package-test reruns from me; everything else in this verdict carries over. No increment 2 is authorized by this review in any case.
+
+---
+
+# Follow-up verdict (appended after I1-F1/F2 fix)
+
+Fix commit `e5a9a05b` (with the intervening request-record commit `e6e30b6f`). The delta from `557aba0f` is exactly the two fixes: the sage hunks are gone from `scripts/test-darkwing-launch.mjs` (agent enumeration back to darkwing/dewey/filbert, 0.85.1 fixture changes retained) and the dangling `./ids.mjs` export is removed from `packages/mosaic/package.json`. Pin, lockfile, package sources, and all other candidate files are untouched.
+
+## Verdict: APPROVED
+
+Rerun per the stated scope, on a clean `git archive` of `e5a9a05b` (no live worktree state):
+
+- Package tests: 15/15 pass, exit 0.
+- Launcher suites (`test-darkwing-launch.mjs` + `test-rocko-launch.mjs` combined): 5 tests, 5 pass, 0 fail — the suite that failed 4/1 on the dirty-state-dependent candidate is green on the committed tree.
+- CLI spot-check: `validate` on the fixture tree exits 0 with `valid`.
+- Committed tree contains no `agents/sage`; the package manifest no longer references `ids.mjs`.
+
+All other verifications from the REQUEST CHANGES verdict carry over unchanged: schema coverage, secret-free CLI with the planted-marker control, exact 0.85.1 pin with lockfile integrity matching PI-REFRESH-ROCKO-1's npm record, config/auth/conductor/release green, task-suite parity for the single pre-existing headless failure, and no hidden mutation/network/credential paths. I1-F3 remains acknowledged later hardening, not a defect of this increment.
+
+## Effect
+
+`e5a9a05b` (with `e6e30b6f` and `557aba0f`) is approved to push as increment 1. Per charter criterion 5, Jason accepts the completion before any increment 2 planning. This review authorizes no increment 2 and no deployment.