HOMELAB-1850: feat(otel): span attribution closure + phase tags (LD4.5d) #60

Merged
aaron merged 2 commits from plane/HOMELAB-1850-attribution into main 2026-10-03 03:49:21 +00:00
Owner

Closes the LD4.5d leg in pi-extensions: the HOMELAB-1591 residuals the brief names, the
LD4.5d phase key, and the dispatch recipe that binds it.

Brief: .rpiv/artifacts/epic-ld4/design-LD4.5d-attribution.md (ticket HOMELAB-1850).
Territory: LD4.4 owns the sandbox_env key + R-06 rollup; this PR owns the phase
key + span-parent residuals at the same emission sites. routines/rollup.go,
sandbox.ts, pi-friction and the swarm-view UI are untouched.

Commit 1 — 1591 residuals + the phase key (pi-otel-telemetry)

  • plane.ticket_id is a VALUE ("none") on every subagent span, never an
    absent attribute. The token counter has always written none; a span without
    one made "no ticket context" indistinguishable from "span predates the key".
  • pi.cwd on every subagent span (event cwd → dispatching session cwd), the
    same join key the root span already writes, so a reviewer dispatched from a
    worktree is placeable.
  • TRACEPARENT ingestion for subprocess children (new traceparent.ts).
    Every landing-pipeline reviewer is pi -p --no-session spawned from bash, emits
    no swarm event, and used to start a NEW trace. pendingDispatches still wins for
    in-process children; the env value is validated (W3C version 00, no all-zero
    placeholders, no extra fields) and degrades to no-parent rather than to a
    half-extracted context.
  • phase.ts — the closed enum build | review | verify plus the honest fourth
    value unphased, resolved ONCE at tracker construction (a mirror of
    sandbox.ts, deliberately copied rather than reinvented). The match is exact:
    "Build" / "deploy" / "" / "bogus; rm -rf" are REFUSED into unphased,
    never guessed at.
  • phase on every subagent span AND on the pi_agent_tokens_total sample,
    resolved once per dispatch (spawn declaration → process binding → unphased) and
    stored on the open-span entry, so span and counter can never disagree — a
    disagreement would silently break the sum invariant.
  • pi_agent_unphased_spend_total — the finding series, mirroring
    pi_agent_unbound_spend_total, so the Go leg's P-ROUT-012 is alertable.

Commit 2 — the spawn declaration + the dispatch recipe (pi-swarm-suite)

  • AgentRecord.phase (next to background/isolated, same stated reason:
    the span needs it at span-create time), SpawnOptions.phase, validated by
    assertValidSpawnPhase — reject, never coerce — and both spawn paths plus the
    Agent tool's new optional phase parameter.
  • subagents:started now carries phase and cwd.
  • dispatch-env.ts: the recipe as data — the file-delivery shape
    ClaimBinding.EnvPairs() established, rendering LD_ROLE/LD_INSTANCE (LD2.7)
    • LD_PHASE + TRACEPARENT, with renderDispatchEnvFile and
      subprocessEnvPrefix.
  • docs/dispatch-attribution-recipe.md: who renders what where. Claim binding =
    identity; dispatch recipe = stage (the stage changes per dispatch inside one
    sandbox, which a claim-time binding cannot express); telemetry reads and refuses.

Verification

  • pi-otel-telemetry: 62 tests passing (phase.test.ts, traceparent.test.ts
    added; the sandbox_env anti-spoof pattern mirrored for phase — a child that
    mutates the env after capture changes nothing; span/counter agreement asserted).
  • pi-swarm-suite: 1464 tests passing (dispatch-env.test.ts added).
  • tsc --noEmit clean in both units.

Not in this PR (by territory)

  • sandbox_env attribute/key and the R-06 rollup (LD4.4).
  • The swarm-view UI rendering (src/view/*) — 3.4's slot. The record now carries
    phase, so that surface can derive it without another schema change.
  • pi-friction (4.6 owns it).
  • The Go phase census/routine (routines/phase.go) and the Tempo values change
    land separately (infra-core; the Tempo one flagged OPERATOR REVIEW).
Closes the LD4.5d leg in pi-extensions: the HOMELAB-1591 residuals the brief names, the LD4.5d phase key, and the dispatch recipe that binds it. Brief: `.rpiv/artifacts/epic-ld4/design-LD4.5d-attribution.md` (ticket HOMELAB-1850). Territory: LD4.4 owns the `sandbox_env` key + R-06 rollup; this PR owns the `phase` key + span-parent residuals at the same emission sites. `routines/rollup.go`, `sandbox.ts`, pi-friction and the swarm-view UI are untouched. ## Commit 1 — 1591 residuals + the phase key (pi-otel-telemetry) * **`plane.ticket_id` is a VALUE (`"none"`)** on every subagent span, never an absent attribute. The token counter has always written `none`; a span without one made "no ticket context" indistinguishable from "span predates the key". * **`pi.cwd` on every subagent span** (event cwd → dispatching session cwd), the same join key the root span already writes, so a reviewer dispatched from a worktree is placeable. * **`TRACEPARENT` ingestion for subprocess children** (new `traceparent.ts`). Every landing-pipeline reviewer is `pi -p --no-session` spawned from bash, emits no swarm event, and used to start a NEW trace. `pendingDispatches` still wins for in-process children; the env value is validated (W3C version 00, no all-zero placeholders, no extra fields) and degrades to no-parent rather than to a half-extracted context. * **`phase.ts`** — the closed enum `build | review | verify` plus the honest fourth value `unphased`, resolved ONCE at tracker construction (a mirror of `sandbox.ts`, deliberately copied rather than reinvented). The match is exact: `"Build"` / `"deploy"` / `""` / `"bogus; rm -rf"` are REFUSED into `unphased`, never guessed at. * **`phase` on every subagent span AND on the `pi_agent_tokens_total` sample**, resolved once per dispatch (spawn declaration → process binding → `unphased`) and stored on the open-span entry, so span and counter can never disagree — a disagreement would silently break the sum invariant. * **`pi_agent_unphased_spend_total`** — the finding series, mirroring `pi_agent_unbound_spend_total`, so the Go leg's P-ROUT-012 is alertable. ## Commit 2 — the spawn declaration + the dispatch recipe (pi-swarm-suite) * `AgentRecord.phase` (next to `background`/`isolated`, same stated reason: the span needs it at span-create time), `SpawnOptions.phase`, validated by `assertValidSpawnPhase` — **reject, never coerce** — and both spawn paths plus the Agent tool's new optional `phase` parameter. * `subagents:started` now carries `phase` and `cwd`. * `dispatch-env.ts`: the recipe as data — the file-delivery shape `ClaimBinding.EnvPairs()` established, rendering `LD_ROLE`/`LD_INSTANCE` (LD2.7) + `LD_PHASE` + `TRACEPARENT`, with `renderDispatchEnvFile` and `subprocessEnvPrefix`. * `docs/dispatch-attribution-recipe.md`: who renders what where. Claim binding = identity; dispatch recipe = stage (the stage changes per dispatch inside one sandbox, which a claim-time binding cannot express); telemetry reads and refuses. ## Verification * `pi-otel-telemetry`: 62 tests passing (`phase.test.ts`, `traceparent.test.ts` added; the `sandbox_env` anti-spoof pattern mirrored for phase — a child that mutates the env after capture changes nothing; span/counter agreement asserted). * `pi-swarm-suite`: 1464 tests passing (`dispatch-env.test.ts` added). * `tsc --noEmit` clean in both units. ## Not in this PR (by territory) * `sandbox_env` attribute/key and the R-06 rollup (LD4.4). * The swarm-view UI rendering (`src/view/*`) — 3.4's slot. The record now carries `phase`, so that surface can derive it without another schema change. * `pi-friction` (4.6 owns it). * The Go phase census/routine (`routines/phase.go`) and the Tempo values change land separately (infra-core; the Tempo one flagged OPERATOR REVIEW).
The HOMELAB-1591 residuals the brief names, at the subagent emission site, plus
the LD4.5d phase key they share the site with:

* plane.ticket_id is now a VALUE ("none") on every subagent span, never an
  absent attribute — the token counter has always written "none", so a TraceQL
  reader can no longer confuse "no ticket context" with "span predates the key".
* pi.cwd is on every subagent span (event cwd -> dispatching session cwd), the
  SAME join key the root span already writes, so a reviewer dispatched from a
  worktree is placeable.
* TRACEPARENT ingestion for SUBPROCESS children (new traceparent.ts): every
  landing-pipeline reviewer is `pi -p --no-session` spawned from bash, emits no
  swarm event, and used to start a new trace. `pendingDispatches` still wins for
  in-process children; the env value is validated (W3C version 00, no all-zero
  placeholders) and degrades to no-parent rather than a half-extracted context.
* phase.ts: the closed enum (build|review|verify) + the honest fourth value
  `unphased`, resolved once at tracker construction (mirror of sandbox.ts),
  refusing anything out of the enum — exact match, never guessed.
* subagents.ts/index.ts: `phase` on every subagent span AND on the
  pi_agent_tokens_total sample, resolved once per dispatch (spawn declaration ->
  process binding -> unphased) so span and counter can never disagree; plus
  pi_agent_unphased_spend_total, the finding series (P-ROUT-012 in the Go leg).

Tests: 62 passing in pi-otel-telemetry (phase + traceparent suites added, the
sandbox_env anti-spoof pattern mirrored for phase). tsc --noEmit clean.
HOMELAB-1850: feat(swarm): the phase declaration on the spawn path + the dispatch recipe
All checks were successful
CI / pr-title (pull_request) Successful in 1s
CI / forgejo-ci (pull_request) Successful in 48s
CI / skill-args (pull_request) Successful in 47s
CI / pi-otel-telemetry (pull_request) Successful in 1m5s
CI / nats-intercom (pull_request) Successful in 21s
CI / plane (pull_request) Successful in 1m23s
CI / forgejo (pull_request) Successful in 1m22s
CI / mcp-bridge (pull_request) Successful in 29s
CI / todo (pull_request) Successful in 51s
CI / ask-user-question (pull_request) Successful in 51s
CI / refocus (pull_request) Successful in 58s
CI / pi-swarm-suite (pull_request) Successful in 3m50s
59518ae6f2
Tier 1 of the phase key (the dispatcher's own declaration), carried on the same
precedent as background/isolated — captured on the record at spawn so the
lifecycle event can hand it to the OTel span at SPAN-CREATE time:

* types.ts: the closed enum `SPAWN_PHASES` + `isSpawnPhase` (exact match; the
  telemetry unit and routines/phase.go state the same enum rather than import
  across a package boundary), and `AgentRecord.phase`.
* agent-manager.ts: `SpawnOptions.phase`, validated by `assertValidSpawnPhase`
  beside `assertValidSpawnCwd` — REJECT, never coerce (silently coercing a typo'd
  stage would hide the bug in exactly the place the key exists to expose it).
* index.ts: the Agent tool gains an optional `phase` parameter; both spawn paths
  pass it; `subagents:started` now carries `phase` AND `cwd` (the workspace join
  key, already on the record — 1591 residual).
* dispatch-env.ts: the recipe as data — `dispatchEnvPairs` /
  `renderDispatchEnvFile` / `subprocessEnvPrefix`, the file-delivery shape
  `ClaimBinding.EnvPairs()` established, rendering LD_ROLE/LD_INSTANCE (LD2.7's
  keys) + LD_PHASE + TRACEPARENT. It refuses an out-of-enum phase and omits
  unbound keys.
* docs/dispatch-attribution-recipe.md: who renders what where — claim binding =
  identity, dispatch recipe = stage (it changes per dispatch inside one sandbox),
  telemetry reads and refuses.

Tests: pi-swarm-suite 1464 passing (dispatch-env suite added); tsc clean.
Author
Owner

Review verdict — CLEAN (no blocker findings)

Reviewed from a fresh context on primary sources only: the PR diff (13 files, +986/−9), the HOMELAB-1850 ticket/verify lines, design-LD4.5d-attribution.md, and the repo at head 59518ae. Reviewer ≠ implementer; implementer's summary not used as the brief.

Findings per review focus

  1. phase.ts — PASS. Closed enum PHASES = ["build","review","verify"]; unphased is the fourth value, explicitly not a member. Exact match (no case-folding, deliberately stricter than normalizeSandboxEnv): "Build", "deploy", "", "bogus; rm -rf" all → unphased. Resolved once at construction — SubagentSpanTracker ctor param 6 defaults to resolvePhase(); index.ts resolves it once beside resolveSandboxEnv() and injects it. Unspoofability tested: mutating process.env.LD_PHASE after construction leaves the emitted attribute "build". Emitted as an attribute value on every span, never absent. Out-of-enum refused into unphased (not coerced, not fallen back to a stale binding — effectivePhase("Deploy", bound("build")) === "unphased", tested).
  2. 1591 residuals — PASS. plane.ticket_id written as carry.ticketId || "none" on every subagent span (value, never absence). pi.cwd written from e.cwd ?? opts?.cwd, with StartOptions.cwd = state.cwd populated unconditionally in before_agent_start (systemPromptOptions.cwd || process.cwd()) — same join key as the root span. TRACEPARENT ingested at claimDispatchParent(key) ?? envTraceparent() ?? paperclipTraceparent — correct precedence (in-process wins); validator enforces W3C version 00, rejects all-zero trace/parent ids and extra fields, degrades to no parent rather than a half-extracted context.
  3. Test claims — CONFIRMED BY RE-RUN. pi-otel-telemetry 62 passed (4 files); pi-swarm-suite 1464 passed (104 files). tsc --noEmit exit 0 in both units.
  4. Territory — PASS. Diff touches only pi-otel-telemetry + pi-swarm-suite. sandbox.ts, routines/rollup.go/rollup-adjacent code, src/view/* and pi-friction (HOMELAB-1690) are absent from the diff — 4.4's and the sibling's territory untouched. main advanced only in voice-whisper since the branch base; zero file overlap, so green CI on head is valid for the merge tree.
  5. Modified assertions — justified, not weakened. The two samples).toEqual([...]) assertions remain exact full-object equality; they only gained phase: "build". No loosening to toMatchObject/partial matching. TokenSample.phase being required plus clean tsc proves no sample-construction site was missed.
  6. Chain complete end-to-end. Agent tool params.phase → SpawnOptions.phase (validated by assertValidSpawnPhase, reject-never-coerce, at both spawn paths) → single AgentRecord construction site (phase: options.phase ?? undefined) → subagents:started payload (phase, plus cwd) → StartedEvent.phase → effectivePhase at span create → span attr phase and entry.phase on the open-span entry → TokenSample.phase. Span and counter share one resolved value, so the sum invariant cannot silently break.

Concerns (non-blocking, posted for the record)

  • trackerPhase() is exported but has no consumer — it mirrors the pre-existing unconsumed trackerSandboxEnv(), so it follows precedent rather than introducing a new dead-code pattern. The startup log uses phaseBinding.source directly.
  • Minor asymmetry: SpawnOptions.phase = "" is rejected on the spawn path while dispatch-env's assertPhase("") treats an empty value as an absent binding. Both are documented and defensible (API reject vs. omit-key); no behavioural risk.
  • pi.cwd is if (cwd)-guarded rather than guaranteed-value; in production state.cwd is always populated so every subagent span carries it. Worth noting only for a hypothetical caller with neither record nor session cwd.

Landing: CI green on 59518ae (run #116, 12/12 jobs incl. pi-otel-telemetry + pi-swarm-suite). Class C (extension TS), not dangerous — merge proceeds.

## Review verdict — CLEAN (no blocker findings) Reviewed from a **fresh context** on primary sources only: the PR diff (13 files, +986/−9), the HOMELAB-1850 ticket/verify lines, `design-LD4.5d-attribution.md`, and the repo at head `59518ae`. Reviewer ≠ implementer; implementer's summary not used as the brief. ### Findings per review focus 1. **`phase.ts` — PASS.** Closed enum `PHASES = ["build","review","verify"]`; `unphased` is the fourth *value*, explicitly not a member. Exact match (no case-folding, deliberately stricter than `normalizeSandboxEnv`): `"Build"`, `"deploy"`, `""`, `"bogus; rm -rf"` all → `unphased`. Resolved **once** at construction — `SubagentSpanTracker` ctor param 6 defaults to `resolvePhase()`; `index.ts` resolves it once beside `resolveSandboxEnv()` and injects it. Unspoofability tested: mutating `process.env.LD_PHASE` after construction leaves the emitted attribute `"build"`. Emitted as an attribute value on **every** span, never absent. Out-of-enum refused into `unphased` (not coerced, not fallen back to a stale binding — `effectivePhase("Deploy", bound("build")) === "unphased"`, tested). 2. **1591 residuals — PASS.** `plane.ticket_id` written as `carry.ticketId || "none"` on every subagent span (value, never absence). `pi.cwd` written from `e.cwd ?? opts?.cwd`, with `StartOptions.cwd = state.cwd` populated unconditionally in `before_agent_start` (`systemPromptOptions.cwd || process.cwd()`) — same join key as the root span. `TRACEPARENT` ingested at `claimDispatchParent(key) ?? envTraceparent() ?? paperclipTraceparent` — correct precedence (in-process wins); validator enforces W3C version `00`, rejects all-zero trace/parent ids and extra fields, degrades to *no parent* rather than a half-extracted context. 3. **Test claims — CONFIRMED BY RE-RUN.** `pi-otel-telemetry` **62 passed** (4 files); `pi-swarm-suite` **1464 passed** (104 files). `tsc --noEmit` exit 0 in both units. 4. **Territory — PASS.** Diff touches only `pi-otel-telemetry` + `pi-swarm-suite`. `sandbox.ts`, `routines/rollup.go`/rollup-adjacent code, `src/view/*` and `pi-friction` (HOMELAB-1690) are absent from the diff — 4.4's and the sibling's territory untouched. `main` advanced only in `voice-whisper` since the branch base; zero file overlap, so green CI on head is valid for the merge tree. 5. **Modified assertions — justified, not weakened.** The two `samples).toEqual([...])` assertions remain exact full-object equality; they only gained `phase: "build"`. No loosening to `toMatchObject`/partial matching. `TokenSample.phase` being *required* plus clean `tsc` proves no sample-construction site was missed. 6. **Chain complete end-to-end.** `Agent tool params.phase` → `SpawnOptions.phase` (validated by `assertValidSpawnPhase`, reject-never-coerce, at **both** spawn paths) → single `AgentRecord` construction site (`phase: options.phase ?? undefined`) → `subagents:started` payload (`phase`, plus `cwd`) → `StartedEvent.phase` → `effectivePhase` at span create → span attr `phase` **and** `entry.phase` on the open-span entry → `TokenSample.phase`. Span and counter share one resolved value, so the sum invariant cannot silently break. ### Concerns (non-blocking, posted for the record) - `trackerPhase()` is exported but has no consumer — it mirrors the pre-existing unconsumed `trackerSandboxEnv()`, so it follows precedent rather than introducing a new dead-code pattern. The startup log uses `phaseBinding.source` directly. - Minor asymmetry: `SpawnOptions.phase = ""` is *rejected* on the spawn path while `dispatch-env`'s `assertPhase("")` treats an empty value as an *absent* binding. Both are documented and defensible (API reject vs. omit-key); no behavioural risk. - `pi.cwd` is `if (cwd)`-guarded rather than guaranteed-value; in production `state.cwd` is always populated so every subagent span carries it. Worth noting only for a hypothetical caller with neither record nor session cwd. Landing: CI green on `59518ae` (run #116, 12/12 jobs incl. `pi-otel-telemetry` + `pi-swarm-suite`). Class C (extension TS), not dangerous — merge proceeds.
aaron merged commit 79671ee57a into main 2026-10-03 03:49:21 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Homelab/pi-extensions!60
No description provided.