feat: skip/filter subagent observations (Dynamic Workflows workflow-subagent) — #2736#2741
feat: skip/filter subagent observations (Dynamic Workflows workflow-subagent) — #2736#2741NRKZOZW wants to merge 1 commit into
workflow-subagent) — #2736#2741Conversation
…ack#2736 Claude Code Dynamic Workflows fan a single prompt out to tens-to-hundreds of parallel subagents (agent_type `workflow-subagent`). Every tool call fires a PostToolUse hook → claude-mem creates one provider-analyzed observation per call, so a single run emits hundreds-to-thousands of low-signal observations. That exhausts the provider's free-tier quota (HTTP 429) and trips the restart guard, dropping the rest of the run — including valuable main-session work. Add two settings, filtered BEFORE any worker HTTP call or provider request so the round-trip and the provider tokens are saved: - CLAUDE_MEM_SKIP_SUBAGENT_OBSERVATIONS (default `false`) — skip every observation carrying an agentId. The robust, type-independent lever. - CLAUDE_MEM_SKIP_AGENT_TYPES (default empty) — comma-separated agent_type skip list, e.g. `workflow-subagent,Explore`. Both default off to preserve current behavior (per the issue's acceptance criteria); `workflow-subagent` is documented as the recommended value. The two combine as a union: skip if the global toggle matches OR the agent_type is listed. Implementation: - New pure helper src/shared/should-skip-agent-observation.ts — single source of truth for both filtering points. - Hook handler (src/cli/handlers/observation.ts) filters ahead of the runtime branch, so it covers both the worker and server-beta runtimes. - Worker ingestObservation (src/services/worker/http/shared.ts) filters again as defense-in-depth for any non-hook caller, before the provider call. - Settings wired into SettingsDefaultsManager (interface + defaults) and the viewer constants; documented in docs/public/configuration.mdx. - Tests: helper unit tests + hook-handler integration covering worker and server-beta runtimes, main-session-unaffected, global toggle, and per-type list. - Rebuilt plugin bundles. Future work (out of scope): an optional per-session/minute throughput guard for runaway bursts from any source. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR adds two new opt-in environment settings (
Confidence Score: 4/5Safe to merge with the per-type filter asymmetry understood; the global toggle is the recommended lever and is correctly bounded to agentId. The global toggle is correctly gated on agentId and cannot touch main-session observations. The per-type list matches on agentType alone with no agentId guard, so a main-session hook payload carrying a matching agentType would be silently dropped—contradicting the documented guarantee. Whether this causes real data loss depends on whether Claude Code ever emits agentType on main-session payloads. src/shared/should-skip-agent-observation.ts — the per-type list branch (lines 55-64) and the test at tests/shared/should-skip-agent-observation.test.ts line 71 that validates agentId-absent skipping. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PostToolUse hook fires] --> B{shouldTrackProject?}
B -- No --> Z[return: skip]
B -- Yes --> C{shouldSkipAgentObservation?}
C -- "SKIP_SUBAGENT=true AND agentId present" --> D[reason: subagent_observation]
C -- "agentType in SKIP_AGENT_TYPES, no agentId needed" --> E[reason: agent_type_excluded]
D --> Z
E --> Z
C -- No skip --> F{resolveRuntimeContext}
F -- server-beta --> G[client.recordEvent]
G -- success --> Z
G -- fallback --> H[dispatchToWorker]
F -- worker --> H
H --> I[Worker HTTP POST]
I --> J{ingestObservation}
J --> K{project excluded?}
K -- Yes --> Z
K -- No --> L{tool excluded?}
L -- Yes --> Z
L -- No --> M{shouldSkipAgentObservation defense-in-depth}
M -- Skip --> Z
M -- No --> N[queueObservation to provider]
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/shared/should-skip-agent-observation.ts:55-64
**Per-type list can silently drop main-session observations**
The global toggle (`CLAUDE_MEM_SKIP_SUBAGENT_OBSERVATIONS`) correctly gates on `agentId`, so it is provably safe for main-session events. The per-type list path (`CLAUDE_MEM_SKIP_AGENT_TYPES`) checks only `agentType` — no `agentId` is required — so any main-session observation that carries a matching `agentType` (without an `agentId`) is dropped. The test at line 71 of the test file explicitly validates this: `shouldSkipAgentObservation(undefined, 'workflow-subagent', s).skip === true`. This contradicts the acceptance criterion "Main-session observations are unaffected" and the docs' claim under `CLAUDE_MEM_SKIP_AGENT_TYPES`. If Claude Code ever emits `agentType` on a main-session hook payload (even without `agentId`), those observations would be silently lost.
### Issue 2 of 2
src/services/worker/http/shared.ts:123-125
When `skip` is `true` the `reason` field is always populated, but the `AgentSkipDecision` type declares it as optional (`reason?: AgentSkipReason`). The non-null assertion silences TypeScript here but will throw at runtime if the invariant is ever broken by a future change to `shouldSkipAgentObservation`. Replacing with `?? 'agent_skip'` makes the fallback explicit.
```suggestion
if (agentSkip.skip) {
return { ok: true, status: 'skipped', reason: agentSkip.reason ?? 'agent_skip' };
}
```
Reviews (1): Last reviewed commit: "feat: skip/filter subagent observations ..." | Re-trigger Greptile |
| * Defaults preserve current behavior: with the global toggle off and an empty | ||
| * skip list, this never skips. | ||
| */ | ||
| export function shouldSkipAgentObservation( | ||
| agentId: string | undefined | null, | ||
| agentType: string | undefined | null, | ||
| settings: AgentSkipSettings, | ||
| ): AgentSkipDecision { | ||
| if (settings.CLAUDE_MEM_SKIP_SUBAGENT_OBSERVATIONS === 'true' && agentId) { | ||
| return { skip: true, reason: 'subagent_observation' }; |
There was a problem hiding this comment.
Per-type list can silently drop main-session observations
The global toggle (CLAUDE_MEM_SKIP_SUBAGENT_OBSERVATIONS) correctly gates on agentId, so it is provably safe for main-session events. The per-type list path (CLAUDE_MEM_SKIP_AGENT_TYPES) checks only agentType — no agentId is required — so any main-session observation that carries a matching agentType (without an agentId) is dropped. The test at line 71 of the test file explicitly validates this: shouldSkipAgentObservation(undefined, 'workflow-subagent', s).skip === true. This contradicts the acceptance criterion "Main-session observations are unaffected" and the docs' claim under CLAUDE_MEM_SKIP_AGENT_TYPES. If Claude Code ever emits agentType on a main-session hook payload (even without agentId), those observations would be silently lost.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/shared/should-skip-agent-observation.ts
Line: 55-64
Comment:
**Per-type list can silently drop main-session observations**
The global toggle (`CLAUDE_MEM_SKIP_SUBAGENT_OBSERVATIONS`) correctly gates on `agentId`, so it is provably safe for main-session events. The per-type list path (`CLAUDE_MEM_SKIP_AGENT_TYPES`) checks only `agentType` — no `agentId` is required — so any main-session observation that carries a matching `agentType` (without an `agentId`) is dropped. The test at line 71 of the test file explicitly validates this: `shouldSkipAgentObservation(undefined, 'workflow-subagent', s).skip === true`. This contradicts the acceptance criterion "Main-session observations are unaffected" and the docs' claim under `CLAUDE_MEM_SKIP_AGENT_TYPES`. If Claude Code ever emits `agentType` on a main-session hook payload (even without `agentId`), those observations would be silently lost.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if (agentSkip.skip) { | ||
| return { ok: true, status: 'skipped', reason: agentSkip.reason! }; | ||
| } |
There was a problem hiding this comment.
When
skip is true the reason field is always populated, but the AgentSkipDecision type declares it as optional (reason?: AgentSkipReason). The non-null assertion silences TypeScript here but will throw at runtime if the invariant is ever broken by a future change to shouldSkipAgentObservation. Replacing with ?? 'agent_skip' makes the fallback explicit.
| if (agentSkip.skip) { | |
| return { ok: true, status: 'skipped', reason: agentSkip.reason! }; | |
| } | |
| if (agentSkip.skip) { | |
| return { ok: true, status: 'skipped', reason: agentSkip.reason ?? 'agent_skip' }; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/worker/http/shared.ts
Line: 123-125
Comment:
When `skip` is `true` the `reason` field is always populated, but the `AgentSkipDecision` type declares it as optional (`reason?: AgentSkipReason`). The non-null assertion silences TypeScript here but will throw at runtime if the invariant is ever broken by a future change to `shouldSkipAgentObservation`. Replacing with `?? 'agent_skip'` makes the fallback explicit.
```suggestion
if (agentSkip.skip) {
return { ok: true, status: 'skipped', reason: agentSkip.reason ?? 'agent_skip' };
}
```
How can I resolve this? If you propose a fix, please make it concise.…-observations-2736 feat: skip/filter subagent observations (Dynamic Workflows `workflow-subagent`) — thedotmack#2736 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tee main-session observations are never dropped Greptile flagged that CLAUDE_MEM_SKIP_AGENT_TYPES matched on agentType alone, with no agentId guard, so a main-session payload carrying a listed agent_type (without an agentId) would be silently dropped — contradicting the PR's own acceptance criterion "Main-session observations are unaffected." Both skip levers now require an agentId first: a payload without one is treated as main-session and never skipped. Real subagents always carry both an agentId and an agentType, so this doesn't weaken the per-type lever in practice — it just makes the guarantee true by construction. - src/shared/should-skip-agent-observation.ts: early-return NO_SKIP when no agentId; both branches now reachable only for genuine subagents. - tests: per-type test now asserts a listed agentType without an agentId is NOT skipped (was asserting the opposite). - docs/configuration.mdx: clarify both levers only apply to agentId-carrying observations. - Rebuilt plugin bundles (worker-service.cjs, server-beta-service.cjs). typecheck clean · full bun test 2080 pass / 0 fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Context: #2736 was consolidated into the plan-12 roadmap (#2785) under Ingestion & filtering. This PR is the concrete, tested implementation of that slice — I've flagged it on #2785 so it can serve as the base rather than a from-scratch reimplementation. Happy to rebase or reshape to match the plan-12 slicing, and to address review feedback — including the per-type-vs-main-session semantics the automated review flagged (the per-type list matches on |
Closes #2736.
Problem
Claude Code Dynamic Workflows fan a single prompt out to tens-to-hundreds of parallel subagents (
agent_type: workflow-subagent). Every tool call by every subagent fires aPostToolUsehook → claude-mem creates one provider-analyzed observation per call, so a single run emits hundreds-to-thousands of low-signal observations.As the issue documents with real DB evidence (634 / 699 ≈ 91% of a day's observations came from
workflow-subagent), this:session is dead, clearing pending and terminating) and the rest of the run's observations — including valuable main-session work — are dropped.Subagent summaries are already skipped; subagent observations were not. The only existing lever (
CLAUDE_MEM_SKIP_TOOLS) is global by tool name and can't target subagents without also dropping main-session observations.Solution
Two settings, filtered before any worker HTTP round-trip or provider request — so the request and the provider tokens are saved:
CLAUDE_MEM_SKIP_SUBAGENT_OBSERVATIONSfalsetrue, skip every observation carrying anagentId(any subagent). The robust, type-independent lever.CLAUDE_MEM_SKIP_AGENT_TYPESagent_typevalues to skip, e.g.workflow-subagent,Explore. Surgical control.The two combine as a union (skip if the global toggle matches or the
agent_typeis listed) — simpler than, and a strict superset of, the issue's "priority" framing, and it satisfies every acceptance-criteria bullet.Design decisions (beyond the issue text)
workflow-subagentis documented as the recommended value. Flipping the default to skipworkflow-subagentout of the box is a one-line change inSettingsDefaultsManagerif you'd prefer that — I went conservative deliberately. (Note: a globalagentIdtoggle is more robust than a type-string match, since a non-workflow-subagentburst trips the same restart guard.)src/shared/should-skip-agent-observation.tsis called from:src/cli/handlers/observation.ts) ahead of the runtime branch, so it covers both theworkerandserver-betaruntimes and avoids the round-trip entirely; andsrc/services/worker/http/shared.tsingestObservation) as defense-in-depth for any non-hook caller, beforequeueObservation(the provider call).This mirrors the existing dual-check pattern for
CLAUDE_MEM_EXCLUDED_PROJECTS.Acceptance criteria
CLAUDE_MEM_SKIP_SUBAGENT_OBSERVATIONS=truedrops every observation carrying anagentIdbefore any provider call.CLAUDE_MEM_SKIP_AGENT_TYPES=workflow-subagent,Exploredrops only those types.Tests & verification
tests/shared/should-skip-agent-observation.test.ts): default no-skip, global toggle, per-type list,agentType-without-agentId, whitespace/dedupe parsing, union precedence.tests/cli/handlers/observation-subagent-skip.test.ts): asserts the worker HTTP call /recordEventis not made when skipped (would fail if the filter were removed), across both the worker and server-beta runtimes, and that main-session + non-listed types still flow.ci.yml):npm run typecheckclean ·npm run buildsucceeds (bundle-size guardrails pass) · fullbun test2080 pass / 0 fail. (Actions on a fork PR waits for a maintainer to approve the run.)Out of scope / future work
An optional per-session/minute throughput guard (the issue's secondary ask) would defend against runaway bursts from any source, not just known agent types. It needs worker-side rate state and risks non-deterministic drops, so I left it out to keep this change deterministic and focused; happy to follow up.
🤖 Generated with Claude Code