feat(orchestration): lead↔worker delegation lineage ledger#631
Conversation
Record the otherwise-implicit lead→worker/validator spawn and result edges as first-class manifest state (manifest.lineage). The agent row carried no parent link; this captures who spawned whom, with what brief (sha256 digest) + resolved model, and what came back. - DelegationEdge/DelegationStatus types + optional manifest.lineage, normalized to [] (back-compat) and shape-validated. - recordDelegationSpawn (best-effort, idempotent per child) wired into both spawn sites (desktop tool + daemon domain). - Result recorded in releaseTask on a worker's terminal transition; releaseStaleClaims reconciles edges whose child went terminal without a release (e.g. a self-completed validator) — lead-triggered. - /lineage denied to every role in patchPolicy; edges written only via the service's directPatch, like the planning-gate state. Tests: +9 (spawn/result/reconcile/idempotency/legacy-default/policy + a characterization test pinning the v1 per-session-lifetime semantic). Docs: orchestration source map + tool-registration updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughAdds delegation lineage tracking to the orchestration manifest. New ChangesDelegation Lineage Tracking
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@copilot review but do not make fixes |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/desktop/src/main/services/orchestration/orchestrationService.test.ts (1)
1974-2025: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAnchor spawn tests to a registered child and assert the full fingerprint.
These first spawn tests call
recordDelegationSpawnwithout an/agentsrow, so they can pass for orphan lineage edges. They also only verifyspawnFingerprint.provider, leaving the rest of the persisted fingerprint unprotected.Proposed test tightening
it("records a running delegation edge at spawn with a brief digest + fingerprint", async () => { const svc = createOrchestrationService({ resolveLaneWorktree: () => lane }); const { manifest } = await makeRun(svc); + await seedChild(svc, manifest, { sessionId: "S-worker" }); + const spawnFingerprint = { + provider: "claude", + modelId: "claude-sonnet-4-6", + reasoningEffort: null, + resolvedAt: "now", + routingKey: "default", + } as const; const res = await svc.recordDelegationSpawn( { runId: manifest.runId, parentSessionId: "S-lead", childSessionId: "S-worker", childRole: "worker", childTag: "impl", stepId: "T-1", briefText: BRIEF, - spawnFingerprint: { - provider: "claude", - modelId: "claude-sonnet-4-6", - reasoningEffort: null, - resolvedAt: "now", - routingKey: "default", - }, + spawnFingerprint, }, manifest.bundlePath, ); @@ expect(edge.childRole).toBe("worker"); expect(edge.briefDigest).toBe(crypto.createHash("sha256").update(BRIEF).digest("hex")); - expect(edge.spawnFingerprint?.provider).toBe("claude"); + expect(edge.spawnFingerprint).toEqual(spawnFingerprint); expect(edge.resultSummary).toBeUndefined(); await svc.dispose(); }); it("recordDelegationSpawn is idempotent per child session", async () => { const svc = createOrchestrationService({ resolveLaneWorktree: () => lane }); const { manifest } = await makeRun(svc); + await seedChild(svc, manifest, { sessionId: "S-worker" }); const first = await svc.recordDelegationSpawn( { runId: manifest.runId, parentSessionId: "S-lead", childSessionId: "S-worker", childRole: "worker", briefText: BRIEF }, manifest.bundlePath,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/orchestration/orchestrationService.test.ts` around lines 1974 - 2025, The spawn tests in the first and second test cases need to be anchored to a registered child agent before calling recordDelegationSpawn, and the spawnFingerprint assertions need to be expanded. First, register a child agent or worker in the system before calling recordDelegationSpawn to ensure the tests validate against actual registered children rather than orphan lineage edges. Second, in the assertion block after recording the delegation spawn, replace the single check on edge.spawnFingerprint?.provider with comprehensive assertions that verify all fingerprint fields including modelId, reasoningEffort, resolvedAt, and routingKey match the provided spawnFingerprint object.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/main/services/orchestration/manifestNormalization.ts`:
- Around line 155-165: The filter callback in manifest.lineage assignment only
validates a subset of required DelegationEdge fields (id, parentSessionId,
childSessionId, status) but not others like childRole, spawnedAt, spawnEtag, and
briefDigest. Expand the validation logic within the filter callback to check all
required fields of the DelegationEdge shape before returning true. Additionally,
apply the same comprehensive validation to the validator function referenced
around line 435 to ensure consistency and prevent invalid edges from being
accepted.
In `@apps/desktop/src/main/services/orchestration/orchestrationService.ts`:
- Around line 1802-1825: Before creating the DelegationEdge object in the
recordDelegationSpawn method, add validation to ensure that both
args.parentSessionId and args.childSessionId exist as agents in manifest.agents,
and verify that args.childRole is consistent with the child agent's configured
role in the manifest. If any validation fails, return an error response with
appropriate details before proceeding to construct the edge object and add it to
the lineage array. This prevents orphaned or incorrect edges from being
persisted due to stale or wrong caller payloads.
---
Nitpick comments:
In `@apps/desktop/src/main/services/orchestration/orchestrationService.test.ts`:
- Around line 1974-2025: The spawn tests in the first and second test cases need
to be anchored to a registered child agent before calling recordDelegationSpawn,
and the spawnFingerprint assertions need to be expanded. First, register a child
agent or worker in the system before calling recordDelegationSpawn to ensure the
tests validate against actual registered children rather than orphan lineage
edges. Second, in the assertion block after recording the delegation spawn,
replace the single check on edge.spawnFingerprint?.provider with comprehensive
assertions that verify all fingerprint fields including modelId,
reasoningEffort, resolvedAt, and routingKey match the provided spawnFingerprint
object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ac367a9-fc08-47cd-b5c0-3af8a0e69b9e
⛔ Files ignored due to path filters (2)
docs/ARCHITECTURE.mdis excluded by!docs/**docs/features/agents/tool-registration.mdis excluded by!docs/**
📒 Files selected for processing (8)
apps/desktop/src/main/services/ai/tools/orchestrationTools.tsapps/desktop/src/main/services/orchestration/manifestNormalization.tsapps/desktop/src/main/services/orchestration/orchestrationDomain.tsapps/desktop/src/main/services/orchestration/orchestrationService.test.tsapps/desktop/src/main/services/orchestration/orchestrationService.tsapps/desktop/src/main/services/orchestration/patchPolicy.test.tsapps/desktop/src/main/services/orchestration/patchPolicy.tsapps/desktop/src/shared/types/orchestration.ts
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
…ds, add-op semantics CodeRabbit: - ensureLineage/validateLineage now check the full required DelegationEdge shape (childRole/spawnedAt/spawnEtag/briefDigest), not just four fields. - recordDelegationSpawn rejects edges whose parent/child aren't registered agents or whose child role mismatches the agent row (no orphan edges). Greptile: - result-side ops use add (not replace) for initially-absent edge fields — RFC-6902-correct and defensive against any applyPatches tightening. - releaseStaleClaims snapshots taskIds against the post-reset projection so a recovered stale task is not recorded onto the edge. +2 regression tests; orchestration suite green (69/69 on the two files), tsc clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d3fc97977
ℹ️ 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".
| // Record the lead→child delegation edge (best-effort; never fails the | ||
| // spawn — the agent row is the source of truth, the edge is provenance). | ||
| try { | ||
| await svc.recordDelegationSpawn( |
There was a problem hiding this comment.
Return the post-lineage etag from spawnAgent
When this recordDelegationSpawn call succeeds it writes /lineage/- via directPatch, advancing serverGeneration and the manifest etag, but the tool ignores that result and later returns patchRes.etag from the earlier /agents/- patch. In the normal successful-spawn path, any caller that uses the returned etag for its next manifestPatch will immediately hit an avoidable etag_conflict; the daemon spawnAgent path in orchestrationDomain.ts has the same stale-etag pattern. Capture the successful lineage result's etag (falling back to patchRes.etag only when the best-effort lineage write fails).
Useful? React with 👍 / 👎.
recordDelegationSpawn advances the manifest etag after the agent-row append, but both spawn sites returned the stale agent-append etag — a caller using it for its next manifestPatch would hit an avoidable etag_conflict. Surface the ledger's etag (fall back to the agent-append etag only when the best-effort lineage write fails). Same fix in the daemon spawn path. (Codex review.) +1 regression test: domain spawn surfaces the lineage etag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex 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". |
What
Adds a delegation lineage ledger to the orchestration system — the lead↔worker/validator spawn and result edges, recorded as first-class manifest state (
manifest.lineage). Today an agent row carries no parent link, so "who spawned whom, for what, and what came back" is only implicit (parent passed tocreateSession+ transient status pings). This makes it durable and inspectable.Idea adapted from t3code PR #2829's thread-lineage model, scoped to ADE's disk-bundle manifest (no event-sourcing rewrite). Lead↔worker/validator only — a worker's own provider-native subagents stay in the observability pane and are deliberately not ingested.
How
DelegationEdge/DelegationStatustypes + optionalmanifest.lineage, normalized to[](back-compat) and shape-validated alongside the existing planning-state validators.recordDelegationSpawn— best-effort, idempotent per child session, sha256 of the dispatched brief + resolved fingerprint; wired into both spawn sites (desktop tool + daemon domain).releaseTaskon a worker's terminal transition;releaseStaleClaimsreconciles edges whose child went terminal without a release (e.g. a self-completed validator) — lead-triggered./lineagedenied to every role inpatchPolicy; edges are written only through the service'sdirectPatch, exactly like the gate-controlled planning state.Scope notes
taskIdsis a snapshot; live truth stays derivable fromtask.assigneeSessionId.cancelled/superseded/supersededBy/parentRunIdare reserved forward-schema (matches the existingcoordinatorSessionId/parentRunIdmanifest reservations), not written in v1.Validation
/qualitydual-review: 0 Blocker / 0 High; 2 behavior-preserving fixes applied; 1 Medium gated as documented v1 semantic./test: orchestration suite 156/156 green (+9 new tests). Typecheck clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Greptile Summary
Adds a durable delegation lineage ledger (
manifest.lineage) to the orchestration system, recording the otherwise-implicit lead→worker/validator spawn and result edges as first-class manifest state. The two issues identified in prior review rounds (result-fieldreplacevsaddops and stale-task-inclusivetaskIdssnapshot in reconcile) have both been addressed in this revision.DelegationEdge/DelegationStatus/SpawnFingerprintadded toorchestration.ts;ensureLineage+validateLineagewire into the existing normalization + validation pipeline with full back-compat (missinglineagedefaults to[]).recordDelegationSpawn(best-effort, idempotent per child session) added to the service;releaseTaskcloses the open edge at terminal task release;releaseStaleClaimsreconciles any edge whose child went terminal without a release (e.g. validator self-complete)./lineageand/lineage/**added toLEAD_DENY_PATTERNS, so all three roles are blocked from forging or rewriting edges via rawmanifestPatch; edges are written exclusively throughdirectPatchinside dedicated service methods.Confidence Score: 5/5
Safe to merge — the ledger is purely additive manifest state, written only through guarded service paths, and the two issues from prior review rounds have been correctly resolved.
Both previously identified concerns (result-field op semantics and stale-task-inclusive taskIds snapshot) are addressed: result-side fields now use
addops andreleaseStaleClaimscomputesownedagainst the post-reset projected manifest. Access control is tight — raw/lineagewrites are denied to all three roles. Back-compat normalization is consistent with existing patterns. Nine new integration tests cover the full lifecycle including idempotency, multi-task v1 semantics, reconcile accuracy, and legacy-manifest defaults.No files require special attention.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Lead participant SpawnPath as Spawn Path (Tool / Domain) participant Service as orchestrationService participant Manifest Lead->>SpawnPath: spawnAgent(role, brief, ...) SpawnPath->>Service: manifestPatch (add agent row) Service->>Manifest: persist → etag-B SpawnPath->>Service: recordDelegationSpawn(parentSessionId, childSessionId, briefText, fingerprint) Service->>Service: validate parent + child registered, role matches Service->>Service: idempotency check (one edge per childSessionId) Service->>Manifest: "directPatch: add /lineage/- {status:running, briefDigest, spawnEtag, ...}" Manifest-->>Service: etag-C Service-->>SpawnPath: "{ok:true, etag:etag-C, edgeId}" SpawnPath-->>Lead: "{sessionId, etag:etag-C}" Note over Service,Manifest: Worker executes task... alt Worker releases task (releaseTask) Service->>Manifest: "ops: task status, agent status=completed/failed" Service->>Service: buildDelegationResultOps(manifest, childSessionId, edgeStatus, taskIds) Service->>Manifest: "directPatch: replace /lineage/{id}/status, add resultSummary/completedAt/taskIds" else Validator self-completes (releaseStaleClaims backstop) Lead->>Service: releaseStaleClaims(runId, bundlePath) Service->>Service: compute projected (post stale-task resets) Service->>Service: find running edges with terminal agents in projected Service->>Manifest: directPatch: stale task resets + edge reconcile ops end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Lead participant SpawnPath as Spawn Path (Tool / Domain) participant Service as orchestrationService participant Manifest Lead->>SpawnPath: spawnAgent(role, brief, ...) SpawnPath->>Service: manifestPatch (add agent row) Service->>Manifest: persist → etag-B SpawnPath->>Service: recordDelegationSpawn(parentSessionId, childSessionId, briefText, fingerprint) Service->>Service: validate parent + child registered, role matches Service->>Service: idempotency check (one edge per childSessionId) Service->>Manifest: "directPatch: add /lineage/- {status:running, briefDigest, spawnEtag, ...}" Manifest-->>Service: etag-C Service-->>SpawnPath: "{ok:true, etag:etag-C, edgeId}" SpawnPath-->>Lead: "{sessionId, etag:etag-C}" Note over Service,Manifest: Worker executes task... alt Worker releases task (releaseTask) Service->>Manifest: "ops: task status, agent status=completed/failed" Service->>Service: buildDelegationResultOps(manifest, childSessionId, edgeStatus, taskIds) Service->>Manifest: "directPatch: replace /lineage/{id}/status, add resultSummary/completedAt/taskIds" else Validator self-completes (releaseStaleClaims backstop) Lead->>Service: releaseStaleClaims(runId, bundlePath) Service->>Service: compute projected (post stale-task resets) Service->>Service: find running edges with terminal agents in projected Service->>Manifest: directPatch: stale task resets + edge reconcile ops endReviews (3): Last reviewed commit: "fix(orchestration): return the post-line..." | Re-trigger Greptile