Skip to content

fix(score): count a skill read that may not have run only when its output shows it - #1358

Merged
jeff-r2026 merged 1 commit into
mainfrom
saulmoro/skill-reads-from-output
Oct 24, 2026
Merged

jeff-r2026 merged 1 commit into
mainfrom
saulmoro/skill-reads-from-output

Conversation

@SaulMoro

Copy link
Copy Markdown
Collaborator

Summary

used.skills credited a SKILL.md read that never ran: one a longer command line names after && (false && cat .claude/skills/review/SKILL.md; true), or any tools.exec_command in a Codex code-mode script, including one under if (false). Such a read now counts only when the call's output shows the skill's frontmatter name: line. Follow-up to #1298, where review raised both cases.

 toolCallSkills(event)                       src/skill-use.ts
   loaded  the Skill tool, the Read tool, a shell read classifyToolCall is sure ran
-          plus any reader namedReads finds, except after ||
+  named   any reader namedReads finds, after &&, ||, ; or a newline
 a live hook, or a recorded call replayed
-  credit loaded
+  credit loaded + the named reads its own output shows
 a Codex code-mode script
-  credit the reads of every exec_command it holds
+  credit the reads of its exec_commands that the script's output shows
  • The name: line must be a whole line, raw, numbered by cat -n, or JSON-escaped the way code mode prints a command's result. An inline description: "name: review" does not count.
  • Scoring now replays each recorded call with its output, as the live hook receives it. A code-mode script's output covers all its commands, so it confirms their reads but is not read as any one command's error lines.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature causing existing behavior to change)
  • Documentation only
  • Refactor / internal cleanup

Test Plan

  • npx tsc --noEmit passes
  • npm run lint passes
  • npx vitest run passes: 536 files passed, 1 skipped. 15412 tests passed, 21 skipped.
  • Added/updated tests for the change
Check Result
npm run build, npx tsc --noEmit, npm run lint, and the touched tests at c6011a0a Pass
npm run test:e2e -- score-session score-external-driver workflow-records-reports-branch-1083 3 files, 4 tests passed
New tests that fail on main A Claude Bash false && cat FILE; true with empty output; test -f /missing || cat FILE; true whose output shows the skill; a Codex exec_command skipped after &&; a code-mode call under if (false)
New tests that pass on main and guard the rule A read after && that printed the skill; an inline name: in a description; a compound read and a cat -n read inside code mode, printed as JSON; a code-mode read followed by another command's cat: <same path>: No such file or directory

Real CLI, after npm run build at main (fb17c488) and at this head (c6011a0a): teamai score --session on a Codex rollout copied from a real session, plus one call naming .codex/skills/wayfinder/SKILL.md, a team skill the session's checkout delivered, relative to its workdir. poteto-mode is a load the copied session made itself.

Call added used.skills on main used.skills at this head
exec_command false && cat … ; true, empty output poteto-mode, wayfinder poteto-mode
code mode, the call under if (false), output done poteto-mode, wayfinder poteto-mode
code mode, the read ran, its result printed as JSON poteto-mode, wayfinder poteto-mode, wayfinder
code mode, the read on one line of sed …\nrg …, printed as JSON poteto-mode, wayfinder poteto-mode, wayfinder

Not run here: a live Codex PostToolUse hook. Codex CLI 0.163.0 lists tool_response in its PostToolUse input, and the unit tests use that shape. Other agents and the gitlab and github providers are left to CI.

Related Issues

Follow-up to #1298: #1298 (comment)

Notes for Reviewers

  • Door: two-way. Nothing stored changes shape; a revert restores the old crediting.
  • Blast radius: skill credit from compound shell commands and Codex code mode, in live usage and in used.skills on session and workflow labels. A read only a longer command line names now needs output: a partial read below the frontmatter (sed -n '20,40p'), output a tool truncated before the name: line, or a hook that sends no output no longer credits it. Single reads, the Skill tool and the Read tool are unchanged.
  • namedReads drops its || exclusion, since the output check covers it.

…tput shows it

A SKILL.md read named in a longer command line after `&&` or `||`, or in
any command of a Codex code-mode script, may never have run:
`false && cat .claude/skills/review/SKILL.md; true` and an
`exec_command` under `if (false)` were both credited. Such a read now
counts only when the call's output shows the skill's frontmatter
`name:` line, as a whole line, in raw, `cat -n` or JSON-escaped form.

Scoring replays each recorded call with its output, as the live hook
receives it. A code-mode script's output covers all its commands, so it
confirms their reads but is not read as any one command's diagnostics.
@github-actions

Copy link
Copy Markdown

Code Erosion Report

Informational — never blocks the merge. scb-check==0.2.0 SlopCodeBench metrics; method & caveats in docs/dev/ci-code-erosion.md.

Metric Value Human band Agent band Reading
Verbosity* 0.058 0.15 0.33 human
Erosion 0.707 0.31 0.68 agent
Cognitive erosion 0.875 – – –

Scanned 431 files / 97063 SLOC · high-CC functions 760/8524

* On TypeScript, verbosity is partial and the bands are Python-calibrated.

Rule hits (TS verbosity layer)

Standalone ast-grep, separate from the number above.

Rule Hits
unnecessary-else-after-return 61
empty-catch-block 5

66 distinct lines flagged across src/.

@github-actions

Copy link
Copy Markdown

No findings in the specified diff.

  • The PR description documents build verification and representative real-CLI runs at c6011a0a, satisfying the testing requirement.
  • The earlier code-erosion report is informational, not a blocking finding.
  • Review was read-only; I did not run, build, or install PR code.

@jeff-r2026 jeff-r2026 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified on top of main fb17c48 merged together with #1357: tsc, lint, build pass; skill-use / session-scoring / shell-command / hook tests 157/157; e2e score-session, score-external-driver, workflow-records-reports-branch-1083 4/4 with the built CLI.

@jeff-r2026
jeff-r2026 merged commit 1e4cec7 into main Oct 24, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants