stack/docs/plans/reviews/2026-09-10_mosaic-i1-verdict.md

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.