Repository navigation
fix(agents): EPMCDME-15968- forward forked skill's name on SubagentStart/Stop - #642
Conversation
EPMCDME-15968: a forked skill's .meta.json sidecar already carries the skill's name under `name` (confirmed from a real local session), but SubagentMeta silently dropped it. Parse it so a follow-up commit can forward it on the SubagentStart/Stop hook payload. Generated with AI Co-Authored-By: codemie-ai <codemie.ai@gmail.com>
EPMCDME-15968: the subagent's .meta.json sidecar was already read at the right point in SubagentStop's lifecycle, but its skill name was only threaded into the sibling agent.subagent.usage event, never into the SubagentStop hook payload itself, so the backend never saw it. Thread it in, and add the same sidecar read to SubagentStart, which previously had no handling at all. Generated with AI Co-Authored-By: codemie-ai <codemie.ai@gmail.com>
Bondimm
left a comment
There was a problem hiding this comment.
- CI hasn't run on this PR. Both
CIruns, ford0deaa24and for the earlier push, are waiting for maintainer approval (action_required), so there are no Build / Test (Ubuntu) / Test (Windows) results yet. The branch is 3 commits behindmainand merges cleanly. - Checked locally on
d0deaa24:npm run typecheck, eslint on the 4 changed files, and commitlint on the commits and the title pass.vitest --project unitonclaude-code-otlp,agents/core, the connector test andotlp-forwardpasses (40 files, 582 tests). On a local merge withmain,npm run typecheckand theclaude-code-otlpandagents/coretests also pass (38 files, 535 tests). - Backend contract: the top-level
skill_namefield lands in the typedhook_events.skill_namecolumn (HOOK_TEXT_FIELDSinvocabulary.py), so the name matches. The backend side of this ticket is codemie !4550, which was merged into the branch of !4468, and !4468 is still open againstmain. It reads the skill only fromagent.subagent.start(see the thread ononSubagentStartEvent). The field is additive, so either deploy order is safe. - Checked against the ticket's Expected result (the ticket has no AC list).
skill_count=1andskills_invokedfor/code-revieware not reached end to end (thread ononSubagentStartEvent).scope_kind=skilland the inline slash-command skills are not covered (PR comment). "The dispatch appears in AGENTS/timeline" can't be checked here. !4468 moves a skill-tagged dispatch out of AGENTS ("never also an agent"). Should the forked skill show under SKILLS only, or under both SKILLS and AGENTS as the ticket's wording suggests? - Feature flag: none. Revert: code only and clean. Events already stored keep their
skill_name. - #634 conflicts with this PR in
claude-code-otlp.plugin.ts(evaluate), so whichever merges second needs a rebase. #634 also sendsagent.skill.dispatchwithskill_namefor every slash command. If that hook also fires for a forked skill's/code-review, the backend would count it twice once the start event carriesskill_name, because the skill branch oftool_facts_dailycounts both events. Worth checking against !4468. #641, #646, #647 and #651 merge cleanly with this PR.
What needs rechecking after the next push:
- CI on the new head, once approved;
- a SubagentStart payload without
agent_transcript_pathgetsskill_namefor a forked skill; - a named regular agent is not tagged as a skill.
|
Two expected results of EPMCDME-15968 are not covered Problem: The ticket has no AC list. Its Expected result reads: "Skills invoked through slash commands count as skills (skill_count=1 for /code-review), appear in skills_invoked, and the forked skill is attributed to scope_kind=skill / scope_name=code-review; the dispatch appears in AGENTS/timeline."
Impact: With this PR and !4468 merged, cost and token views still attribute a forked skill's tokens to Recommendation: For the scope, consider passing a skill scope in |
…EPMCDME-15968) Two real bugs confirmed by running the real evaluate() against this machine's actual subagent sidecar files: - onSubagentStartEvent gated on agent_transcript_path, a field SubagentStart's real payload never has (confirmed against the SDK's own SubagentStartHookInput type and real marker-file mtimes), so skill_name never actually reached agent.subagent.start. Now builds the subagent's own transcript path from the triggering hook's transcript_path + agent_id instead. - Both handlers read .meta.json's name field as the skill signal, but that is the Agent tool's generic addressing name (set for any named agent, forked skill or not) — confirmed with a real agent named 'otel-deep-analysis' on this machine that would have been misclassified. Now reads the real <agent>.forked-skill.marker.json sidecar instead. Also scopes a forked skill's usage as scope_kind:'skill'/scope_name:<skillName> in deriveSubagentEvents, instead of 'agent'/<agentType>, per the ticket's Expected result. Verified: tsc --noEmit clean, eslint clean, 210/210 vitest pass (new regression tests for both bugs using the real file shapes found on disk). Confirmed live end-to-end against the local analytics backend after linking this build into a real Claude Code session: a real /code-review forked-skill dispatch's agent.subagent.start hook event now carries skill_name='code-review', and its usage_requests rows carry scope_kind='skill'/scope_name='code-review'. Not addressed here (separate ticket, EPMCDME-15301/codemie-ai#634): inline slash-command skills (/skill-doctor, /anthropic-skills:pdf) still need a UserPromptExpansion -> agent.skill.dispatch mapping that does not exist on main yet. Generated with AI Co-Authored-By: codemie-ai <codemie.ai@gmail.com>
|
Fixed the Also fixed the two line-level bugs youfound, which were the actual reason the ticket's main case stayed unfixed: Not touched here: the inline slash-command case ( |
|
Test results after the latest push (bfb6c73)
Live end-to-end verification (beyond unit tests) is in my earlier replies on this PR: a real CI itself (GitHub Actions) is still `action_required" — pending maintainer approval on run https://github.com/codemie-ai/codemie-code/actions/runs/39271966994, same as the earlier runs on this PR. |
…MCDME-15968) Two bugs found by review and confirmed by source inspection: - deriveSubagentEvents mixed a truthy check (scope_kind) with a nullish check (scope_name) on SubagentFile.skillName, so an empty-string skillName would produce scope_kind='agent' with scope_name='' instead of falling back to agentType. Both now use the same truthy check. - onSubagentStartEvent had no agent_type guard, unlike onSubagentStopEvent, which filters out Claude Code's internal non-subagent check-ins. Added the same guard so a check-in SubagentStart is never forwarded or read. Verified live against a real forked-skill subagent from this session (/code-review dispatch): skill_name and scope_kind/scope_name still correct after the fix, and a synthetic no-agent_type SubagentStart produces zero spool entries, confirming the new guard works end-to-end through the linked CLI, not just in unit tests. 212/212 vitest passing (2 new regression tests), tsc --noEmit clean, eslint clean. Generated with AI Co-Authored-By: codemie-ai <codemie.ai@gmail.com>
|
A forked skill is counted twice once #634 is merged too Problem: Claude Code runs the Impact: With #634, this PR and !4468 merged (all three are needed for the ticket: #634 for the inline slash-command skills), Recommendation: The backend seems the simplest place to fix this. !4468 already has the same kind of guard for spans: |
…PMCDME-15968) deriveSubagentEvents scopes a forked skill's subagent usage as scopeKind:'skill', same as a main-thread inline skill, but deriveMainEvents' api_calls filter only checked scopeKind, not agentId, so a forked skill's subagent requests (which carry their own agentId) were double-counted into the main session's api_calls once persisted by their own SubagentStop. Added the missing agentId guard: main-thread records always carry an empty agentId, so this only changes behavior for the forked-skill case. Verified live against two real past /code-review forked-skill sessions found on this machine (28 and 14 subagent requests): pre-fix api_calls was inflated by exactly that amount (132 instead of 90); post-fix it matches the main-thread count (90). Also added a unit regression test reproducing the same sequence (SubagentStop persists first, then a later Stop's summary must not inflate), confirmed to fail against the pre-fix filter and pass after. 213/213 claude-code-otlp tests passing, 538/538 across claude-code-otlp + agents/core, tsc --noEmit clean, eslint clean. Generated with AI Co-Authored-By: codemie-ai <codemie.ai@gmail.com>
|
Pushed fbd9e58 fixing the remaining open thread: |
…lePath (EPMCDME-15968) Found by a multi-lens code review (blind + edge-case + acceptance) of this branch, each independently verified against real data/behavior before landing: - claude-code-otlp.plugin.ts: onSubagentStopEvent's agentId fallback (hookInput.agent_id || basename(agentTranscriptPath)-stripped) could resolve to '' when both are empty (a degenerate transcript path). An empty agentId satisfies deriveMainEvents' recent `!r.agentId` guard, so a forked skill with this fallback would silently reopen the exact api_calls double-count that guard exists to close. Now falls back further to the (already confirmed non-empty) agentTranscriptPath itself. - transcript/subagent-usage.ts: the new subagentFilePath(mainTranscriptPath, agentId) joined agentId straight into a file path with no validation, so a crafted agent_id could resolve outside the subagents directory and have its arbitrary JSON content trusted as a skill marker. Now confines the result with isPathWithinDirectory (the project's documented path-security pattern), falling back to a safe, guaranteed-absent in-bounds path — never throwing, matching this module's existing "never throws" contract for every other sidecar reader. Verified both unit and live: - Unit: one regression test per fix, each confirmed to fail against the pre-fix code (stashed just the two source changes, kept the new tests) before passing after. Full suite 215/215, tsc --noEmit clean, eslint --max-warnings=0 clean. - Live, against the real compiled build: fed subagentFilePath actual "../../../etc/passwd"-style payloads — all resolved in-bounds. Fed the real onSubagentStopEvent fallback formula a genuinely degenerate "agent-.jsonl" transcript path with no agent_id, ran it through the real compiled openSubagentPass -> openMainTranscriptPass, and confirmed api_calls stayed correct (main-thread count only) instead of being inflated. Both fixes are isolated to code this PR's own commits introduced/rely on; neither touches or conflicts with the earlier api_calls filter fix. Generated with AI Co-Authored-By: codemie-ai <codemie.ai@gmail.com>
Summary
Forward a forked skill's name from its
.forked-skill.marker.jsonsidecar onto theSubagentStart/SubagentStophook payload asskill_name, scope its usage asscope_kind: 'skill'/scope_name: <skillName>(instead of'agent'/<agentType>), and keep the main session'sapi_callscorrectly excluding that subagent usage — so CodeMie CLI Analytics classifies a skill invoked via slash command and forked as a background subagent (e.g./code-review) as a skill instead of a generic agent dispatch.Motivation
EPMCDME-15968: skills run as slash commands (forked skills) are not counted —
skill_countis 0 and the skill subagent is not reflected in CLI Analytics.Changes
transcript/subagent-usage.ts:SubagentFile.skillNameis read from the subagent's.forked-skill.marker.jsonsidecar (readForkedSkillMarker, only whenforkedSkill === true) — not from.meta.json'sname, which is the Agent tool's generic addressing name and is also set for any named agent, forked skill or not.claude-code-otlp.plugin.ts:onSubagentStartEventforwardsskill_nameonSubagentStart(building the subagent's transcript path from the triggering hook'stranscript_path+agent_id, sinceSubagentStart's real payload has noagent_transcript_path);onSubagentStopEventdoes the same onSubagentStop. Both now guard onagent_typeso an internal Claude Code check-inSubagentStartis never forwarded.transcript/orchestrator.ts:deriveSubagentEventsscopes a forked skill's subagent usage asscope_kind: 'skill'/scope_name: <skillName>(falling back toagentTypeconsistently whenskillNameis empty), matchingderiveMainEvents' existing pattern for an inline skill.deriveMainEvents'api_callsfilter now also requires an emptyagentId, so a forked skill's subagent requests (which carry their ownagentIddespitescope_kind: 'skill') are excluded from the main session'sapi_calls, same as a regular agent's always were.claude-code-otlp.plugin.ts:onSubagentStopEvent'sagentIdfallback (hookInput.agent_id || basename(agentTranscriptPath)-stripped) could resolve to''for a degenerate transcript path, which would satisfy the new!r.agentIdguard above and reopen the exactapi_callsdouble-count it was added to close. Now falls back further to the (already confirmed non-empty)agentTranscriptPathitself.transcript/subagent-usage.ts: the newsubagentFilePath(mainTranscriptPath, agentId)joinedagentIdstraight into a file path with no validation. Now confines the result withisPathWithinDirectory(the project's documented path-security pattern,.ai-run/guides/security/security-practices.md), falling back to a safe, guaranteed-absent in-bounds path rather than ever resolving outside the subagents directory — never throwing, matching this module's existing "never throws" contract.Testing
subagent-usage.test.ts,claude-code-otlp.plugin.test.ts,orchestrator.test.ts: regression tests for the forked-skill-marker fix, theSubagentStartpath fix, thescope_kind/scope_nameattribution, theagent_typecheck-in guard, theapi_callsexclusion, and the two hardening fixes above (each confirmed to fail against the pre-fix code before passing after).skill_name/scope_kind/scope_nameconfirmed correct against genuine past/code-reviewdispatches found on disk on this machine;agent.session.summary.api_callsconfirmed no longer inflated (pre-fix: 132 = 90 main + 42 subagent; post-fix: 90);subagentFilePathfed actual../../../etc/passwd-style payloads and confirmed to stay in-bounds; theagentIdfallback fed a genuinely degenerate transcript path with noagent_idand confirmedapi_callsstill held correct end-to-end throughopenSubagentPass→openMainTranscriptPass.npx tsc --noEmit -p .: clean.eslint --max-warnings=0: clean.claude-code-otlpsuite: 215/215.Not covered by this PR
/skill-doctor,/anthropic-skills:pdf) still depend on feat(analytics): Enrich existing hook events and add new synthetic ones #634 (EPMCDME-15301) landingUserPromptExpansion→agent.skill.dispatch.agent.skill.dispatchfrom feat(analytics): Enrich existing hook events and add new synthetic ones #634'sUserPromptExpansionhook, oneagent.subagent.startfrom this PR, both for the same dispatch) — flagged by review; the backend side (!4468/rollups.py) is the suggested place to de-duplicate, similar to its existing_NO_SKILL_SPAN_OF_NAMEguard.Related
Backend companion fix (classifies this new signal, un-gates existing skill counting): https://gitbud.epam.com/epm-cdme/codemie/-/merge_requests/4550
Relates to EPMCDME-15968