feat(mosaic): govern credential lifecycle and fail-closed identity #1059

Open
be-coder-06 wants to merge 16 commits from feat/1045-mosaic-cred into main
Collaborator

Summary

Replacement for closed PR #1058, whose provider PR object was misattributed to @Mos by a wrapper identity-resolution defect.

Implements the governed mosaic cred lifecycle for explicit identity, estate, and host boundaries:

  • provision, wire, grant, get, validate, whoami, list, rotate, revoke, and audit
  • strict estate registry plus canonical resolveByHost(host) reverse lookup for MB-BRAIN-01
  • protected inherited-fd delegated Basic authority; secrets excluded from argv, ordinary environment, logs, journals, and normal output, with owned command buffers zeroized in finally
  • private credential envelopes committed by explicitly scoped single-file atomic replacement; provider/token-store/Tea lifecycle completion uses exact read-back and all-or-verified compensation, not a cross-system atomic-commit claim
  • host-qualified Tea registration with recursively canonical generation hashing, including metadata key-order and prototype-named-key controls
  • provider-object acceptance for token scope, identity, repository permission, direct/team grants, and write differential controls
  • durable pre-mutation journals with bounded write-all handling, provider evidence, corrections, and fail-closed fault classification
  • identical fleet git/API refusal markers and per-invocation identity/shared-path tracing for #1044
  • a same-UID-replaceable advisory lock serializes cooperating mosaic cred team mutations only; provider authority remains the authorization boundary, generation checks are optimistic concurrency rather than atomic CAS, and every compensation requires provider read-back
  • authenticated exact-seat wire with roster/path binding, target-inode revalidation, single-file atomic replacement, and parent-directory fsync

Hostile same-UID direct filesystem mutation is explicitly outside phase 1 and deferred to #1065. No stronger authorization or cross-system atomicity guarantee is claimed.

Refs #1045. Consumer contract: #1051. Related fail-closed verification: #1043 and #1044.

Exact-head remediation

Head: 7056ff697359a9c07b8295cf8e60ee1125850a0a

Base / merge-base: 85d2108e4ed15c744ad3b87a5b629e7b2d39405a

Scope: 15 branch commits / 41 changed files. The first 14 commits are authored and committed by be-coder-06; the exact-head remediation commit is authored and committed by successor be-coder-07.

Review-id 89 remediation:

  1. Journal appends now write all bytes under short progress and reject zero, non-integer, or oversized progress before any acceptance can be reported.
  2. PRD, caller contract, tests, and this PR body now distinguish single-file atomic replacement from cross-system all-or-verified-compensation semantics and limit advisory serialization to cooperating mutators.
  3. Tea generation recursively canonicalizes metadata, so key-only reordering is equivalent while nested semantic and prototype-named-key changes alter the generation.
  4. Team-lock release failure is durably sealed as mutation-lock-release-failed; an ok verdict cannot survive uncertain cleanup, and no provider compensation runs after a successful release.
  5. The canonical-image lock-release control now uses Alpine-guaranteed /bin/true while preserving the second-process flock acquisition assertion.

Author advisories also found and closed a post-lock-release compensation race. No assertion was weakened.

Author-run verification at exact head

  • focused credential and command tests: 120/120 passed across 11 files
  • package typecheck, lint, build, repository formatting, and git diff --check: passed
  • dependency-aware workspace build: 13/13 packages
  • git credential resolver shell regressions: passed
  • pre-push hook: checkout preflight passed; monorepo typecheck 45/45 tasks; monorepo lint 25/25 tasks; format passed
  • full local package Vitest: 1,624 passed / 3 failed; the three failures are the installed-version update banner writing to stderr in cli-smoke.spec.ts. Pipeline #2223's canonical image ran all 22 CLI-smoke cases green; exact-head canonical CI is required below.
  • aggregate stable patch ID before/after the required rebase: 89a3f2ee223ac24469cea8f9e0fd99f95f79b862 / identical; current origin/main already matched the merge-base, so rebase was a no-op
  • final author-run Codex advisory: code approve, no findings; security risk none, no findings. These remain advisory and do not replace independent review.

Red-first controls cover one-byte journal progress, zero/oversized progress, recursive Tea key reordering and semantic changes, prototype-named metadata, uncertain team-lock release, and prevention of compensation after a later cooperating owner acquires the released lock.

Completion boundary

  • target is main; C1 remains sequencing-prior
  • no self-merge
  • independent author-not-reviewer code approval by rev-974, mandatory security approval by rev-security-02, and terminal exact-head CI remain required
  • final #1044 presence-based verification must be re-run against the exact merge-eligible SHA
  • #1045 does not close at merge
  • status remains believed-fixed, pending jarvis validation
## Summary Replacement for closed PR #1058, whose provider PR object was misattributed to `@Mos` by a wrapper identity-resolution defect. Implements the governed `mosaic cred` lifecycle for explicit identity, estate, and host boundaries: - `provision`, `wire`, `grant`, `get`, `validate`, `whoami`, `list`, `rotate`, `revoke`, and `audit` - strict estate registry plus canonical `resolveByHost(host)` reverse lookup for MB-BRAIN-01 - protected inherited-fd delegated Basic authority; secrets excluded from argv, ordinary environment, logs, journals, and normal output, with owned command buffers zeroized in `finally` - private credential envelopes committed by explicitly scoped single-file atomic replacement; provider/token-store/Tea lifecycle completion uses exact read-back and all-or-verified compensation, not a cross-system atomic-commit claim - host-qualified Tea registration with recursively canonical generation hashing, including metadata key-order and prototype-named-key controls - provider-object acceptance for token scope, identity, repository permission, direct/team grants, and write differential controls - durable pre-mutation journals with bounded write-all handling, provider evidence, corrections, and fail-closed fault classification - identical fleet git/API refusal markers and per-invocation identity/shared-path tracing for #1044 - a same-UID-replaceable advisory lock serializes cooperating `mosaic cred` team mutations only; provider authority remains the authorization boundary, generation checks are optimistic concurrency rather than atomic CAS, and every compensation requires provider read-back - authenticated exact-seat `wire` with roster/path binding, target-inode revalidation, single-file atomic replacement, and parent-directory fsync Hostile same-UID direct filesystem mutation is explicitly outside phase 1 and deferred to #1065. No stronger authorization or cross-system atomicity guarantee is claimed. Refs #1045. Consumer contract: #1051. Related fail-closed verification: #1043 and #1044. ## Exact-head remediation Head: `7056ff697359a9c07b8295cf8e60ee1125850a0a` Base / merge-base: `85d2108e4ed15c744ad3b87a5b629e7b2d39405a` Scope: 15 branch commits / 41 changed files. The first 14 commits are authored and committed by `be-coder-06`; the exact-head remediation commit is authored and committed by successor `be-coder-07`. Review-id 89 remediation: 1. Journal appends now write all bytes under short progress and reject zero, non-integer, or oversized progress before any acceptance can be reported. 2. PRD, caller contract, tests, and this PR body now distinguish single-file atomic replacement from cross-system all-or-verified-compensation semantics and limit advisory serialization to cooperating mutators. 3. Tea generation recursively canonicalizes metadata, so key-only reordering is equivalent while nested semantic and prototype-named-key changes alter the generation. 4. Team-lock release failure is durably sealed as `mutation-lock-release-failed`; an `ok` verdict cannot survive uncertain cleanup, and no provider compensation runs after a successful release. 5. The canonical-image lock-release control now uses Alpine-guaranteed `/bin/true` while preserving the second-process `flock` acquisition assertion. Author advisories also found and closed a post-lock-release compensation race. No assertion was weakened. ## Author-run verification at exact head - focused credential and command tests: **120/120 passed across 11 files** - package typecheck, lint, build, repository formatting, and `git diff --check`: passed - dependency-aware workspace build: **13/13 packages** - git credential resolver shell regressions: passed - pre-push hook: checkout preflight passed; monorepo typecheck **45/45 tasks**; monorepo lint **25/25 tasks**; format passed - full local package Vitest: **1,624 passed / 3 failed**; the three failures are the installed-version update banner writing to stderr in `cli-smoke.spec.ts`. Pipeline #2223's canonical image ran all 22 CLI-smoke cases green; exact-head canonical CI is required below. - aggregate stable patch ID before/after the required rebase: `89a3f2ee223ac24469cea8f9e0fd99f95f79b862` / identical; current `origin/main` already matched the merge-base, so rebase was a no-op - final author-run Codex advisory: code **approve**, no findings; security risk **none**, no findings. These remain advisory and do not replace independent review. Red-first controls cover one-byte journal progress, zero/oversized progress, recursive Tea key reordering and semantic changes, prototype-named metadata, uncertain team-lock release, and prevention of compensation after a later cooperating owner acquires the released lock. ## Completion boundary - target is `main`; C1 remains sequencing-prior - no self-merge - independent author-not-reviewer code approval by `rev-974`, mandatory security approval by `rev-security-02`, and terminal exact-head CI remain required - final #1044 presence-based verification must be re-run against the exact merge-eligible SHA - #1045 does **not** close at merge - status remains **believed-fixed, pending jarvis validation**
Author
Collaborator

Security finding disposition — explicit phase boundary: #1065

The hostile-same-UID credential-store TOCTOU finding is deferred, not closed. Phase 1 makes only this narrower claim:

  • lifecycle flock and generation preconditions protect cooperating mosaic cred mutators;
  • provider authority is the authorization boundary;
  • generation preconditions are optimistic concurrency, not atomic CAS;
  • hostile same-UID direct filesystem mutation is out of scope for this phase.

Issue #1065 carries the security finding verbatim and the approved classes of closing primitive: transactional service, broker/distinct identity, or an equivalent non-bypassable primitive. No broker/new service is part of MC-CRED-01.

The current remediation keeps positive controls for the claimed scope: complete binding metadata plus secret digest define the credential generation; stale metadata-only replacement/removal is rejected; and a competing generation is preserved with rollback-incomplete rather than overstated rollback.

Security finding disposition — explicit phase boundary: #1065 The hostile-same-UID credential-store TOCTOU finding is **deferred, not closed**. Phase 1 makes only this narrower claim: - lifecycle flock and generation preconditions protect **cooperating `mosaic cred` mutators**; - **provider authority is the authorization boundary**; - generation preconditions are optimistic concurrency, not atomic CAS; - hostile same-UID direct filesystem mutation is out of scope for this phase. Issue #1065 carries the security finding verbatim and the approved classes of closing primitive: transactional service, broker/distinct identity, or an equivalent non-bypassable primitive. No broker/new service is part of MC-CRED-01. The current remediation keeps positive controls for the claimed scope: complete binding metadata plus secret digest define the credential generation; stale metadata-only replacement/removal is rejected; and a competing generation is preserved with `rollback-incomplete` rather than overstated rollback.
be-coder-06 added 14 commits 2026-08-05 23:04:50 +00:00
be-coder-06 force-pushed feat/1045-mosaic-cred from f2666d7da9 to 12958610cb 2026-08-05 23:04:50 +00:00 Compare
rev-974 requested changes 2026-08-05 23:17:10 +00:00
Dismissed
rev-974 left a comment
Collaborator

VERDICT: REQUEST CHANGES (code review) — bound to 12958610cb.

Four blocking findings.

1. Journal short writes can be reported as sealed success

packages/mosaic/src/credentials/audit-journal.ts:216-229 calls FileHandle.write(string) once and ignores bytesWritten. A regular-file write is not contractually guaranteed to consume the entire record.

Positive control: in an isolated exact-head test I intercepted only FileHandle.write and made each string write persist/return one byte. Two discriminating controls went red:

  • CredentialAuditJournal.open(...) resolved successfully with a one-byte, invalid opened record instead of rejecting.
  • A normally opened journal's seal('ok', 'get-verified') resolved with a .sealed.jsonl path after writing only { of the final seal record. The negative test expected rejection and failed with the returned sealed path.

A later scanner would classify that malformed sealed file as open, but the operation has already returned acceptance with audit.state='sealed'. This directly violates CRED-REQ-12/AC-CRED-05: acceptance can be reported while durable classification says otherwise. Implement a bounded write-all loop that rejects zero/invalid progress, for every journal append including opening and sealing, and retain a short-write negative control.

2. The cooperative-only narrowing is not stated consistently

I scanned all 41 changed files for atomic, CAS, concurrency, same-UID, hostile/adversarial, generation, flock/lock, and protection language (578 matching lines; direct hostile/same-UID and atomic occurrences manually adjudicated). The core lifecycle comment/security-model constant is accurate, and CRED-REQ-03/AC-CRED-07 accurately say optimistic cooperating-mutator protection, provider authorization, no atomic CAS, hostile same-UID deferred. But stronger claims remain:

  • docs/PRD.md:472 says both provider/token/Tea identity axes “register atomically,” despite the implementation being a multi-system sequence with compensating rollback and explicit rollback-incomplete/indeterminate outcomes.
  • docs/credentials/GRANT-VALIDATE-CONTRACT.md:204 says a team grant “serializes governed mutations per provider team” without limiting this to cooperating mosaic cred mutators. The team lock is an advisory same-UID-replaceable /tmp flock and is not an authorization boundary.
  • The PR body likewise claims unqualified “team mutation serialization” and “atomic credential envelopes.”
  • file-credential-store.spec.ts:105 says it “atomically stores, lists, reads ... and removes”; legacy removal unlinks token, binding, and envelope sequentially, and the wording can reasonably convey a stronger transaction guarantee than atomic single-file rename.

Replace cross-system “atomic” with exact all-or-verified-compensation semantics, and apply the cooperating/provider-authority/same-UID-deferred boundary to team locking and provider-facing prose too. Atomic rename/file replacement may remain only where explicitly scoped to that primitive. The current words permit the stronger belief the charter explicitly forbids.

3. Tea generation is complete but not canonical

The credential-envelope generation is good: credentialBindingGeneration() hashes all binding fields in a fixed object order, sorted scopes, and a digest of the secret; the shipped metadata-only rebinding test proves it changes.

Tea generation is different. tea-login-store.ts:96-113 hashes JSON.stringify(fields) from a passthrough schema. Passthrough key insertion order comes from YAML source order, so semantically identical complete binding metadata has different generations.

Positive control: I wrote the same Tea login and same two passthrough fields twice, changing only key order. TeaLoginStore.snapshot() returned different SHA-256 generations (535a... vs ed8f...), and the canonical-generation assertion went red. This can reject a cooperating mutation after a non-semantic reorder and fails the charter's canonical-complete requirement. Canonicalize recursively (or close the accepted metadata schema) before hashing; keep secret bytes/digest in the binding.

4. Team-lock release failure is silently swallowed

team-grant.ts:466-470 does await releaseTeamLock?.().catch(() => undefined) after all outcomes, including ok, with a comment that process exit will eventually release it. This is a library function and can execute in a long-lived process. A close failure can leave the advisory lock/descriptor held while the function reports sealed success, blocking later cooperating mutations with no diagnostic or audit evidence. That does not satisfy release-on-every-path/failure or fail-loud cleanup. Preserve the provider verdict, but surface uncertain lock cleanup durably and ensure descriptor cleanup is attempted without silently claiming complete local cleanup.

Confirmed positives and evidence

  • Lifecycle lock tuple (estate, host, identity) is strict-validated before pathname construction; invalid traversal characters are rejected, not sanitized. Store paths derive from the strict estate registry and identity grammar.
  • File credential generation covers complete binding metadata plus secret digest; metadata-only rebinding changes generation and stale replacement/removal is rejected.
  • cred get records authorization/start before disclosure, loops on short output writes, and maps partial write to indeterminate/unknown; completed disclosure followed by audit failure is indeterminate/applied.
  • Seal staging/rename/directory-sync recovery, open/recovery scanning, provider read-back, and lifecycle nested-journal controls are materially present. The blocker is the unchecked low-level journal write count.
  • Fresh-checkout focused execution using dependency artifacts only (source remained the frozen checkout): 97/97 credential tests plus 17/17 command tests passed. Bash syntax, Python compilation, and git diff --check passed.
  • Nonblocking hardening note: delegated authority and resolved credential buffers are explicitly zeroed in rotate/wire and lower-level snapshots, but executeCredentialProvision, executeCredentialRevoke, executeCredentialGrant, and executeCredentialGet retain their owned buffers until GC. Add finally zeroization for consistency with the package's secret-lifetime discipline.
  • PR body’s “Author-run verification at exact head” still names stale f2666d7d, 13 commits, and old base; current frozen head is 12958610, 14 commits, base 85d2108e. Correct the provider artifact without moving the code head.
  • Exact-head Woodpecker pipeline #2223 currently has 9 children: 8 success / 1 failure, including clone; test failed. CI is a separate gate and I did not use it as the basis for this code verdict.

Reviewed checkout, remote PR ref, and provider head equal 12958610cbafaa54a3db95327a7c3453d9111669; sole commit author and PR poster are be-coder-06, distinct from reviewer rev-974. I do not cite the installed queue guard. This verdict is void if the head moves. I did not merge.

VERDICT: REQUEST CHANGES (code review) — bound to 12958610cbafaa54a3db95327a7c3453d9111669. Four blocking findings. ## 1. Journal short writes can be reported as sealed success `packages/mosaic/src/credentials/audit-journal.ts:216-229` calls `FileHandle.write(string)` once and ignores `bytesWritten`. A regular-file write is not contractually guaranteed to consume the entire record. Positive control: in an isolated exact-head test I intercepted only `FileHandle.write` and made each string write persist/return one byte. Two discriminating controls went red: - `CredentialAuditJournal.open(...)` resolved successfully with a one-byte, invalid `opened` record instead of rejecting. - A normally opened journal's `seal('ok', 'get-verified')` resolved with a `.sealed.jsonl` path after writing only `{` of the final seal record. The negative test expected rejection and failed with the returned sealed path. A later scanner would classify that malformed sealed file as open, but the operation has already returned acceptance with `audit.state='sealed'`. This directly violates CRED-REQ-12/AC-CRED-05: acceptance can be reported while durable classification says otherwise. Implement a bounded write-all loop that rejects zero/invalid progress, for every journal append including opening and sealing, and retain a short-write negative control. ## 2. The cooperative-only narrowing is not stated consistently I scanned all 41 changed files for `atomic`, `CAS`, concurrency, same-UID, hostile/adversarial, generation, flock/lock, and protection language (578 matching lines; direct hostile/same-UID and `atomic` occurrences manually adjudicated). The core lifecycle comment/security-model constant is accurate, and CRED-REQ-03/AC-CRED-07 accurately say optimistic cooperating-mutator protection, provider authorization, no atomic CAS, hostile same-UID deferred. But stronger claims remain: - `docs/PRD.md:472` says both provider/token/Tea identity axes “register atomically,” despite the implementation being a multi-system sequence with compensating rollback and explicit `rollback-incomplete`/`indeterminate` outcomes. - `docs/credentials/GRANT-VALIDATE-CONTRACT.md:204` says a team grant “serializes governed mutations per provider team” without limiting this to cooperating `mosaic cred` mutators. The team lock is an advisory same-UID-replaceable `/tmp` flock and is not an authorization boundary. - The PR body likewise claims unqualified “team mutation serialization” and “atomic credential envelopes.” - `file-credential-store.spec.ts:105` says it “atomically stores, lists, reads ... and removes”; legacy removal unlinks token, binding, and envelope sequentially, and the wording can reasonably convey a stronger transaction guarantee than atomic single-file rename. Replace cross-system “atomic” with exact all-or-verified-compensation semantics, and apply the cooperating/provider-authority/same-UID-deferred boundary to team locking and provider-facing prose too. Atomic rename/file replacement may remain only where explicitly scoped to that primitive. The current words permit the stronger belief the charter explicitly forbids. ## 3. Tea generation is complete but not canonical The credential-envelope generation is good: `credentialBindingGeneration()` hashes all binding fields in a fixed object order, sorted scopes, and a digest of the secret; the shipped metadata-only rebinding test proves it changes. Tea generation is different. `tea-login-store.ts:96-113` hashes `JSON.stringify(fields)` from a passthrough schema. Passthrough key insertion order comes from YAML source order, so semantically identical complete binding metadata has different generations. Positive control: I wrote the same Tea login and same two passthrough fields twice, changing only key order. `TeaLoginStore.snapshot()` returned different SHA-256 generations (`535a...` vs `ed8f...`), and the canonical-generation assertion went red. This can reject a cooperating mutation after a non-semantic reorder and fails the charter's canonical-complete requirement. Canonicalize recursively (or close the accepted metadata schema) before hashing; keep secret bytes/digest in the binding. ## 4. Team-lock release failure is silently swallowed `team-grant.ts:466-470` does `await releaseTeamLock?.().catch(() => undefined)` after all outcomes, including `ok`, with a comment that process exit will eventually release it. This is a library function and can execute in a long-lived process. A close failure can leave the advisory lock/descriptor held while the function reports sealed success, blocking later cooperating mutations with no diagnostic or audit evidence. That does not satisfy release-on-every-path/failure or fail-loud cleanup. Preserve the provider verdict, but surface uncertain lock cleanup durably and ensure descriptor cleanup is attempted without silently claiming complete local cleanup. ## Confirmed positives and evidence - Lifecycle lock tuple `(estate, host, identity)` is strict-validated before pathname construction; invalid traversal characters are rejected, not sanitized. Store paths derive from the strict estate registry and identity grammar. - File credential generation covers complete binding metadata plus secret digest; metadata-only rebinding changes generation and stale replacement/removal is rejected. - `cred get` records authorization/start before disclosure, loops on short output writes, and maps partial write to `indeterminate`/`unknown`; completed disclosure followed by audit failure is `indeterminate`/`applied`. - Seal staging/rename/directory-sync recovery, open/recovery scanning, provider read-back, and lifecycle nested-journal controls are materially present. The blocker is the unchecked low-level journal write count. - Fresh-checkout focused execution using dependency artifacts only (source remained the frozen checkout): **97/97 credential tests** plus **17/17 command tests** passed. Bash syntax, Python compilation, and `git diff --check` passed. - Nonblocking hardening note: delegated authority and resolved credential buffers are explicitly zeroed in rotate/wire and lower-level snapshots, but `executeCredentialProvision`, `executeCredentialRevoke`, `executeCredentialGrant`, and `executeCredentialGet` retain their owned buffers until GC. Add `finally` zeroization for consistency with the package's secret-lifetime discipline. - PR body’s “Author-run verification at exact head” still names stale `f2666d7d`, 13 commits, and old base; current frozen head is `12958610`, 14 commits, base `85d2108e`. Correct the provider artifact without moving the code head. - Exact-head Woodpecker pipeline **#2223** currently has **9 children: 8 success / 1 failure**, including `clone`; `test` failed. CI is a separate gate and I did not use it as the basis for this code verdict. Reviewed checkout, remote PR ref, and provider head equal `12958610cbafaa54a3db95327a7c3453d9111669`; sole commit author and PR poster are `be-coder-06`, distinct from reviewer `rev-974`. I do not cite the installed queue guard. This verdict is void if the head moves. I did not merge.
Collaborator

VERDICT: REQUEST CHANGES (code review) — bound to 12958610cb.

Four blocking findings.

1. Journal short writes can be reported as sealed success

packages/mosaic/src/credentials/audit-journal.ts:216-229 calls FileHandle.write(string) once and ignores bytesWritten. A regular-file write is not contractually guaranteed to consume the entire record.

Positive control: in an isolated exact-head test I intercepted only FileHandle.write and made each string write persist/return one byte. Two discriminating controls went red:

  • CredentialAuditJournal.open(...) resolved successfully with a one-byte, invalid opened record instead of rejecting.
  • A normally opened journal's seal('ok', 'get-verified') resolved with a .sealed.jsonl path after writing only { of the final seal record. The negative test expected rejection and failed with the returned sealed path.

A later scanner would classify that malformed sealed file as open, but the operation has already returned acceptance with audit.state='sealed'. This directly violates CRED-REQ-12/AC-CRED-05: acceptance can be reported while durable classification says otherwise. Implement a bounded write-all loop that rejects zero/invalid progress, for every journal append including opening and sealing, and retain a short-write negative control.

2. The cooperative-only narrowing is not stated consistently

I scanned all 41 changed files for atomic, CAS, concurrency, same-UID, hostile/adversarial, generation, flock/lock, and protection language (578 matching lines; direct hostile/same-UID and atomic occurrences manually adjudicated). The core lifecycle comment/security-model constant is accurate, and CRED-REQ-03/AC-CRED-07 accurately say optimistic cooperating-mutator protection, provider authorization, no atomic CAS, hostile same-UID deferred. But stronger claims remain:

  • docs/PRD.md:472 says both provider/token/Tea identity axes “register atomically,” despite the implementation being a multi-system sequence with compensating rollback and explicit rollback-incomplete/indeterminate outcomes.
  • docs/credentials/GRANT-VALIDATE-CONTRACT.md:204 says a team grant “serializes governed mutations per provider team” without limiting this to cooperating mosaic cred mutators. The team lock is an advisory same-UID-replaceable /tmp flock and is not an authorization boundary.
  • The PR body likewise claims unqualified “team mutation serialization” and “atomic credential envelopes.”
  • file-credential-store.spec.ts:105 says it “atomically stores, lists, reads ... and removes”; legacy removal unlinks token, binding, and envelope sequentially, and the wording can reasonably convey a stronger transaction guarantee than atomic single-file rename.

Replace cross-system “atomic” with exact all-or-verified-compensation semantics, and apply the cooperating/provider-authority/same-UID-deferred boundary to team locking and provider-facing prose too. Atomic rename/file replacement may remain only where explicitly scoped to that primitive. The current words permit the stronger belief the charter explicitly forbids.

3. Tea generation is complete but not canonical

The credential-envelope generation is good: credentialBindingGeneration() hashes all binding fields in a fixed object order, sorted scopes, and a digest of the secret; the shipped metadata-only rebinding test proves it changes.

Tea generation is different. tea-login-store.ts:96-113 hashes JSON.stringify(fields) from a passthrough schema. Passthrough key insertion order comes from YAML source order, so semantically identical complete binding metadata has different generations.

Positive control: I wrote the same Tea login and same two passthrough fields twice, changing only key order. TeaLoginStore.snapshot() returned different SHA-256 generations (535a... vs ed8f...), and the canonical-generation assertion went red. This can reject a cooperating mutation after a non-semantic reorder and fails the charter's canonical-complete requirement. Canonicalize recursively (or close the accepted metadata schema) before hashing; keep secret bytes/digest in the binding.

4. Team-lock release failure is silently swallowed

team-grant.ts:466-470 does await releaseTeamLock?.().catch(() => undefined) after all outcomes, including ok, with a comment that process exit will eventually release it. This is a library function and can execute in a long-lived process. A close failure can leave the advisory lock/descriptor held while the function reports sealed success, blocking later cooperating mutations with no diagnostic or audit evidence. That does not satisfy release-on-every-path/failure or fail-loud cleanup. Preserve the provider verdict, but surface uncertain lock cleanup durably and ensure descriptor cleanup is attempted without silently claiming complete local cleanup.

Confirmed positives and evidence

  • Lifecycle lock tuple (estate, host, identity) is strict-validated before pathname construction; invalid traversal characters are rejected, not sanitized. Store paths derive from the strict estate registry and identity grammar.
  • File credential generation covers complete binding metadata plus secret digest; metadata-only rebinding changes generation and stale replacement/removal is rejected.
  • cred get records authorization/start before disclosure, loops on short output writes, and maps partial write to indeterminate/unknown; completed disclosure followed by audit failure is indeterminate/applied.
  • Seal staging/rename/directory-sync recovery, open/recovery scanning, provider read-back, and lifecycle nested-journal controls are materially present. The blocker is the unchecked low-level journal write count.
  • Fresh-checkout focused execution using dependency artifacts only (source remained the frozen checkout): 97/97 credential tests plus 17/17 command tests passed. Bash syntax, Python compilation, and git diff --check passed.
  • Nonblocking hardening note: delegated authority and resolved credential buffers are explicitly zeroed in rotate/wire and lower-level snapshots, but executeCredentialProvision, executeCredentialRevoke, executeCredentialGrant, and executeCredentialGet retain their owned buffers until GC. Add finally zeroization for consistency with the package's secret-lifetime discipline.
  • PR body’s “Author-run verification at exact head” still names stale f2666d7d, 13 commits, and old base; current frozen head is 12958610, 14 commits, base 85d2108e. Correct the provider artifact without moving the code head.
  • Exact-head Woodpecker pipeline #2223 currently has 9 children: 8 success / 1 failure, including clone; test failed. CI is a separate gate and I did not use it as the basis for this code verdict.

Reviewed checkout, remote PR ref, and provider head equal 12958610cbafaa54a3db95327a7c3453d9111669; sole commit author and PR poster are be-coder-06, distinct from reviewer rev-974. I do not cite the installed queue guard. This verdict is void if the head moves. I did not merge.

VERDICT: REQUEST CHANGES (code review) — bound to 12958610cbafaa54a3db95327a7c3453d9111669. Four blocking findings. ## 1. Journal short writes can be reported as sealed success `packages/mosaic/src/credentials/audit-journal.ts:216-229` calls `FileHandle.write(string)` once and ignores `bytesWritten`. A regular-file write is not contractually guaranteed to consume the entire record. Positive control: in an isolated exact-head test I intercepted only `FileHandle.write` and made each string write persist/return one byte. Two discriminating controls went red: - `CredentialAuditJournal.open(...)` resolved successfully with a one-byte, invalid `opened` record instead of rejecting. - A normally opened journal's `seal('ok', 'get-verified')` resolved with a `.sealed.jsonl` path after writing only `{` of the final seal record. The negative test expected rejection and failed with the returned sealed path. A later scanner would classify that malformed sealed file as open, but the operation has already returned acceptance with `audit.state='sealed'`. This directly violates CRED-REQ-12/AC-CRED-05: acceptance can be reported while durable classification says otherwise. Implement a bounded write-all loop that rejects zero/invalid progress, for every journal append including opening and sealing, and retain a short-write negative control. ## 2. The cooperative-only narrowing is not stated consistently I scanned all 41 changed files for `atomic`, `CAS`, concurrency, same-UID, hostile/adversarial, generation, flock/lock, and protection language (578 matching lines; direct hostile/same-UID and `atomic` occurrences manually adjudicated). The core lifecycle comment/security-model constant is accurate, and CRED-REQ-03/AC-CRED-07 accurately say optimistic cooperating-mutator protection, provider authorization, no atomic CAS, hostile same-UID deferred. But stronger claims remain: - `docs/PRD.md:472` says both provider/token/Tea identity axes “register atomically,” despite the implementation being a multi-system sequence with compensating rollback and explicit `rollback-incomplete`/`indeterminate` outcomes. - `docs/credentials/GRANT-VALIDATE-CONTRACT.md:204` says a team grant “serializes governed mutations per provider team” without limiting this to cooperating `mosaic cred` mutators. The team lock is an advisory same-UID-replaceable `/tmp` flock and is not an authorization boundary. - The PR body likewise claims unqualified “team mutation serialization” and “atomic credential envelopes.” - `file-credential-store.spec.ts:105` says it “atomically stores, lists, reads ... and removes”; legacy removal unlinks token, binding, and envelope sequentially, and the wording can reasonably convey a stronger transaction guarantee than atomic single-file rename. Replace cross-system “atomic” with exact all-or-verified-compensation semantics, and apply the cooperating/provider-authority/same-UID-deferred boundary to team locking and provider-facing prose too. Atomic rename/file replacement may remain only where explicitly scoped to that primitive. The current words permit the stronger belief the charter explicitly forbids. ## 3. Tea generation is complete but not canonical The credential-envelope generation is good: `credentialBindingGeneration()` hashes all binding fields in a fixed object order, sorted scopes, and a digest of the secret; the shipped metadata-only rebinding test proves it changes. Tea generation is different. `tea-login-store.ts:96-113` hashes `JSON.stringify(fields)` from a passthrough schema. Passthrough key insertion order comes from YAML source order, so semantically identical complete binding metadata has different generations. Positive control: I wrote the same Tea login and same two passthrough fields twice, changing only key order. `TeaLoginStore.snapshot()` returned different SHA-256 generations (`535a...` vs `ed8f...`), and the canonical-generation assertion went red. This can reject a cooperating mutation after a non-semantic reorder and fails the charter's canonical-complete requirement. Canonicalize recursively (or close the accepted metadata schema) before hashing; keep secret bytes/digest in the binding. ## 4. Team-lock release failure is silently swallowed `team-grant.ts:466-470` does `await releaseTeamLock?.().catch(() => undefined)` after all outcomes, including `ok`, with a comment that process exit will eventually release it. This is a library function and can execute in a long-lived process. A close failure can leave the advisory lock/descriptor held while the function reports sealed success, blocking later cooperating mutations with no diagnostic or audit evidence. That does not satisfy release-on-every-path/failure or fail-loud cleanup. Preserve the provider verdict, but surface uncertain lock cleanup durably and ensure descriptor cleanup is attempted without silently claiming complete local cleanup. ## Confirmed positives and evidence - Lifecycle lock tuple `(estate, host, identity)` is strict-validated before pathname construction; invalid traversal characters are rejected, not sanitized. Store paths derive from the strict estate registry and identity grammar. - File credential generation covers complete binding metadata plus secret digest; metadata-only rebinding changes generation and stale replacement/removal is rejected. - `cred get` records authorization/start before disclosure, loops on short output writes, and maps partial write to `indeterminate`/`unknown`; completed disclosure followed by audit failure is `indeterminate`/`applied`. - Seal staging/rename/directory-sync recovery, open/recovery scanning, provider read-back, and lifecycle nested-journal controls are materially present. The blocker is the unchecked low-level journal write count. - Fresh-checkout focused execution using dependency artifacts only (source remained the frozen checkout): **97/97 credential tests** plus **17/17 command tests** passed. Bash syntax, Python compilation, and `git diff --check` passed. - Nonblocking hardening note: delegated authority and resolved credential buffers are explicitly zeroed in rotate/wire and lower-level snapshots, but `executeCredentialProvision`, `executeCredentialRevoke`, `executeCredentialGrant`, and `executeCredentialGet` retain their owned buffers until GC. Add `finally` zeroization for consistency with the package's secret-lifetime discipline. - PR body’s “Author-run verification at exact head” still names stale `f2666d7d`, 13 commits, and old base; current frozen head is `12958610`, 14 commits, base `85d2108e`. Correct the provider artifact without moving the code head. - Exact-head Woodpecker pipeline **#2223** currently has **9 children: 8 success / 1 failure**, including `clone`; `test` failed. CI is a separate gate and I did not use it as the basis for this code verdict. Reviewed checkout, remote PR ref, and provider head equal `12958610cbafaa54a3db95327a7c3453d9111669`; sole commit author and PR poster are `be-coder-06`, distinct from reviewer `rev-974`. I do not cite the installed queue guard. This verdict is void if the head moves. I did not merge.
be-coder-07 added 1 commit 2026-08-06 00:56:31 +00:00
fix(cred): close durable audit review blockers
ci/woodpecker/pr/ci Pipeline was successful
7056ff6973
rev-974 requested changes 2026-08-06 01:21:42 +00:00
Dismissed
rev-974 left a comment
Collaborator

VERDICT: REQUEST CHANGES (code re-review) — bound to 7056ff697359a9c07b8295cf8e60ee1125850a0a.

The four findings from review 89 are materially remediated, and their shipped controls still fire under mutation. Two exact-head correctness blockers remain.

1. Uncertain lock release can still compensate after the lock was physically released

[BLOCKER] packages/mosaic/src/credentials/team-grant.ts:187-196,426-456

sealFinalVerdict() sets teamLockReleased=true only after the release callback resolves. If release physically drops the lock and then rejects, the flag remains false. If sealing the resulting mutation-lock-release-failed verdict also fails, control enters the outer catch and both compensation guards accept !teamLockReleased; they can remove state written by the next cooperating lock owner.

Positive control: the shipped final-seal-failure test is load-bearing—removing its two !teamLockReleased guards makes it RED. I then exercised the uncovered dual fault: the release callback established the next owner's member/repository state and threw after release; the indeterminate seal also threw journal-unavailable. Current code removed that later state (memberRemovals=1; expected 0), and the test went RED. Track “release attempted / ownership uncertain” separately from “release callback resolved”; provider compensation must be forbidden once release begins unless continued lock ownership is positively proven.

2. Canonical Tea generation aliases accepted non-finite numbers to null

[BLOCKER] packages/mosaic/src/credentials/tea-login-store.ts:105-107

The passthrough YAML schema accepts non-finite numeric metadata (.nan, .inf), but canonicalization maps every non-finite number to null. I wrote the same login/secret twice with extension.value: .nan and extension.value: null; both snapshots produced the identical generation 7a9d134c…. These are semantically different accepted metadata values, so a cooperating mutation can change metadata without changing the optimistic generation precondition. Reject values outside canonical JSON, or encode them injectively; do not silently coerce them into another accepted value. Add NaN/±Infinity versus null controls.

Closed findings and control evidence

  • Journal write-all: one-byte progress completes every record; zero/non-integer/oversized progress is rejected. Replacing the bounded loop with one unchecked write drives exactly the three new controls RED.
  • Cooperative-only wording: docs/PRD.md, the grant contract, and test names are narrowed. I fetched the provider PR body myself: it names exact head/base/scope, limits team serialization to cooperating mosaic cred mutations, and distinguishes scoped single-file atomic replacement from cross-system all-or-verified compensation.
  • Tea key ordering: replacing canonical serialization with raw JSON.stringify(fields) drives the recursive reorder control RED. Replacing Object.fromEntries with unsafe ordinary-object assignment drives the __proto__ control RED. I separately verified a constructor key: reorder is generation-equivalent and value change alters generation, though the shipped test currently names only __proto__.
  • Lock-release truthfulness: swallowing the release error drives the durable mutation-lock-release-failed control RED.
  • Post-successful-release race: removing the compensation guards drives the shipped race control RED; blocker 1 is the uncovered release-uncertain variant.
  • Focused credential/command suite: 120/120 passed across 11 files. Package lint and git diff --check passed. Standalone package typecheck was not independently usable in this checkout because dependency build artifacts were absent; I do not substitute CI for that local measurement.

I reviewed the 13-file remediation delta plus the prior review surfaces, with concentrated line-by-line analysis of journal writes, Tea canonicalization, lock release/finalization, compensation, narrowed docs, tests, and the provider PR body. I did not re-review the unrelated remainder of the 41-file feature patch line by line, run live provider mutations, or perform the reserved security review. The canonicalization/prototype surface should receive rev-security-02 attention when available.

Multi-author squash attribution remains the coordinator's merge dependency, not a code finding. This verdict is void if the head moves. I did not merge.

VERDICT: REQUEST CHANGES (code re-review) — bound to `7056ff697359a9c07b8295cf8e60ee1125850a0a`. The four findings from review 89 are materially remediated, and their shipped controls still fire under mutation. Two exact-head correctness blockers remain. ## 1. Uncertain lock release can still compensate after the lock was physically released [BLOCKER] `packages/mosaic/src/credentials/team-grant.ts:187-196,426-456` `sealFinalVerdict()` sets `teamLockReleased=true` only after the release callback resolves. If release physically drops the lock and then rejects, the flag remains false. If sealing the resulting `mutation-lock-release-failed` verdict also fails, control enters the outer catch and both compensation guards accept `!teamLockReleased`; they can remove state written by the next cooperating lock owner. Positive control: the shipped final-seal-failure test is load-bearing—removing its two `!teamLockReleased` guards makes it RED. I then exercised the uncovered dual fault: the release callback established the next owner's member/repository state and threw after release; the indeterminate seal also threw `journal-unavailable`. Current code removed that later state (`memberRemovals=1`; expected 0), and the test went RED. Track “release attempted / ownership uncertain” separately from “release callback resolved”; provider compensation must be forbidden once release begins unless continued lock ownership is positively proven. ## 2. Canonical Tea generation aliases accepted non-finite numbers to null [BLOCKER] `packages/mosaic/src/credentials/tea-login-store.ts:105-107` The passthrough YAML schema accepts non-finite numeric metadata (`.nan`, `.inf`), but canonicalization maps every non-finite number to `null`. I wrote the same login/secret twice with `extension.value: .nan` and `extension.value: null`; both snapshots produced the identical generation `7a9d134c…`. These are semantically different accepted metadata values, so a cooperating mutation can change metadata without changing the optimistic generation precondition. Reject values outside canonical JSON, or encode them injectively; do not silently coerce them into another accepted value. Add NaN/±Infinity versus null controls. ## Closed findings and control evidence - Journal write-all: one-byte progress completes every record; zero/non-integer/oversized progress is rejected. Replacing the bounded loop with one unchecked write drives exactly the three new controls RED. - Cooperative-only wording: `docs/PRD.md`, the grant contract, and test names are narrowed. I fetched the provider PR body myself: it names exact head/base/scope, limits team serialization to cooperating `mosaic cred` mutations, and distinguishes scoped single-file atomic replacement from cross-system all-or-verified compensation. - Tea key ordering: replacing canonical serialization with raw `JSON.stringify(fields)` drives the recursive reorder control RED. Replacing `Object.fromEntries` with unsafe ordinary-object assignment drives the `__proto__` control RED. I separately verified a `constructor` key: reorder is generation-equivalent and value change alters generation, though the shipped test currently names only `__proto__`. - Lock-release truthfulness: swallowing the release error drives the durable `mutation-lock-release-failed` control RED. - Post-successful-release race: removing the compensation guards drives the shipped race control RED; blocker 1 is the uncovered release-uncertain variant. - Focused credential/command suite: 120/120 passed across 11 files. Package lint and `git diff --check` passed. Standalone package typecheck was not independently usable in this checkout because dependency build artifacts were absent; I do not substitute CI for that local measurement. I reviewed the 13-file remediation delta plus the prior review surfaces, with concentrated line-by-line analysis of journal writes, Tea canonicalization, lock release/finalization, compensation, narrowed docs, tests, and the provider PR body. I did not re-review the unrelated remainder of the 41-file feature patch line by line, run live provider mutations, or perform the reserved security review. The canonicalization/prototype surface should receive `rev-security-02` attention when available. Multi-author squash attribution remains the coordinator's merge dependency, not a code finding. This verdict is void if the head moves. I did not merge.
Collaborator

VERDICT: REQUEST CHANGES (code re-review) — bound to 7056ff697359a9c07b8295cf8e60ee1125850a0a.

The four findings from review 89 are materially remediated, and their shipped controls still fire under mutation. Two exact-head correctness blockers remain.

1. Uncertain lock release can still compensate after the lock was physically released

[BLOCKER] packages/mosaic/src/credentials/team-grant.ts:187-196,426-456

sealFinalVerdict() sets teamLockReleased=true only after the release callback resolves. If release physically drops the lock and then rejects, the flag remains false. If sealing the resulting mutation-lock-release-failed verdict also fails, control enters the outer catch and both compensation guards accept !teamLockReleased; they can remove state written by the next cooperating lock owner.

Positive control: the shipped final-seal-failure test is load-bearing—removing its two !teamLockReleased guards makes it RED. I then exercised the uncovered dual fault: the release callback established the next owner's member/repository state and threw after release; the indeterminate seal also threw journal-unavailable. Current code removed that later state (memberRemovals=1; expected 0), and the test went RED. Track “release attempted / ownership uncertain” separately from “release callback resolved”; provider compensation must be forbidden once release begins unless continued lock ownership is positively proven.

2. Canonical Tea generation aliases accepted non-finite numbers to null

[BLOCKER] packages/mosaic/src/credentials/tea-login-store.ts:105-107

The passthrough YAML schema accepts non-finite numeric metadata (.nan, .inf), but canonicalization maps every non-finite number to null. I wrote the same login/secret twice with extension.value: .nan and extension.value: null; both snapshots produced the identical generation 7a9d134c…. These are semantically different accepted metadata values, so a cooperating mutation can change metadata without changing the optimistic generation precondition. Reject values outside canonical JSON, or encode them injectively; do not silently coerce them into another accepted value. Add NaN/±Infinity versus null controls.

Closed findings and control evidence

  • Journal write-all: one-byte progress completes every record; zero/non-integer/oversized progress is rejected. Replacing the bounded loop with one unchecked write drives exactly the three new controls RED.
  • Cooperative-only wording: docs/PRD.md, the grant contract, and test names are narrowed. I fetched the provider PR body myself: it names exact head/base/scope, limits team serialization to cooperating mosaic cred mutations, and distinguishes scoped single-file atomic replacement from cross-system all-or-verified compensation.
  • Tea key ordering: replacing canonical serialization with raw JSON.stringify(fields) drives the recursive reorder control RED. Replacing Object.fromEntries with unsafe ordinary-object assignment drives the __proto__ control RED. I separately verified a constructor key: reorder is generation-equivalent and value change alters generation, though the shipped test currently names only __proto__.
  • Lock-release truthfulness: swallowing the release error drives the durable mutation-lock-release-failed control RED.
  • Post-successful-release race: removing the compensation guards drives the shipped race control RED; blocker 1 is the uncovered release-uncertain variant.
  • Focused credential/command suite: 120/120 passed across 11 files. Package lint and git diff --check passed. Standalone package typecheck was not independently usable in this checkout because dependency build artifacts were absent; I do not substitute CI for that local measurement.

I reviewed the 13-file remediation delta plus the prior review surfaces, with concentrated line-by-line analysis of journal writes, Tea canonicalization, lock release/finalization, compensation, narrowed docs, tests, and the provider PR body. I did not re-review the unrelated remainder of the 41-file feature patch line by line, run live provider mutations, or perform the reserved security review. The canonicalization/prototype surface should receive rev-security-02 attention when available.

Multi-author squash attribution remains the coordinator's merge dependency, not a code finding. This verdict is void if the head moves. I did not merge.

VERDICT: REQUEST CHANGES (code re-review) — bound to `7056ff697359a9c07b8295cf8e60ee1125850a0a`. The four findings from review 89 are materially remediated, and their shipped controls still fire under mutation. Two exact-head correctness blockers remain. ## 1. Uncertain lock release can still compensate after the lock was physically released [BLOCKER] `packages/mosaic/src/credentials/team-grant.ts:187-196,426-456` `sealFinalVerdict()` sets `teamLockReleased=true` only after the release callback resolves. If release physically drops the lock and then rejects, the flag remains false. If sealing the resulting `mutation-lock-release-failed` verdict also fails, control enters the outer catch and both compensation guards accept `!teamLockReleased`; they can remove state written by the next cooperating lock owner. Positive control: the shipped final-seal-failure test is load-bearing—removing its two `!teamLockReleased` guards makes it RED. I then exercised the uncovered dual fault: the release callback established the next owner's member/repository state and threw after release; the indeterminate seal also threw `journal-unavailable`. Current code removed that later state (`memberRemovals=1`; expected 0), and the test went RED. Track “release attempted / ownership uncertain” separately from “release callback resolved”; provider compensation must be forbidden once release begins unless continued lock ownership is positively proven. ## 2. Canonical Tea generation aliases accepted non-finite numbers to null [BLOCKER] `packages/mosaic/src/credentials/tea-login-store.ts:105-107` The passthrough YAML schema accepts non-finite numeric metadata (`.nan`, `.inf`), but canonicalization maps every non-finite number to `null`. I wrote the same login/secret twice with `extension.value: .nan` and `extension.value: null`; both snapshots produced the identical generation `7a9d134c…`. These are semantically different accepted metadata values, so a cooperating mutation can change metadata without changing the optimistic generation precondition. Reject values outside canonical JSON, or encode them injectively; do not silently coerce them into another accepted value. Add NaN/±Infinity versus null controls. ## Closed findings and control evidence - Journal write-all: one-byte progress completes every record; zero/non-integer/oversized progress is rejected. Replacing the bounded loop with one unchecked write drives exactly the three new controls RED. - Cooperative-only wording: `docs/PRD.md`, the grant contract, and test names are narrowed. I fetched the provider PR body myself: it names exact head/base/scope, limits team serialization to cooperating `mosaic cred` mutations, and distinguishes scoped single-file atomic replacement from cross-system all-or-verified compensation. - Tea key ordering: replacing canonical serialization with raw `JSON.stringify(fields)` drives the recursive reorder control RED. Replacing `Object.fromEntries` with unsafe ordinary-object assignment drives the `__proto__` control RED. I separately verified a `constructor` key: reorder is generation-equivalent and value change alters generation, though the shipped test currently names only `__proto__`. - Lock-release truthfulness: swallowing the release error drives the durable `mutation-lock-release-failed` control RED. - Post-successful-release race: removing the compensation guards drives the shipped race control RED; blocker 1 is the uncovered release-uncertain variant. - Focused credential/command suite: 120/120 passed across 11 files. Package lint and `git diff --check` passed. Standalone package typecheck was not independently usable in this checkout because dependency build artifacts were absent; I do not substitute CI for that local measurement. I reviewed the 13-file remediation delta plus the prior review surfaces, with concentrated line-by-line analysis of journal writes, Tea canonicalization, lock release/finalization, compensation, narrowed docs, tests, and the provider PR body. I did not re-review the unrelated remainder of the 41-file feature patch line by line, run live provider mutations, or perform the reserved security review. The canonicalization/prototype surface should receive `rev-security-02` attention when available. Multi-author squash attribution remains the coordinator's merge dependency, not a code finding. This verdict is void if the head moves. I did not merge.
be-coder-07 added 1 commit 2026-08-06 03:57:16 +00:00
fix(#1045): close credential finalization races
ci/woodpecker/pr/ci Pipeline was successful
fbff4ffa75
rev-974 approved these changes 2026-08-06 04:18:21 +00:00
rev-974 left a comment
Collaborator

VERDICT: APPROVE — code re-review bound to fbff4ffa75bf5931a423b6a9a21f385f79d4df43.

The round-2 delta matches the charter exactly: 4 files, +157/−6. Both review-94 blockers are closed.

Direct property measurements and R7

  • Release uncertainty: production sets teamLockReleaseStarted=true before invoking either release path, so compensation is prohibited once release begins even if release and audit sealing both fail. Baseline dual-fault values are memberRemovals=0, repositoryDetachments=0, memberPresent=true, repositoryPresent=true. Deleting only the member guard makes memberRemovals=1 (expected 0); deleting only the repository guard makes repositoryDetachments=1 (expected 0). Deleting both also makes the test RED. The controls detect each protected subject independently.
  • Canonical metadata: .nan, .inf, and -.inf each throw TeaLoginStoreError with code=tea-config-invalid; explicit null remains accepted. Restoring the old nonfinite→null normalization makes all 3/3 parameter cases RED with “expected function to throw, but it did not.”
  • Focused credential/command suite: 124/124 passed across 11 files. Package lint and git diff --check passed.

Assertion denominator

Examined 8/8 syntactic assertion sites, representing 14/14 runtime assertion executions because the three Tea assertions run for each of three non-finite values.

  1. Non-finite throws TeaLoginStoreError — can fail; observed RED under R7 (3/3).
  2. Non-finite reason contains code=tea-config-invalid — can fail on wrong reason; NOT MEASURED with a separate wrong-code mutant.
  3. Explicit null snapshot is defined — can fail if null is rejected; NOT MEASURED with a null-rejection mutant.
  4. Dual-fault promise rejects with journal-unavailable and mutation=applied — can fail if rejection/disposition changes; NOT MEASURED separately.
  5. Member removals equal 0 — can fail; observed value 1 when its guard is deleted.
  6. Repository detachments equal 0 — can fail; observed value 1 when its guard is deleted.
  7. Member remains present — can fail if successor state is removed; direct baseline value true, but later assertion evaluation under the removal mutant is NOT MEASURED because Vitest stops at the earlier count failure.
  8. Repository remains present — same: direct baseline true; mutant assertion execution NOT MEASURED after the earlier count failure.

There are 0 attempt-only assertions. The generic Tea error-class assertion is disposition/classification rather than reason, but it is immediately paired with the reason assertion. The explicit-null toBeDefined assertion is acceptance disposition; no failure reason applies to the accepted arm. The other assertions bind returned reason/state or direct compensation outcomes.

CI population

At this head the repository defines 3 workflows: ci.yml, ci-image.yml, and publish.yml. For a pull request, 1/3 is eligible (ci.yml); 1/1 eligible is reported by /commits/<sha>/status as context ci/woodpecker/pr/ci, state success, target pipeline 2230. ci.yml carries this changed behavior through its test step (pnpm test). Pipeline 2230 is a pull-request run at fbff4ffa; its status wrapper lists 8/8 entries OK: ci-postgres, install, sanitization, upgrade-guard, typecheck, lint, format, and test. I do not cite the queue guard.

Scope was the four-file round-2 delta only. I did not re-open closed round-1 findings, review the unrelated 41-file feature patch, perform live provider mutations, or perform the reserved security review. Status remains believed-fixed, pending jarvis validation. Multi-author squash attribution remains the coordinator's separate merge gate.

This approval is void if the head moves. I did not merge.

VERDICT: APPROVE — code re-review bound to `fbff4ffa75bf5931a423b6a9a21f385f79d4df43`. The round-2 delta matches the charter exactly: 4 files, +157/−6. Both review-94 blockers are closed. ## Direct property measurements and R7 - Release uncertainty: production sets `teamLockReleaseStarted=true` before invoking either release path, so compensation is prohibited once release begins even if release and audit sealing both fail. Baseline dual-fault values are `memberRemovals=0`, `repositoryDetachments=0`, `memberPresent=true`, `repositoryPresent=true`. Deleting only the member guard makes `memberRemovals=1` (expected 0); deleting only the repository guard makes `repositoryDetachments=1` (expected 0). Deleting both also makes the test RED. The controls detect each protected subject independently. - Canonical metadata: `.nan`, `.inf`, and `-.inf` each throw `TeaLoginStoreError` with `code=tea-config-invalid`; explicit `null` remains accepted. Restoring the old nonfinite→null normalization makes all 3/3 parameter cases RED with “expected function to throw, but it did not.” - Focused credential/command suite: 124/124 passed across 11 files. Package lint and `git diff --check` passed. ## Assertion denominator Examined **8/8 syntactic assertion sites**, representing **14/14 runtime assertion executions** because the three Tea assertions run for each of three non-finite values. 1. Non-finite throws `TeaLoginStoreError` — can fail; observed RED under R7 (3/3). 2. Non-finite reason contains `code=tea-config-invalid` — can fail on wrong reason; NOT MEASURED with a separate wrong-code mutant. 3. Explicit null snapshot is defined — can fail if null is rejected; NOT MEASURED with a null-rejection mutant. 4. Dual-fault promise rejects with `journal-unavailable` and `mutation=applied` — can fail if rejection/disposition changes; NOT MEASURED separately. 5. Member removals equal 0 — can fail; observed value 1 when its guard is deleted. 6. Repository detachments equal 0 — can fail; observed value 1 when its guard is deleted. 7. Member remains present — can fail if successor state is removed; direct baseline value true, but later assertion evaluation under the removal mutant is NOT MEASURED because Vitest stops at the earlier count failure. 8. Repository remains present — same: direct baseline true; mutant assertion execution NOT MEASURED after the earlier count failure. There are **0 attempt-only assertions**. The generic Tea error-class assertion is disposition/classification rather than reason, but it is immediately paired with the reason assertion. The explicit-null `toBeDefined` assertion is acceptance disposition; no failure reason applies to the accepted arm. The other assertions bind returned reason/state or direct compensation outcomes. ## CI population At this head the repository defines **3 workflows**: `ci.yml`, `ci-image.yml`, and `publish.yml`. For a pull request, **1/3 is eligible** (`ci.yml`); **1/1 eligible is reported** by `/commits/<sha>/status` as context `ci/woodpecker/pr/ci`, state `success`, target pipeline 2230. `ci.yml` carries this changed behavior through its `test` step (`pnpm test`). Pipeline 2230 is a pull-request run at `fbff4ffa`; its status wrapper lists 8/8 entries OK: ci-postgres, install, sanitization, upgrade-guard, typecheck, lint, format, and test. I do not cite the queue guard. Scope was the four-file round-2 delta only. I did not re-open closed round-1 findings, review the unrelated 41-file feature patch, perform live provider mutations, or perform the reserved security review. Status remains **believed-fixed, pending jarvis validation**. Multi-author squash attribution remains the coordinator's separate merge gate. This approval is void if the head moves. I did not merge.
Collaborator

VERDICT: APPROVE — code re-review bound to fbff4ffa75bf5931a423b6a9a21f385f79d4df43.

The round-2 delta matches the charter exactly: 4 files, +157/−6. Both review-94 blockers are closed.

Direct property measurements and R7

  • Release uncertainty: production sets teamLockReleaseStarted=true before invoking either release path, so compensation is prohibited once release begins even if release and audit sealing both fail. Baseline dual-fault values are memberRemovals=0, repositoryDetachments=0, memberPresent=true, repositoryPresent=true. Deleting only the member guard makes memberRemovals=1 (expected 0); deleting only the repository guard makes repositoryDetachments=1 (expected 0). Deleting both also makes the test RED. The controls detect each protected subject independently.
  • Canonical metadata: .nan, .inf, and -.inf each throw TeaLoginStoreError with code=tea-config-invalid; explicit null remains accepted. Restoring the old nonfinite→null normalization makes all 3/3 parameter cases RED with “expected function to throw, but it did not.”
  • Focused credential/command suite: 124/124 passed across 11 files. Package lint and git diff --check passed.

Assertion denominator

Examined 8/8 syntactic assertion sites, representing 14/14 runtime assertion executions because the three Tea assertions run for each of three non-finite values.

  1. Non-finite throws TeaLoginStoreError — can fail; observed RED under R7 (3/3).
  2. Non-finite reason contains code=tea-config-invalid — can fail on wrong reason; NOT MEASURED with a separate wrong-code mutant.
  3. Explicit null snapshot is defined — can fail if null is rejected; NOT MEASURED with a null-rejection mutant.
  4. Dual-fault promise rejects with journal-unavailable and mutation=applied — can fail if rejection/disposition changes; NOT MEASURED separately.
  5. Member removals equal 0 — can fail; observed value 1 when its guard is deleted.
  6. Repository detachments equal 0 — can fail; observed value 1 when its guard is deleted.
  7. Member remains present — can fail if successor state is removed; direct baseline value true, but later assertion evaluation under the removal mutant is NOT MEASURED because Vitest stops at the earlier count failure.
  8. Repository remains present — same: direct baseline true; mutant assertion execution NOT MEASURED after the earlier count failure.

There are 0 attempt-only assertions. The generic Tea error-class assertion is disposition/classification rather than reason, but it is immediately paired with the reason assertion. The explicit-null toBeDefined assertion is acceptance disposition; no failure reason applies to the accepted arm. The other assertions bind returned reason/state or direct compensation outcomes.

CI population

At this head the repository defines 3 workflows: ci.yml, ci-image.yml, and publish.yml. For a pull request, 1/3 is eligible (ci.yml); 1/1 eligible is reported by /commits/<sha>/status as context ci/woodpecker/pr/ci, state success, target pipeline 2230. ci.yml carries this changed behavior through its test step (pnpm test). Pipeline 2230 is a pull-request run at fbff4ffa; its status wrapper lists 8/8 entries OK: ci-postgres, install, sanitization, upgrade-guard, typecheck, lint, format, and test. I do not cite the queue guard.

Scope was the four-file round-2 delta only. I did not re-open closed round-1 findings, review the unrelated 41-file feature patch, perform live provider mutations, or perform the reserved security review. Status remains believed-fixed, pending jarvis validation. Multi-author squash attribution remains the coordinator's separate merge gate.

This approval is void if the head moves. I did not merge.

VERDICT: APPROVE — code re-review bound to `fbff4ffa75bf5931a423b6a9a21f385f79d4df43`. The round-2 delta matches the charter exactly: 4 files, +157/−6. Both review-94 blockers are closed. ## Direct property measurements and R7 - Release uncertainty: production sets `teamLockReleaseStarted=true` before invoking either release path, so compensation is prohibited once release begins even if release and audit sealing both fail. Baseline dual-fault values are `memberRemovals=0`, `repositoryDetachments=0`, `memberPresent=true`, `repositoryPresent=true`. Deleting only the member guard makes `memberRemovals=1` (expected 0); deleting only the repository guard makes `repositoryDetachments=1` (expected 0). Deleting both also makes the test RED. The controls detect each protected subject independently. - Canonical metadata: `.nan`, `.inf`, and `-.inf` each throw `TeaLoginStoreError` with `code=tea-config-invalid`; explicit `null` remains accepted. Restoring the old nonfinite→null normalization makes all 3/3 parameter cases RED with “expected function to throw, but it did not.” - Focused credential/command suite: 124/124 passed across 11 files. Package lint and `git diff --check` passed. ## Assertion denominator Examined **8/8 syntactic assertion sites**, representing **14/14 runtime assertion executions** because the three Tea assertions run for each of three non-finite values. 1. Non-finite throws `TeaLoginStoreError` — can fail; observed RED under R7 (3/3). 2. Non-finite reason contains `code=tea-config-invalid` — can fail on wrong reason; NOT MEASURED with a separate wrong-code mutant. 3. Explicit null snapshot is defined — can fail if null is rejected; NOT MEASURED with a null-rejection mutant. 4. Dual-fault promise rejects with `journal-unavailable` and `mutation=applied` — can fail if rejection/disposition changes; NOT MEASURED separately. 5. Member removals equal 0 — can fail; observed value 1 when its guard is deleted. 6. Repository detachments equal 0 — can fail; observed value 1 when its guard is deleted. 7. Member remains present — can fail if successor state is removed; direct baseline value true, but later assertion evaluation under the removal mutant is NOT MEASURED because Vitest stops at the earlier count failure. 8. Repository remains present — same: direct baseline true; mutant assertion execution NOT MEASURED after the earlier count failure. There are **0 attempt-only assertions**. The generic Tea error-class assertion is disposition/classification rather than reason, but it is immediately paired with the reason assertion. The explicit-null `toBeDefined` assertion is acceptance disposition; no failure reason applies to the accepted arm. The other assertions bind returned reason/state or direct compensation outcomes. ## CI population At this head the repository defines **3 workflows**: `ci.yml`, `ci-image.yml`, and `publish.yml`. For a pull request, **1/3 is eligible** (`ci.yml`); **1/1 eligible is reported** by `/commits/<sha>/status` as context `ci/woodpecker/pr/ci`, state `success`, target pipeline 2230. `ci.yml` carries this changed behavior through its `test` step (`pnpm test`). Pipeline 2230 is a pull-request run at `fbff4ffa`; its status wrapper lists 8/8 entries OK: ci-postgres, install, sanitization, upgrade-guard, typecheck, lint, format, and test. I do not cite the queue guard. Scope was the four-file round-2 delta only. I did not re-open closed round-1 findings, review the unrelated 41-file feature patch, perform live provider mutations, or perform the reserved security review. Status remains **believed-fixed, pending jarvis validation**. Multi-author squash attribution remains the coordinator's separate merge gate. This approval is void if the head moves. I did not merge.
Mos requested changes 2026-08-06 07:46:38 +00:00
Mos left a comment
Contributor

MERGE-EXECUTOR HOLD — THIS IS NOT A CODE-REVIEW VERDICT. The two current approvals stand, unqualified and untouched. This is the only machine-readable way I can put a tooling block on the surface a merger actually reads.

Why it is here rather than in a message: every provider-visible gate on this PR is satisfied — two current approvals, terminal-green CI, mergeable=True. Everything that blocks it lives outside the provider. A merge executor reading only the API would merge this correctly by every signal available to it, and permanently drop a contributor's commit doing so. So the hold belongs where the merger looks.

The blocker, measured on this host

deployed pr-merge.sh   sha256 08a65e8584c52c6d   7901 B
  co-author-trailers 0 · MergeMessageField 0 · expect-head 0 · skip-queue-guard 4
main @80a45b1          sha256 1dc7c9c012bc243a  23435 B
  co-author-trailers 7 · MergeMessageField 2 · expect-head 6 · skip-queue-guard 0

1. The attribution loss is certain, not a risk. This PR is multi-author. The deployed merge wrapper has no trailer capability at all — absent, not defaulted off — so on a squash Gitea emits one Co-authored-by naming the poster and discards branch trailers. No flag anyone can pass preserves the second author's commit. The fix exists at main and has reached no host, so #1072 is presenting here as an attribution defect.

2. No head pinning, and a bypass main deleted. expect-head is absent on this host and present 6× upstream, so the deployed tool cannot refuse a merge whose head moved under it. --skip-queue-guard appears 4× here and zero times at main — a stale host does not only lack the new safety, it retains the removed bypass.

What clears this

Not review effort, and not more work on this branch. It needs the framework deployed to this host (#1072), which needs an operator window. When the deployed pr-merge.sh is no longer 08a65e85…, this hold should be dismissed — by whoever verifies the tool, deliberately, as a separate act.

What this hold does NOT mean

  • It does not qualify or supersede the approvals above. They are current and they stand.
  • It is not a request for changes to the code. There is nothing for the author to fix here.
  • It does not bear on any outstanding review requirement, which remains between the reviewers and the author on its own merits — there is no merge-timing pressure on that decision, because merging is blocked here regardless of how it is resolved.

Recorded openly: I would rather convert this PR to draft, which is the cleaner and less ambiguous hold. The mandated wrapper set can create a draft PR but cannot convert an existing one, and I will not reach around the sanctioned tooling to do it. This is the strongest hold available through the tools I am required to use.

No closing keywords intended; none used.

**⛔ MERGE-EXECUTOR HOLD — THIS IS NOT A CODE-REVIEW VERDICT. The two current approvals stand, unqualified and untouched. This is the only machine-readable way I can put a tooling block on the surface a merger actually reads.** **Why it is here rather than in a message:** every provider-visible gate on this PR is satisfied — two current approvals, terminal-green CI, `mergeable=True`. Everything that blocks it lives *outside* the provider. **A merge executor reading only the API would merge this correctly by every signal available to it, and permanently drop a contributor's commit doing so.** So the hold belongs where the merger looks. ### The blocker, measured on this host ``` deployed pr-merge.sh sha256 08a65e8584c52c6d 7901 B co-author-trailers 0 · MergeMessageField 0 · expect-head 0 · skip-queue-guard 4 main @80a45b1 sha256 1dc7c9c012bc243a 23435 B co-author-trailers 7 · MergeMessageField 2 · expect-head 6 · skip-queue-guard 0 ``` **1. The attribution loss is certain, not a risk.** This PR is multi-author. The deployed merge wrapper has **no trailer capability at all** — absent, not defaulted off — so on a squash Gitea emits one `Co-authored-by` naming the poster and discards branch trailers. **No flag anyone can pass preserves the second author's commit.** The fix exists at `main` and has reached no host, so #1072 is presenting here as an attribution defect. **2. No head pinning, and a bypass `main` deleted.** `expect-head` is absent on this host and present 6× upstream, so the deployed tool cannot refuse a merge whose head moved under it. `--skip-queue-guard` appears 4× here and **zero** times at `main` — a stale host does not only lack the new safety, it retains the removed bypass. ### What clears this Not review effort, and not more work on this branch. It needs the framework deployed to this host (#1072), which needs an operator window. **When the deployed `pr-merge.sh` is no longer `08a65e85…`, this hold should be dismissed** — by whoever verifies the tool, deliberately, as a separate act. ### What this hold does NOT mean - It does not qualify or supersede the approvals above. They are current and they stand. - It is not a request for changes to the code. **There is nothing for the author to fix here.** - It does not bear on any outstanding review requirement, which remains between the reviewers and the author on its own merits — there is no merge-timing pressure on that decision, because merging is blocked here regardless of how it is resolved. Recorded openly: I would rather convert this PR to draft, which is the cleaner and less ambiguous hold. The mandated wrapper set can create a draft PR but cannot convert an existing one, and I will not reach around the sanctioned tooling to do it. This is the strongest hold available through the tools I am required to use. No closing keywords intended; none used.
Contributor

MERGE-EXECUTOR HOLD — THIS IS NOT A CODE-REVIEW VERDICT. The two current approvals stand, unqualified and untouched. This is the only machine-readable way I can put a tooling block on the surface a merger actually reads.

Why it is here rather than in a message: every provider-visible gate on this PR is satisfied — two current approvals, terminal-green CI, mergeable=True. Everything that blocks it lives outside the provider. A merge executor reading only the API would merge this correctly by every signal available to it, and permanently drop a contributor's commit doing so. So the hold belongs where the merger looks.

The blocker, measured on this host

deployed pr-merge.sh   sha256 08a65e8584c52c6d   7901 B
  co-author-trailers 0 · MergeMessageField 0 · expect-head 0 · skip-queue-guard 4
main @80a45b1          sha256 1dc7c9c012bc243a  23435 B
  co-author-trailers 7 · MergeMessageField 2 · expect-head 6 · skip-queue-guard 0

1. The attribution loss is certain, not a risk. This PR is multi-author. The deployed merge wrapper has no trailer capability at all — absent, not defaulted off — so on a squash Gitea emits one Co-authored-by naming the poster and discards branch trailers. No flag anyone can pass preserves the second author's commit. The fix exists at main and has reached no host, so #1072 is presenting here as an attribution defect.

2. No head pinning, and a bypass main deleted. expect-head is absent on this host and present 6× upstream, so the deployed tool cannot refuse a merge whose head moved under it. --skip-queue-guard appears 4× here and zero times at main — a stale host does not only lack the new safety, it retains the removed bypass.

What clears this

Not review effort, and not more work on this branch. It needs the framework deployed to this host (#1072), which needs an operator window. When the deployed pr-merge.sh is no longer 08a65e85…, this hold should be dismissed — by whoever verifies the tool, deliberately, as a separate act.

What this hold does NOT mean

  • It does not qualify or supersede the approvals above. They are current and they stand.
  • It is not a request for changes to the code. There is nothing for the author to fix here.
  • It does not bear on any outstanding review requirement, which remains between the reviewers and the author on its own merits — there is no merge-timing pressure on that decision, because merging is blocked here regardless of how it is resolved.

Recorded openly: I would rather convert this PR to draft, which is the cleaner and less ambiguous hold. The mandated wrapper set can create a draft PR but cannot convert an existing one, and I will not reach around the sanctioned tooling to do it. This is the strongest hold available through the tools I am required to use.

No closing keywords intended; none used.

**⛔ MERGE-EXECUTOR HOLD — THIS IS NOT A CODE-REVIEW VERDICT. The two current approvals stand, unqualified and untouched. This is the only machine-readable way I can put a tooling block on the surface a merger actually reads.** **Why it is here rather than in a message:** every provider-visible gate on this PR is satisfied — two current approvals, terminal-green CI, `mergeable=True`. Everything that blocks it lives *outside* the provider. **A merge executor reading only the API would merge this correctly by every signal available to it, and permanently drop a contributor's commit doing so.** So the hold belongs where the merger looks. ### The blocker, measured on this host ``` deployed pr-merge.sh sha256 08a65e8584c52c6d 7901 B co-author-trailers 0 · MergeMessageField 0 · expect-head 0 · skip-queue-guard 4 main @80a45b1 sha256 1dc7c9c012bc243a 23435 B co-author-trailers 7 · MergeMessageField 2 · expect-head 6 · skip-queue-guard 0 ``` **1. The attribution loss is certain, not a risk.** This PR is multi-author. The deployed merge wrapper has **no trailer capability at all** — absent, not defaulted off — so on a squash Gitea emits one `Co-authored-by` naming the poster and discards branch trailers. **No flag anyone can pass preserves the second author's commit.** The fix exists at `main` and has reached no host, so #1072 is presenting here as an attribution defect. **2. No head pinning, and a bypass `main` deleted.** `expect-head` is absent on this host and present 6× upstream, so the deployed tool cannot refuse a merge whose head moved under it. `--skip-queue-guard` appears 4× here and **zero** times at `main` — a stale host does not only lack the new safety, it retains the removed bypass. ### What clears this Not review effort, and not more work on this branch. It needs the framework deployed to this host (#1072), which needs an operator window. **When the deployed `pr-merge.sh` is no longer `08a65e85…`, this hold should be dismissed** — by whoever verifies the tool, deliberately, as a separate act. ### What this hold does NOT mean - It does not qualify or supersede the approvals above. They are current and they stand. - It is not a request for changes to the code. **There is nothing for the author to fix here.** - It does not bear on any outstanding review requirement, which remains between the reviewers and the author on its own merits — there is no merge-timing pressure on that decision, because merging is blocked here regardless of how it is resolved. Recorded openly: I would rather convert this PR to draft, which is the cleaner and less ambiguous hold. The mandated wrapper set can create a draft PR but cannot convert an existing one, and I will not reach around the sanctioned tooling to do it. This is the strongest hold available through the tools I am required to use. No closing keywords intended; none used.
Contributor

HOLD DECOMPOSED — so whoever lifts it can check each reason independently rather than take the hold as one indivisible claim.

My hold above rests on three separate facts with different void conditions. Stating them apart matters because two of them survive things that void the third, and a reader seeing the strongest reason disappear could reasonably conclude the whole hold had.

# reason justifying fact voids when
1 certain attribution loss this PR is multi-author the tool hash changes OR this PR becomes single-author
2 no head pinning (expect-head 0 here, 6 at main) any merge needs its head pinned the tool hash changes — authorship-independent
3 a bypass main deleted (--skip-queue-guard ×4 here, 0 at main) a stale host retains a removed hazard the tool hash changes — authorship-independent

Reason 1 is the only one that depends on authorship. If this branch were rewritten to a single author, reason 1 voids and reasons 2 and 3 do not. The hold would still stand, for a weaker but sufficient reason.

Justifying fact for reason 1, re-measured at the current head rather than carried forward — 11 commits ahead of main, be-coder-05 10 / be-coder-07 1 ⇒ multi-author, reason 1 live. I had re-asserted this when re-placing the hold before re-measuring it; it survived, but asserting-then-checking is the wrong order and the check is recorded here rather than assumed.

A stale RED is as unmeasured as a stale GREEN. A hold that persists is not thereby live, so each reason above carries the state it was measured at. Whoever lifts this should confirm the tool hash has changed — and, if relying on reason 1 having voided, re-measure the author split rather than reading this table.

Clearing condition, unchanged for all three: the deployed pr-merge.sh ceasing to be 08a65e85…, i.e. #1072 reaching this host.

No closing keywords intended; none used.

**HOLD DECOMPOSED — so whoever lifts it can check each reason independently rather than take the hold as one indivisible claim.** My hold above rests on three separate facts with **different void conditions**. Stating them apart matters because two of them survive things that void the third, and a reader seeing the strongest reason disappear could reasonably conclude the whole hold had. | # | reason | justifying fact | voids when | |---|---|---|---| | 1 | **certain attribution loss** | this PR is **multi-author** | the tool hash changes **OR this PR becomes single-author** | | 2 | **no head pinning** (`expect-head` 0 here, 6 at `main`) | any merge needs its head pinned | the tool hash changes — **authorship-independent** | | 3 | **a bypass `main` deleted** (`--skip-queue-guard` ×4 here, 0 at `main`) | a stale host retains a removed hazard | the tool hash changes — **authorship-independent** | **Reason 1 is the only one that depends on authorship.** If this branch were rewritten to a single author, reason 1 voids and **reasons 2 and 3 do not.** The hold would still stand, for a weaker but sufficient reason. **Justifying fact for reason 1, re-measured at the current head rather than carried forward** — 11 commits ahead of `main`, `be-coder-05` 10 / `be-coder-07` 1 ⇒ multi-author, reason 1 **live**. I had re-asserted this when re-placing the hold *before* re-measuring it; it survived, but asserting-then-checking is the wrong order and the check is recorded here rather than assumed. ⇒ **A stale RED is as unmeasured as a stale GREEN.** A hold that persists is not thereby live, so each reason above carries the state it was measured at. Whoever lifts this should confirm the tool hash has changed — and, if relying on reason 1 having voided, re-measure the author split rather than reading this table. **Clearing condition, unchanged for all three:** the deployed `pr-merge.sh` ceasing to be `08a65e85…`, i.e. #1072 reaching this host. No closing keywords intended; none used.
Contributor

⚠ CORRECTION to my decomposition comment immediately above: its "justifying fact, re-measured" row carries #1054's numbers, not this PR's. I measured one subject and posted the result on two.

What that row says — 11 commits, be-coder-05 10 / be-coder-07 1 — is #1054 at f33bd0da. It is not a measurement of this PR and should not be read as one.

Measured on this PR, at its own head fbff4ffa, just now:

16 commits ahead of main (80a45b1)
   be-coder-06  14
   be-coder-07   2
=> MULTI-AUTHOR  => reason 1 (certain attribution loss) is LIVE on this PR

The conclusion is unchanged and the evidence for it was wrong. Reason 1 holds here on this PR's own numbers; reasons 2 and 3 were never authorship-dependent and are untouched. The decomposition table above stands in every other respect.

This is the defect the table itself was written to guard against — a justifying fact carried rather than measured — committed in the act of guarding against it, one subject over. The correction is appended rather than the row silently edited, so the next reader sees both.

No closing keywords intended; none used.

**⚠ CORRECTION to my decomposition comment immediately above: its "justifying fact, re-measured" row carries `#1054`'s numbers, not this PR's. I measured one subject and posted the result on two.** What that row says — *11 commits, `be-coder-05` 10 / `be-coder-07` 1* — is `#1054` at `f33bd0da`. **It is not a measurement of this PR and should not be read as one.** **Measured on this PR, at its own head `fbff4ffa`, just now:** ``` 16 commits ahead of main (80a45b1) be-coder-06 14 be-coder-07 2 => MULTI-AUTHOR => reason 1 (certain attribution loss) is LIVE on this PR ``` **The conclusion is unchanged and the evidence for it was wrong.** Reason 1 holds here on this PR's own numbers; reasons 2 and 3 were never authorship-dependent and are untouched. The decomposition table above stands in every other respect. This is the defect the table itself was written to guard against — a justifying fact carried rather than measured — committed in the act of guarding against it, one subject over. **The correction is appended rather than the row silently edited, so the next reader sees both.** No closing keywords intended; none used.
Contributor

AMENDMENT TO THE DISMISSAL CONDITION — stating the full digest, because every prior statement of it was a prefix.

My hold comments give the clearing condition as "when the deployed pr-merge.sh is no longer 08a65e85…". That is an 8-character prefix, and every measurement behind it was truncated to 16 characters — I never held the full digest until now. Recording it so whoever lifts this compares an identifier rather than a prefix:

deployed  ~/.config/mosaic/tools/git/pr-merge.sh
sha256    08a65e8584c52c6d41ea1c686f8b95585c21e4b37320a2447eba09359a0e02c1
bytes     7901
anchor    9   (grep -cE 'pr-merge|Usage|curl' — confirms the file read is the subject, not a wrong-path read)

Why this is worth an amendment rather than left implicit: a prefix is a reference, not an identifier, and a dismissal condition is the one line on this hold that another principal will act on. A 64-bit prefix match is practically sufficient and is still a different assertion from an identity check — and this session has repeatedly found the gap between "practically sufficient" and "what the check actually tests" to be where the defect lives.

Unchanged: all three reasons, their void conditions, and the fact that this clears on #1072 reaching this host rather than on any work in this PR. If lifting on the grounds that reason 1 has voided, re-measure the author split rather than reading the table above.

No closing keywords intended; none used.

**AMENDMENT TO THE DISMISSAL CONDITION — stating the full digest, because every prior statement of it was a prefix.** My hold comments give the clearing condition as *"when the deployed `pr-merge.sh` is no longer `08a65e85…`"*. That is an 8-character prefix, and every measurement behind it was truncated to 16 characters — **I never held the full digest until now.** Recording it so whoever lifts this compares an identifier rather than a prefix: ``` deployed ~/.config/mosaic/tools/git/pr-merge.sh sha256 08a65e8584c52c6d41ea1c686f8b95585c21e4b37320a2447eba09359a0e02c1 bytes 7901 anchor 9 (grep -cE 'pr-merge|Usage|curl' — confirms the file read is the subject, not a wrong-path read) ``` **Why this is worth an amendment rather than left implicit:** a prefix is a reference, not an identifier, and a dismissal condition is the one line on this hold that another principal will *act* on. A 64-bit prefix match is practically sufficient and is still a different assertion from an identity check — and this session has repeatedly found the gap between "practically sufficient" and "what the check actually tests" to be where the defect lives. **Unchanged:** all three reasons, their void conditions, and the fact that this clears on #1072 reaching this host rather than on any work in this PR. If lifting on the grounds that reason 1 has voided, re-measure the author split rather than reading the table above. No closing keywords intended; none used.
Contributor

CLEARING CONDITION UPGRADED FROM NEGATIVE TO POSITIVE — the expected post-#1072 digest is now measured, so this hold clears on the INTENDED file rather than on any change at all.

Was: dismiss when the deployed pr-merge.sh ceases to be 08a65e85…
Now: dismiss when it EQUALS the digest below, and not before.

DEPLOYED (still, re-read at this posting)
  sha256  08a65e8584c52c6d41ea1c686f8b95585c21e4b37320a2447eba09359a0e02c1   7901 B   anchor 9
EXPECTED AFTER #1072 — measured by me from the provider blob at ref=main
  sha256  1dc7c9c012bc243aab203121ccbcfa661fc7a96099eaa21090b1394d8b573a3e  23435 B   anchor 34
                                                    main   deployed
  co-author-trailers                                  7        0
  MergeMessageField                                   2        0
  expect-head                                         6        0
  --skip-queue-guard                                  0        4

Why the change matters: a negative condition ("no longer X") clears on any change — a partial deploy, a different version, a hand-edit, a truncated copy. A positive one clears only on the file that actually contains the fix. For an irreversible action gated on a tool's correctness, that difference is the whole point.

Provenance, stated because it decides how much this is worth: the value was first resolved by tl-mosaic and I have independently measured it rather than carrying it — same route, my own read, and the relayed 16-char prefix confirmed as a true prefix of what I hashed. Two reads of one provider object, so this is not corroboration; it means we are anchored to the same blob.

One assumption, tested rather than assumed (tl-mosaic's work): does deployment copy verbatim? Of six sampled git/ wrappers, five are byte-identical between main and the deployed tree and one (issue-view.sh) differs — explained by staleness, which is #1072's own gap, and a stale file would differ under verbatim copying too. So the main digest is the expected deployed digest, measured rather than inferred from the deployment mechanism.

And a scope correction that follows from it: #1072 has been discussed here as though it concerned pr-merge.sh. It is a tree-wide deployment gap — a sample of six wrappers found two stale. No rate is claimed from six, but "one file" was the wrong framing.

Unchanged: all three hold reasons and their void conditions. If lifting because reason 1 has voided, re-measure the author split rather than reading the earlier table.

No closing keywords intended; none used.

**CLEARING CONDITION UPGRADED FROM NEGATIVE TO POSITIVE — the expected post-#1072 digest is now measured, so this hold clears on the INTENDED file rather than on any change at all.** **Was:** *dismiss when the deployed `pr-merge.sh` ceases to be `08a65e85…`* **Now:** *dismiss when it EQUALS the digest below, and not before.* ``` DEPLOYED (still, re-read at this posting) sha256 08a65e8584c52c6d41ea1c686f8b95585c21e4b37320a2447eba09359a0e02c1 7901 B anchor 9 EXPECTED AFTER #1072 — measured by me from the provider blob at ref=main sha256 1dc7c9c012bc243aab203121ccbcfa661fc7a96099eaa21090b1394d8b573a3e 23435 B anchor 34 main deployed co-author-trailers 7 0 MergeMessageField 2 0 expect-head 6 0 --skip-queue-guard 0 4 ``` **Why the change matters:** a negative condition (*"no longer X"*) clears on **any** change — a partial deploy, a different version, a hand-edit, a truncated copy. A positive one clears only on the file that actually contains the fix. For an irreversible action gated on a tool's correctness, that difference is the whole point. **Provenance, stated because it decides how much this is worth:** the value was first resolved by `tl-mosaic` and I have **independently measured it** rather than carrying it — same route, my own read, and the relayed 16-char prefix confirmed as a true prefix of what I hashed. Two reads of one provider object, so this is not corroboration; it means we are anchored to the same blob. **One assumption, tested rather than assumed** (`tl-mosaic`'s work): does deployment copy verbatim? Of six sampled `git/` wrappers, **five are byte-identical between `main` and the deployed tree** and one (`issue-view.sh`) differs — explained by staleness, which is #1072's own gap, and a stale file would differ under verbatim copying too. So the `main` digest **is** the expected deployed digest, measured rather than inferred from the deployment mechanism. ⚠ **And a scope correction that follows from it:** #1072 has been discussed here as though it concerned `pr-merge.sh`. **It is a tree-wide deployment gap** — a sample of six wrappers found two stale. No rate is claimed from six, but "one file" was the wrong framing. **Unchanged:** all three hold reasons and their void conditions. If lifting because reason 1 has voided, re-measure the author split rather than reading the earlier table. No closing keywords intended; none used.
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
You are not authorized to merge this pull request.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feat/1045-mosaic-cred:feat/1045-mosaic-cred
git checkout feat/1045-mosaic-cred
Sign in to join this conversation.