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
pull from: feat/1045-mosaic-cred
merge into: :main
:main
:greenfield/fomo-lin
:feat/lease-promotion-and-harness-isolation
:fix/1099-pipefail-wake
:fix/1099-pipefail-tests
:fix/1099-pipefail-sweep
:fix/framework-shell-portability
:fix/1043-pane-git-identity
:fix/1081-issue-close-silent-comment-failure
:fix/1090-enrollment-wallclock-tolerance
:feat/1082-tea-stale-token-diagnostic
:fix/detect-platform-silent-128-outside-repo
:feat/1050-install-state-machine-red-fixture
:fix/pr-merge-message-field
:feat/1051-mosaic-brain-installer
:feat/1045-mosaic-cred
:remediation/state
:fix/1056-upgrade-rollback-control-race
:fix/1019-ci-queue-timeout-harness
:next
:feat/rm-02-gate-registry
:fix/rm-01-reproducible-checkout
:remediation/mission-setup
:fix/hygiene-inert-format-gate
:fix/1019-queue-guard-stdin
:feat/mos-ste-writing-standard
:fix/1007-suite-hermeticity
:fix/991-comment-url-scheme-normalise
:feat/push-guard-null-case-verification
:mos-comms-live
:docs/heartbeat-framework-layering-ms-lead
:feat/869-c4-version-coupling
:feat/869-c2-install-ordering-guard
:feat/869-c5-doctor-activation-check
:feat/per-agent-gitea-identity
:fix/875-belongs-case-insensitive-slug
:fix/ci-queue-wait-404-branch-absent
:feat/869-c1-activation-probe
:feat/869-c3-broker-supervisor
:fix/865-tea-cli-comment-invocation
:feat/glpi-skills
:fix/860-deflake-mutator-lease-gate
:fix/850-detect-platform-port-normalization
:fix/856-worktree-deps-preflight
:fix/835-pr-review-approve-reject-comment-flag
:fix/848-truthful-evidence
:fix/812-pr-review-comment
:fix/849-recovery-runtime-fixture-race
:docs/758-ledger-m5-001-sync
:feat/834-tc-server-side-doc
:feat/833-constrained-recovery-command
:feat/827-gate0-probe
:governance/gate0-probe3-amendment
:fix/795-codex-pr-diff
:fix/795-ci-base-jq
:fix/795-ci-base-git
:feat/791-pr3-fleet-regen
:feat/791-pr2-snapshot-restore
:fix/807-glpi-206
:fix/808-agent-send-false-sender
:feat/791-upgrade-config-protection
:feat/790-mosaic-yolo-claudex-pr2
:feat/790-mosaic-yolo-claudex
:feat/758-v1-v2-migrator
:fix/766-exact-fleet-comms
:test/758-reconciler-lifecycle-gates
:docs/771-kbn101-db-role-split
:test/758-example-profile-dispositions
:feat/758-shared-role-resolution
:feat/mos-logical-identity-fencing
:feat/769-kbn100-unified-schema
:docs/753-kbn010-threat-gate
:feat/758-roster-v2-compiler
:feat/756-official-discord-plugin
:docs/758-fleet-config-management
:fix/mos-option2-qualification-format
:docs/issue-758-m0
:docs/mos-option2-qualification
:mos-comms
:feat/tess-interaction-agent
:fix/tess-docs-format
:draft/mosaic-platform-prd
:fix/installer-provider-gate-and-local-gateway-redis
:release/mosaic-cli-0.0.37
:feat/framework-constitution-alpha
:fix/git-wrapper-repo-detection
:fix/woodpecker-wrapper-legacy-mosaic
:fix/t-a292e96f-gitea-pr-metadata
:fix/gitea-pr-metadata-login-t-a292e96f
:fix/t_a292e96f-pr-metadata-gitea
:fix/t_3a368a52-gitea-usc-login
:fix/bootstrap-hotfix
:fix/populate-known-packages-list
:fix/idempotent-init
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
Replacement for closed PR #1058, whose provider PR object was misattributed to
@Mosby a wrapper identity-resolution defect.Implements the governed
mosaic credlifecycle for explicit identity, estate, and host boundaries:provision,wire,grant,get,validate,whoami,list,rotate,revoke, andauditresolveByHost(host)reverse lookup for MB-BRAIN-01finallymosaic credteam mutations only; provider authority remains the authorization boundary, generation checks are optimistic concurrency rather than atomic CAS, and every compensation requires provider read-backwirewith roster/path binding, target-inode revalidation, single-file atomic replacement, and parent-directory fsyncHostile 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:
7056ff697359a9c07b8295cf8e60ee1125850a0aBase / merge-base:
85d2108e4ed15c744ad3b87a5b629e7b2d39405aScope: 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 successorbe-coder-07.Review-id 89 remediation:
mutation-lock-release-failed; anokverdict cannot survive uncertain cleanup, and no provider compensation runs after a successful release./bin/truewhile preserving the second-processflockacquisition assertion.Author advisories also found and closed a post-lock-release compensation race. No assertion was weakened.
Author-run verification at exact head
git diff --check: passedcli-smoke.spec.ts. Pipeline #2223's canonical image ran all 22 CLI-smoke cases green; exact-head canonical CI is required below.89a3f2ee223ac24469cea8f9e0fd99f95f79b862/ identical; currentorigin/mainalready matched the merge-base, so rebase was a no-opRed-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
main; C1 remains sequencing-priorrev-974, mandatory security approval byrev-security-02, and terminal exact-head CI remain requiredSecurity 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:
mosaic credmutators;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-incompleterather than overstated rollback.f2666d7da9to12958610cbVERDICT: 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-229callsFileHandle.write(string)once and ignoresbytesWritten. 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.writeand made each string write persist/return one byte. Two discriminating controls went red:CredentialAuditJournal.open(...)resolved successfully with a one-byte, invalidopenedrecord instead of rejecting.seal('ok', 'get-verified')resolved with a.sealed.jsonlpath 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 andatomicoccurrences 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:472says both provider/token/Tea identity axes “register atomically,” despite the implementation being a multi-system sequence with compensating rollback and explicitrollback-incomplete/indeterminateoutcomes.docs/credentials/GRANT-VALIDATE-CONTRACT.md:204says a team grant “serializes governed mutations per provider team” without limiting this to cooperatingmosaic credmutators. The team lock is an advisory same-UID-replaceable/tmpflock and is not an authorization boundary.file-credential-store.spec.ts:105says 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-113hashesJSON.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...vsed8f...), 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-470doesawait releaseTeamLock?.().catch(() => undefined)after all outcomes, includingok, 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
(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.cred getrecords authorization/start before disclosure, loops on short output writes, and maps partial write toindeterminate/unknown; completed disclosure followed by audit failure isindeterminate/applied.git diff --checkpassed.executeCredentialProvision,executeCredentialRevoke,executeCredentialGrant, andexecuteCredentialGetretain their owned buffers until GC. Addfinallyzeroization for consistency with the package's secret-lifetime discipline.f2666d7d, 13 commits, and old base; current frozen head is12958610, 14 commits, base85d2108e. Correct the provider artifact without moving the code head.clone;testfailed. 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 arebe-coder-06, distinct from reviewerrev-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
12958610cb.Four blocking findings.
1. Journal short writes can be reported as sealed success
packages/mosaic/src/credentials/audit-journal.ts:216-229callsFileHandle.write(string)once and ignoresbytesWritten. 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.writeand made each string write persist/return one byte. Two discriminating controls went red:CredentialAuditJournal.open(...)resolved successfully with a one-byte, invalidopenedrecord instead of rejecting.seal('ok', 'get-verified')resolved with a.sealed.jsonlpath 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 andatomicoccurrences 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:472says both provider/token/Tea identity axes “register atomically,” despite the implementation being a multi-system sequence with compensating rollback and explicitrollback-incomplete/indeterminateoutcomes.docs/credentials/GRANT-VALIDATE-CONTRACT.md:204says a team grant “serializes governed mutations per provider team” without limiting this to cooperatingmosaic credmutators. The team lock is an advisory same-UID-replaceable/tmpflock and is not an authorization boundary.file-credential-store.spec.ts:105says 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-113hashesJSON.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...vsed8f...), 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-470doesawait releaseTeamLock?.().catch(() => undefined)after all outcomes, includingok, 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
(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.cred getrecords authorization/start before disclosure, loops on short output writes, and maps partial write toindeterminate/unknown; completed disclosure followed by audit failure isindeterminate/applied.git diff --checkpassed.executeCredentialProvision,executeCredentialRevoke,executeCredentialGrant, andexecuteCredentialGetretain their owned buffers until GC. Addfinallyzeroization for consistency with the package's secret-lifetime discipline.f2666d7d, 13 commits, and old base; current frozen head is12958610, 14 commits, base85d2108e. Correct the provider artifact without moving the code head.clone;testfailed. 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 arebe-coder-06, distinct from reviewerrev-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 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-456sealFinalVerdict()setsteamLockReleased=trueonly after the release callback resolves. If release physically drops the lock and then rejects, the flag remains false. If sealing the resultingmutation-lock-release-failedverdict 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
!teamLockReleasedguards 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 threwjournal-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-107The passthrough YAML schema accepts non-finite numeric metadata (
.nan,.inf), but canonicalization maps every non-finite number tonull. I wrote the same login/secret twice withextension.value: .nanandextension.value: null; both snapshots produced the identical generation7a9d134c…. 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
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 cooperatingmosaic credmutations, and distinguishes scoped single-file atomic replacement from cross-system all-or-verified compensation.JSON.stringify(fields)drives the recursive reorder control RED. ReplacingObject.fromEntrieswith unsafe ordinary-object assignment drives the__proto__control RED. I separately verified aconstructorkey: reorder is generation-equivalent and value change alters generation, though the shipped test currently names only__proto__.mutation-lock-release-failedcontrol RED.git diff --checkpassed. 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-02attention 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-456sealFinalVerdict()setsteamLockReleased=trueonly after the release callback resolves. If release physically drops the lock and then rejects, the flag remains false. If sealing the resultingmutation-lock-release-failedverdict 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
!teamLockReleasedguards 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 threwjournal-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-107The passthrough YAML schema accepts non-finite numeric metadata (
.nan,.inf), but canonicalization maps every non-finite number tonull. I wrote the same login/secret twice withextension.value: .nanandextension.value: null; both snapshots produced the identical generation7a9d134c…. 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
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 cooperatingmosaic credmutations, and distinguishes scoped single-file atomic replacement from cross-system all-or-verified compensation.JSON.stringify(fields)drives the recursive reorder control RED. ReplacingObject.fromEntrieswith unsafe ordinary-object assignment drives the__proto__control RED. I separately verified aconstructorkey: reorder is generation-equivalent and value change alters generation, though the shipped test currently names only__proto__.mutation-lock-release-failedcontrol RED.git diff --checkpassed. 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-02attention 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: 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
teamLockReleaseStarted=truebefore invoking either release path, so compensation is prohibited once release begins even if release and audit sealing both fail. Baseline dual-fault values arememberRemovals=0,repositoryDetachments=0,memberPresent=true,repositoryPresent=true. Deleting only the member guard makesmemberRemovals=1(expected 0); deleting only the repository guard makesrepositoryDetachments=1(expected 0). Deleting both also makes the test RED. The controls detect each protected subject independently..nan,.inf, and-.infeach throwTeaLoginStoreErrorwithcode=tea-config-invalid; explicitnullremains accepted. Restoring the old nonfinite→null normalization makes all 3/3 parameter cases RED with “expected function to throw, but it did not.”git diff --checkpassed.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.
TeaLoginStoreError— can fail; observed RED under R7 (3/3).code=tea-config-invalid— can fail on wrong reason; NOT MEASURED with a separate wrong-code mutant.journal-unavailableandmutation=applied— can fail if rejection/disposition changes; NOT MEASURED separately.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
toBeDefinedassertion 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, andpublish.yml. For a pull request, 1/3 is eligible (ci.yml); 1/1 eligible is reported by/commits/<sha>/statusas contextci/woodpecker/pr/ci, statesuccess, target pipeline 2230.ci.ymlcarries this changed behavior through itsteststep (pnpm test). Pipeline 2230 is a pull-request run atfbff4ffa; 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
teamLockReleaseStarted=truebefore invoking either release path, so compensation is prohibited once release begins even if release and audit sealing both fail. Baseline dual-fault values arememberRemovals=0,repositoryDetachments=0,memberPresent=true,repositoryPresent=true. Deleting only the member guard makesmemberRemovals=1(expected 0); deleting only the repository guard makesrepositoryDetachments=1(expected 0). Deleting both also makes the test RED. The controls detect each protected subject independently..nan,.inf, and-.infeach throwTeaLoginStoreErrorwithcode=tea-config-invalid; explicitnullremains accepted. Restoring the old nonfinite→null normalization makes all 3/3 parameter cases RED with “expected function to throw, but it did not.”git diff --checkpassed.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.
TeaLoginStoreError— can fail; observed RED under R7 (3/3).code=tea-config-invalid— can fail on wrong reason; NOT MEASURED with a separate wrong-code mutant.journal-unavailableandmutation=applied— can fail if rejection/disposition changes; NOT MEASURED separately.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
toBeDefinedassertion 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, andpublish.yml. For a pull request, 1/3 is eligible (ci.yml); 1/1 eligible is reported by/commits/<sha>/statusas contextci/woodpecker/pr/ci, statesuccess, target pipeline 2230.ci.ymlcarries this changed behavior through itsteststep (pnpm test). Pipeline 2230 is a pull-request run atfbff4ffa; 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.
⛔ 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
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-bynaming the poster and discards branch trailers. No flag anyone can pass preserves the second author's commit. The fix exists atmainand has reached no host, so #1072 is presenting here as an attribution defect.2. No head pinning, and a bypass
maindeleted.expect-headis absent on this host and present 6× upstream, so the deployed tool cannot refuse a merge whose head moved under it.--skip-queue-guardappears 4× here and zero times atmain— 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.shis no longer08a65e85…, this hold should be dismissed — by whoever verifies the tool, deliberately, as a separate act.What this hold does NOT mean
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
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-bynaming the poster and discards branch trailers. No flag anyone can pass preserves the second author's commit. The fix exists atmainand has reached no host, so #1072 is presenting here as an attribution defect.2. No head pinning, and a bypass
maindeleted.expect-headis absent on this host and present 6× upstream, so the deployed tool cannot refuse a merge whose head moved under it.--skip-queue-guardappears 4× here and zero times atmain— 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.shis no longer08a65e85…, this hold should be dismissed — by whoever verifies the tool, deliberately, as a separate act.What this hold does NOT mean
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.
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.
expect-head0 here, 6 atmain)maindeleted (--skip-queue-guard×4 here, 0 atmain)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-0510 /be-coder-071 ⇒ 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.shceasing to be08a65e85…, i.e. #1072 reaching this host.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-0510 /be-coder-071 — is#1054atf33bd0da. 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: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.
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.shis no longer08a65e85…". 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: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.
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.shceases to be08a65e85…Now: dismiss when it EQUALS the digest below, and not before.
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-mosaicand 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 sampledgit/wrappers, five are byte-identical betweenmainand 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 themaindigest 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.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.