feat(org): detect cross-role assignment conflicts - #266
Conversation
|
@claude please review this PR and publish your findings as a FORMAL GitHub review pinned to head 1b16150 — not only an issue comment. Run exactly: gh pr review 266 --repo itsHabib/workbench --comment --body "<your review, stating head 1b16150>" This is stacked on #265. Review only the child diff. Focus on:
Try to break the positive/mutant controls rather than blessing the shape. |
|
@codex review |
|
Claude finished @itsHabib's task in 7m 13s —— View job PR #266 Review —
|
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
🟢 Approval recommended
The tenant-scoped conflict projection and malformed-tail handling are consistent across CLI/JSON/text surfaces and are backed by targeted unit and end-to-end tests.
Pull request overview
Adds tenant-scoped detection of cross-role assignment conflicts to org sweep, surfacing “detected, not prevented” ownership contention when the same opaque work URI is held by multiple role chains in the configured tenant.
Changes:
- Scope
org sweeprows/totals to the configured tenant and emit deterministicassign_conflictsin both JSON (always[], nevernull) and text output. - Preserve valid-prefix sweep findings when a chain has a malformed (non-JSON) tail, while keeping
Load/append paths fail-closed. - Add CLI-level and survey-level tests for conflict detection, tenant isolation, deterministic ordering, and malformed-tail behavior.
File summaries
| File | Description |
|---|---|
| cmd/org/main.go | Scopes sweep to -tenant, adds assign_conflicts to JSON output, and passes conflicts into the renderer. |
| cmd/org/main_test.go | Adds end-to-end CLI tests covering conflict detection, tenant isolation, unassign reconciliation, and malformed-tail behavior. |
| cmd/org/internal/survey/survey.go | Carries folded “held work” through role rows and adds deterministic tenant-scoped AssignConflicts projection. |
| cmd/org/internal/survey/survey_test.go | Adds unit tests for conflict detection determinism, tenant scoping, and held-vs-dangling semantics. |
| cmd/org/internal/render/sweep.go | Extends sweep text rendering to optionally show conflict details and include conflicts in the attention summary. |
| cmd/org/internal/home/home.go | Returns decoded-prefix records on JSON decode failure so sweep can still report valid-prefix findings. |
| cmd/org/CLAUDE.md | Updates sweep documentation to reflect tenant scoping and conflict detection behavior. |
| cmd/org/AGENTS.md | Mirrors the sweep documentation updates (kept in sync with CLAUDE.md). |
| cmd/org-mcp/internal/server/verbs.go | Updates MCP verb description to include tenant-scoped ownership conflict reporting. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed head 1b16150 (child diff only, base origin/feat/org-sweep).
Findings
P2 — render/sweep.go:41-44: attention line emits zero-counts when only conflicts exist
When len(conflicts) > 0 but t.Dangling + t.Late + t.Broken == 0, the current code unconditionally emits:
attention: 0 dangling · 0 late · 0 chain(s) that do not fold · 2 assign_conflict(s)
The no-conflict branch below it only prints the attention line when t.Dangling+t.Late+t.Broken > 0:
if t.Dangling+t.Late+t.Broken > 0 {
fmt.Fprintf(&sb, "attention: %d dangling · %d late · %d chain(s) that do not fold\n", ...)
}The conflict branch doesn't guard the same way, producing noisy 0 dangling · 0 late · 0 chain(s) prefix whenever conflicts are the only issue. No test in TestSweepDetectsCrossRoleAssignmentConflicts pins whether this is intentional — it only uses strings.Contains for positive matches, so the zeros slip through unobserved.
Fix (or document the intent explicitly if showing full context alongside conflicts is desired):
// merge both into one branch that always shows all counts when anything needs attention
if len(conflicts) > 0 || t.Dangling+t.Late+t.Broken > 0 {
if len(conflicts) > 0 {
fmt.Fprintf(&sb, "attention: %d dangling · %d late · %d chain(s) that do not fold · %d assign_conflict(s)\n",
t.Dangling, t.Late, t.Broken, len(conflicts))
} else {
fmt.Fprintf(&sb, "attention: %d dangling · %d late · %d chain(s) that do not fold\n",
t.Dangling, t.Late, t.Broken)
}
}P3 — survey.go:35: Records docstring slightly imprecise after the malformed-tail fix
The updated comment says Records "includes a record that stopped the fold; when a line itself is not JSON, Err names that line and Records stops at the last decodable record." After the fix in readChain, a non-JSON line is NOT included in the returned records slice (the append happens after the failed unmarshal). So Records = len(records) counts only successfully decoded lines; the bad line is never in records at all. "stops at the last decodable record" is correct, but "It includes a record that stopped the fold" is misleading — the stopping non-JSON line is excluded, not included. Minor.
What holds
Conflict identity (tenant + exact work URI, sorted owners): Correct. heldWorks extracts only assignment.Work from state.Held — Digest, Party, Phase, Dangling, and OpenIntents cannot alter the finding. The set-valued owners[work][role] map de-duplicates the same role appearing twice for the same URI (tested by the duplicate-row mutant). sortedOwners gives deterministic output.
Tenant scope — no mixed-scope false-clean: cmdSweep now pre-filters h.Roles() pairs with if p[0] != s.tenant { continue } before building the roles slice. AssignConflicts re-filters by tenant inside the function (defense-in-depth). The beta sweep test verifies len(beta.Roles) == 0 and beta.AssignConflicts == [] while acme roles hold a conflict — confirming no cross-tenant bleed.
Malformed JSON tail — valid prefix for sweep, fail-closed for Load/Append:
readChain changed from return nil, err to return records, err at the parse-failure point. The returned records is the valid prefix (the non-JSON line is not appended before the early return). cmdSweep calls h.Records() which propagates this partial result; the fold over the valid prefix succeeds; withState stamps Held from the last valid folded state; and if err != nil && row.Err == "" marks the row BROKEN. Load() calls readChain and returns nil, org.RoleState{}, err on any parse error, so Append() (which calls Load under lock) still fails closed. TestSweepMalformedTailKeepsAssignmentConflict verifies all three behaviors: conflict preserved, row BROKEN, note write refused.
Deterministic JSON — assign_conflicts: [] not null: conflicts := make([]AssignConflict, 0) in AssignConflicts is always non-nil; the only append is inside the len(roles) >= 2 guard. Go marshals a non-nil empty slice as []. The unit test explicitly checks got == nil and the e2e JSON test unmarshals and checks beta.AssignConflicts == nil || len(...) != 0.
No-conflict text byte-compatibility: Both the conflict section and the augmented attention line are guarded by if len(conflicts) > 0, so the text output for a clean sweep (zero conflicts) is byte-identical to pre-PR. TestSweepReportsBrokenChainThroughTheCLI already asserts !strings.Contains(out, "assign_conflict").
Sequential detected-not-prevented: The render explicitly annotates the section "assign_conflicts (detected, not prevented):". The AssignConflict type comment states it is "deliberately a finding, not an admission result." CLAUDE.md describes it as "an honest detected-not-prevented finding, not a global lock or admission claim. The file-home scan is sequential, not an atomic snapshot across chains." No uniqueness is claimed in the fold or write path.
Mutant controls — attempts to break:
- Duplicate role row:
addOwnerusesmap[string]struct{}so the same(work, role)pair is idempotent. The unit test fixture includes an explicit duplicate row and verifies only two distinct owners are reported. - Cross-tenant: beta role holding the same URI is filtered before it can join acme's owner map. The CLI test verifies beta sees
[] rolesand[] conflicts. - Dangling-not-held:
Danglingis a separate field;AssignConflictsonly readsrole.Held. The unit test verifies a dangling claim against a live owner produces zero conflicts. - Released phase: A PhaseChartered (released) role still appears in
Heldif the unassign was never written. The unit test verifies the conflict is detected regardless of phase. - Malformed tail erasing ownership: Verified above — the valid prefix preserves
Held; the test proves the conflict survives a corrupt tail. - After unassign: The e2e test writes
unassignfor lead:alpha and re-sweeps;assign_conflictsbecomes[]and the text output drops the conflict section.
Two findings (one P2, one P3). The logic is sound.
|
@claude please re-review this PR after review-fix round 1 and publish your findings as a FORMAL GitHub review pinned to head 6587d43 — not only an issue comment. Run exactly: gh pr review 266 --repo itsHabib/workbench --comment --body "<your review, stating head 6587d43>" This remains stacked on #265. Review only the child diff from origin/feat/org-sweep through 6587d43. Confirm the two prior findings are resolved:
Then re-test the original safety properties: tenant + exact opaque work URI identity, distinct deterministic role owners, single-tenant sweep scope, valid-prefix malformed-tail diagnostics with fail-closed writes, [] not null JSON, byte-compatible no-conflict text, and honest sequential detected-not-prevented semantics. Try to break the tests and report any remaining actionable finding. |
|
@codex review |
|
Claude finished @itsHabib's task in 6m 2s —— View job PR #266 Re-review — Round 2 (head
|
There was a problem hiding this comment.
🟢 Approval recommended
The implementation is tenant-scoped, deterministic, preserves fail-closed semantics for load/write paths, and is backed by focused unit + end-to-end CLI tests for the new behavior.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6587d435f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Reviewed head 6587d43 (child diff only, base origin/feat/org-sweep). Round 2 re-review after fix round 1.
Prior findings — both resolved
P2 resolved — render/sweep.go:41-49: conflict-only attention now emits only the conflict count
The fix adds an explicit guard: when len(conflicts) > 0 && t.Dangling+t.Late+t.Broken == 0, the attention line is:
attention: N assign_conflict(s)
When len(conflicts) > 0 and at least one of dangling/late/broken is nonzero (mixed case), the full context is preserved:
attention: %d dangling · %d late · %d chain(s) that do not fold · %d assign_conflict(s)
The test TestSweepDetectsCrossRoleAssignmentConflicts now explicitly pins both sides (main_test.go:276):
if !strings.Contains(out, "attention: 1 assign_conflict(s)") || strings.Contains(out, "attention: 0 dangling") {Positive match required, zero-noise string rejected. The mutant (zero-valued prefix) is now machine-caught.
P3 resolved — survey.go:33-35: Records docstring correctly distinguishes the two broken-chain cases
New text:
Records is the number of successfully decoded records supplied to replay.
A decoded record that stops the fold is included; an undecodable JSON line
is excluded, Err names it, and Records ends at the last decodable prefix.
This correctly covers both cases:
- org.Advance failure (fold-stopping decoded record): the record was parsed by readChain and is in the slice; Records = len(records) includes it; Of exits on Advance error with the last-valid state.
- JSON parse failure (undecodable tail): readChain returns before appending the bad line; the line is never in records; Records = len(records) excludes it; cmdSweep sets row.Err from the readChain error (main.go:556-557).
The test at survey_test.go:261-264 also pins the Held field for the broken-chain case:
if !reflect.DeepEqual(r.Held, []string{work}) {
t.Fatalf("held = %v, want [%s] — conflicts use the last state that folded", r.Held, work)
}All five safety properties re-verified
Conflict identity (tenant + exact opaque work URI, distinct deterministic role owners):
heldWorks extracts only assignment.Work from state.Held (survey.go:122-128) — digest, party, phase, dangling, and OpenIntents are not consulted. addOwner uses an inner map[string]struct{} so the same (work, role) pair is idempotent. sortedOwners produces a deterministic list. AssignConflicts sorts the final slice by Work for stable output order. The unit test fixture includes an explicit duplicate role row and verifies only two distinct owners appear. ✓
Single-tenant sweep scope, no mixed-scope false-clean:
cmdSweep pre-filters pairs with if p[0] != s.tenant { continue } before building the roles slice (main.go:547-548). AssignConflicts re-filters by tenant inside the function (defense-in-depth). The beta sweep test checks beta.AssignConflicts == nil || len(beta.AssignConflicts) != 0 || len(beta.Roles) != 0 — any bleed fails it. ✓
Valid-prefix malformed-tail diagnostics, fail-closed writes:
readChain returns records, err (not nil, err) on JSON parse failure (home.go:306). The valid prefix is passed to Of, which folds it successfully. cmdSweep then sets row.Err = err.Error() because row.Err == "" after the successful fold (main.go:556-557). Load discards the partial result and returns the error, so Append still fails closed. All three behaviors are machine-checked by TestSweepMalformedTailKeepsAssignmentConflict. ✓
[] not null JSON:
conflicts := make([]AssignConflict, 0) (survey.go:155) is always non-nil. Appends happen only inside the len(roles) >= 2 guard. Go marshals a non-nil empty slice as []. The unit test explicitly checks if got == nil || len(got) != 0. The e2e beta sweep test unmarshals JSON and checks the same condition. ✓
Byte-compatible no-conflict text:
Both the conflict detail section (render/sweep.go:28-34) and the augmented attention line (lines 41-49) are guarded by if len(conflicts) > 0. When conflicts is empty the code falls through to the original format unchanged. TestSweepReportsBrokenChainThroughTheCLI asserts !strings.Contains(out, "assign_conflict") on a chain with only a BROKEN row — pinning that unrelated BROKEN attention does not gain a zero-conflict suffix. ✓
Sequential detected-not-prevented semantics:
The render section is labeled "assign_conflicts (detected, not prevented):". The AssignConflict type comment says "deliberately a finding, not an admission result." CLAUDE.md describes it as "an honest detected-not-prevented finding, not a global lock or admission claim. The file-home scan is sequential, not an atomic snapshot across chains." No lock, no uniqueness claim, no admission gate in the write path. ✓
Mutant controls re-tested
- Duplicate role row: idempotent via inner
map[string]struct{}; pinned by unit test fixture. - Cross-tenant: filtered at both cmdSweep (pre-filter) and AssignConflicts (re-filter); beta e2e test covers.
- Dangling-not-held: AssignConflicts reads
role.Held, notrole.Dangling;TestAssignConflictUsesHeldNotPhaseOrDanglingexplicitly covers a dangling claim against a live owner producing zero conflicts. - Released phase (PhaseChartered): a chartered role still appears in Held if unassign was never written; unit test covers.
- Malformed tail erasing ownership: valid prefix from readChain preserves Held; e2e test covers.
- After unassign: e2e test writes unassign for lead:alpha and re-sweeps; assign_conflicts becomes [] and text drops the conflict section.
- Deterministic ordering: conflicts sorted by Work, owners sorted by role name; both pinned by reflect.DeepEqual.
One P3 observation (not a defect)
render/sweep.go:46 — mixed-attention line emits zero-valued individual fields
When len(conflicts) > 0 and t.Dangling+t.Late+t.Broken > 0 (sum nonzero, but some terms zero), output such as:
attention: 0 dangling · 0 late · 1 chain(s) that do not fold · 1 assign_conflict(s)
is possible. This is consistent with the pre-PR no-conflict attention line (which also shows all three fields unconditionally). The P2 fix correctly cleans up the conflict-only case; the mixed case deliberately mirrors the original format for completeness. No test pins the mixed-format attention line — TestSweepMalformedTailKeepsAssignmentConflict (BROKEN + conflict) does not check the specific attention string. Cosmetic and consistent with existing behavior; flagging for completeness.
Both prior findings resolved. No new P1 or P2. One P3 cosmetic observation. The logic is sound.
|
@claude please perform the final exact-head review after fix round 2 and publish a FORMAL GitHub review pinned to cfe4d0d, not only an issue comment. Run exactly: gh pr review 266 --repo itsHabib/workbench --comment --body "<your review, stating head cfe4d0d>" This remains stacked on #265. Review only origin/feat/org-sweep..cfe4d0d. Re-test, do not merely acknowledge:
This is the repository's second and final fix round. Publish any remaining actionable finding with severity and exact line; otherwise explicitly state no remaining actionable findings at this head. |
|
@codex review |
|
Claude finished @itsHabib's task in 6m 26s —— View job PR #266 Final Review — Round 2 (head
|
There was a problem hiding this comment.
🟢 Approval recommended
The changes are tenant-scoped, deterministic, and well-covered by both unit and CLI tests without introducing a write-time global-lock or cross-tool coupling.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Reviewed head cfe4d0d (child diff only, base origin/feat/org-sweep). Final review after fix round 2.
Round-2 tenant boundary — re-tested, not acknowledged
Exact immediate child match (home.go:85-97)
RolesForTenant calls os.ReadDir(h.root) and then matches each entry by t.Name() == tenant. os.DirEntry.Name() is always a simple basename — it cannot contain / on any supported filesystem, so the comparison is structurally bounded to immediate children of the state root.
No unreadable-sibling traversal
For every entry that does not match the tenant name (or is not a directory, or is the blobs entry), the loop continues. rolesInTenant is called for exactly one entry — the matching tenant — and is never called on unrelated sibling directories. An unreadable beta directory cannot fail an acme sweep because beta's directory is never opened.
Path escape impossible
The actual call is h.rolesInTenant(t.Name()) (home.go:94), not h.rolesInTenant(tenant). Even if the caller supplied ../etc as a tenant, the code would only proceed if some directory entry had Name() == "../etc" — structurally impossible on Unix because directory names cannot contain /, so filepath.Join(h.root, t.Name()) is always exactly one level below the state root.
-tenant blobs is a safe no-op
The condition t.Name() == "blobs" in the skip clause prevents the blob store from ever being enumerated as a tenant, even if explicitly requested.
Test coverage
TestSweepDoesNotTraverseOtherTenants (main_test.go:293-323): creates a readable acme tenant and an unreadable beta directory (chmod 0), sweeps for acme, and asserts exit 0 with only one acme role. The skip guard (os.ReadDir(unrelated) == nil -> t.Skip) handles environments where the test process is root.
Prior findings — both confirmed resolved
P2 resolved — render/sweep.go:41-49: conflict-only attention has no zero-count noise
When len(conflicts) > 0 && t.Dangling+t.Late+t.Broken == 0, the attention line is attention: N assign_conflict(s) with no zero-valued dangling/late/broken prefix. The machine-caught mutant at main_test.go:276 requires the positive match and rejects the zero-noise string.
P3 resolved — survey.go:33-35: Records docstring distinguishes the two broken-chain cases
"A decoded record that stops the fold is included; an undecodable JSON line is excluded, Err names it, and Records ends at the last decodable prefix." This correctly covers: (a) org.Advance failure — record appended by readChain, in len(records), counted; (b) JSON parse failure — readChain returns before append (home.go:329-333), never in records, cmdSweep sets row.Err from returned error (main.go:553-555).
All original invariants re-verified
Conflict identity — tenant + exact opaque work URI, distinct deterministic role owners
heldWorks reads only assignment.Work from state.Held (survey.go:122-128). Digest, party, phase, dangling, and open intents cannot alter the finding. addOwner uses an inner map[string]struct{} so the same (work, role) pair is idempotent. sortedOwners produces a deterministic list; the final sort.Slice by Work gives stable output order. TestAssignConflictsAreTenantScopedAndDeterministic uses reflect.DeepEqual against an exact fixture.
Single-tenant rows/totals/conflicts — no mixed-scope false-clean
cmdSweep now calls h.RolesForTenant(s.tenant) (main.go:540) — all returned pairs are already tenant-scoped; the old pre-filter if p[0] != s.tenant { continue } was correctly removed. AssignConflicts retains its if role.Tenant != tenant { continue } guard (survey.go:147-149) as defense-in-depth. TestSweepDetectsCrossRoleAssignmentConflicts verifies beta sees zero roles, zero totals, and [] conflicts while acme's roles conflict.
Valid-prefix malformed-tail diagnostics + fail-closed writes
readChain returns records, err on JSON parse failure (home.go:333). The valid prefix reaches survey.Of, which folds it and sets row.Held from the last valid state. cmdSweep sets row.Err = err.Error() when row.Err == "" (main.go:553-555). Load discards the partial result and propagates the error (home.go:128-136), so Append still fails closed. All three behaviors are pinned by TestSweepMalformedTailKeepsAssignmentConflict.
Deterministic [] JSON
make([]AssignConflict, 0) at survey.go:155 is always non-nil; the only append is inside the len(roles) >= 2 guard. Go marshals a non-nil empty slice as []. Unit test and e2e beta sweep test (main_test.go:267) both assert the non-nil empty condition.
Unchanged no-conflict text
Both the conflict detail section (render/sweep.go:28-34) and the attention line (lines 41-49) are guarded by if len(conflicts) > 0. TestSweepReportsBrokenChainThroughTheCLI asserts !strings.Contains(out, "assign_conflict") on a BROKEN-only chain.
Sequential detected-not-prevented semantics
render/sweep.go:29 labels the section "assign_conflicts (detected, not prevented):". The AssignConflict type comment (survey.go:131-133) says "deliberately a finding, not an admission result." CLAUDE.md states "an honest detected-not-prevented finding, not a global lock or admission claim. The file-home scan is sequential, not an atomic snapshot across chains." No lock, no uniqueness claim, no admission gate in the write path.
Mutant controls re-tested
- Duplicate role row:
addOwner's innermap[string]struct{}is idempotent; unit test fixture includes an explicit duplicate and verifies only two distinct owners. - Cross-tenant: Beta entries filtered before
rolesInTenantis called;AssignConflictsre-filters. Beta e2e sweep verifies zero roles and[]conflicts. - Dangling-not-held:
AssignConflictsreadsrole.Held, notrole.Dangling;TestAssignConflictUsesHeldNotPhaseOrDanglingverifies a dangling claim against a live owner produces zero conflicts. - Released phase (PhaseChartered): A chartered role still appears in
Heldif unassign was never written; unit test covers. - Malformed tail erasing ownership: Valid prefix from
readChainpreservesHeld; e2e test verifies conflict survives a corrupt tail. - After unassign: e2e test writes unassign for lead:alpha and re-sweeps;
assign_conflictsbecomes[]and text drops the conflict section. -tenant blobs:t.Name() == "blobs"guard inRolesForTenantensures the blob store is never treated as a tenant.- Path-like tenant: Structurally impossible to match any
os.DirEntry.Name()on Unix; even if it matched,rolesInTenant(t.Name())uses the OS-verified name.
No remaining actionable findings
Both prior findings are resolved. The round-2 tenant boundary is correctly implemented, tested, and safe against unreadable sibling traversal and path escape. No remaining P1 or P2. The pre-existing P3 cosmetic (mixed-attention line showing zero-valued fields when e.g. only Broken is nonzero alongside conflicts) is consistent with the pre-PR format and is not a new defect. The logic is sound.
…cisions to fold Independent reconciliation of the whole ownership-continuity lineage at 2026-08-26 heads: what #245 designed, what #246/#248/#262/#263 shipped through the governed path, what #265/#266 measure, what hooks#42/#43 and drive#47 add, what cc-skills#29 proves in fixture, and what remains hypothesis. Every major claim classified on the honest rung ladder; duplications and missing joins named; D1-D10 written to be accepted, amended, or struck in place. Indexed from vision.md's document table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…265) * feat(org): sweep — the instrument that says whether continuity works The substrate is a bet: that a session leaves something the next one can inherit. Nothing measured it. `org sweep` does, from the chains alone, with no cooperation from any agent. It REPLAYS each chain through the kernel rather than scanning kinds, because the number that matters is invisible to a scan: a takeover's record does not name the claim it stranded, so an orphaned obligation is only visible as a fold transition. The sweep counts orphans, and how many a successor then discharged. The headline pair: distilled session ends (a checkpoint — a real conclusion) against ones only observed (a mark), and inherited obligations discharged against orphaned. A rate with no data renders as an em dash, never 0% — "no session has ended yet" and "every session ended undistilled" are the two findings that must never share a value. A chain that stops folding is a BROKEN row carrying the counts from before the break, not a failed sweep. Exposed over MCP as org_sweep (read-only, so it joins the allowlist). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(org/sweep): a broken chain keeps its obligation, and the CLI can see it Two related findings from review round 1, both about the same gap: the sweep claimed to preserve what a chain recorded before a break, and neither path actually delivered it. survey.Of returned early on a refused record without carrying the state that DID fold, so a BROKEN row reported no dangling work and Sum's counter undercounted — a chain could strand an obligation and the aggregate would say zero. Both exits now go through one withState, so the broken and healthy paths cannot report different fields. cmdSweep read through Load, which folds internally and is all-or-nothing: a chain that stopped folding arrived empty, so the CLI reported zero records for exactly the chain worth looking at. It now reads records via the new home.Records and lets the replay decide admissibility. Tests pin both, plus the revoke-while-active orphan path the review flagged as untested — revoke reaches the same kernel orphan() as takeover through a different transition, and "high confidence by inspection" is how the untested arm of a pair diverges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(org/sweep): a broken row says when the chain last held valid state Round-2 finding: LastAt was stamped from the tip after the loop, so the early return on a refused record left it empty — a BROKEN row exists to be diagnosed from, and a reader cannot diagnose a chain that will not say when it was last valid. It is now stamped per folded record, which is the only place it can come from: state carries no timestamp, so withState cannot recover it afterwards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(org): detect cross-role assignment conflicts (#266) * feat(org): detect cross-role assignment conflicts * fix(org): tighten assignment conflict reporting * fix(org): isolate tenant sweep enumeration --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
Adds the honest cross-chain half of org ownership:
org sweepnow reports when the same opaque work URI is still held by multiple role chains in the configured tenant. This is deliberately detected, not prevented—no provider adapter, global write lock, or new authority claim. The PR is stacked on #265.What this adds
Tenant-scoped assignment conflict projection
assign_conflictsentries with the tenant, exact work URI, and sorted owner roles.[], nevernull, in JSON.Honest broken-chain diagnostics
Executable positive and mutant controls
unassignremoves it; different tenants and dangling-but-unassigned work do not create one.Validation
gofmt -l .go vet ./...golangci-lint run ./...go test -race -count=1 ./...go build ./...go run ./cmd/tracelens eval ./cmd/tracelens/testdata/corpus(12/12, 100% macro precision)assign_conflicts: []