[007Ghz]

Summary

Codex can load a Skill by reading its SKILL.md through the Bash tool, but common compound shell commands were not being recorded. This change adds a conservative Codex reader for foreground command lists and credits only installed Skills whose reads are provably reached.

AI Review Follow-up

• P1: Reader command errors are now tracked even when residual help text remains. An unattributed reader error rejects that read regardless of the final command-list status, so a trailing true cannot mask a failed cat --invalid-option <SKILL.md>. Exact failed operands are still excluded individually, while unrelated earlier shell errors do not hide a successful read.
• P2: The syntax scanner treats backslash-newline as a shell continuation without counting it as a command token. It rejects the resulting empty segment, including true; \\\n; cat <SKILL.md>.

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

• npm run build passes
• npx tsc --noEmit passes
• npm run lint passes
• Added/updated tests for the change
• npx vitest run src/__tests__/tool-call.test.ts — 161 passed.
• npm run test:e2e -- src/__tests__/e2e/codex-compound-skill-use.test.ts — passed with the built CLI. It checks successful reads, explicit and masked reader failures, repeated separators, escaped newlines, and no false event for rejected commands.
• The most recent full-suite run was on f8e7d82, before this follow-up: 474 files passed, 7 failed, 1 skipped; 11,272 tests passed, 13 failed, 144 skipped. Two submodule-checkout.test.ts failures reproduce alone because hook.teamai-post-checkout.command is absent; the other failures were not individually isolated. The latest follow-up was verified with the full affected unit test file and the real CLI E2E test above.

Related Issues

None known.

Notes for Reviewers

• The compound reader is called only from Codex Bash Skill-use handling when the shared classifier did not already return a read.
• Supported command lists are foreground and flat, using ;, newline, &&, and ||; unsupported shell syntax, dynamic operands, redirects, pipelines, background execution, and state-changing commands are rejected.
• Unknown command outcomes are propagated conservatively. A read is credited only when every feasible trace consistent with the event reaches it.
• Reader diagnostics must be attributable to exact operands. Merged diagnostics across multiple reader commands are ignored when attribution is ambiguous.

[github-actions]

• [P1 blocking] Reader help text can credit a failed read — src/utils/tool-call.ts:456: For true; cat --invalid-option <installed-SKILL.md>, GNU cat fails before reading the file but prints Try 'cat --help' for more information. After filtering the error line, that help text satisfies outputBelongsToReader, so the skill is credited even with tool_status: 'failure'. Remaining reader output is not necessarily file content; reject unattributed reader failures.

• [P2 non-blocking] Invalid separators can credit a command that never ran — src/utils/tool-call.ts:302: The syntax guard accepts ;;, which the tokenizer collapses into an ordinary separator. With echo before\ntrue;; cat <installed-SKILL.md>, Bash prints before, then reports a syntax error without running cat. For Codex’s plain-string response (unknown status), filtering the shell diagnostics leaves before, and the helper credits the skill. Reject invalid empty commands instead of trusting the tokenizer’s collapsed list.

Testing: The PR description satisfies the testing requirement: it records a successful build followed by representative built-CLI hook verification, including a negative case. I did not run, build, or install anything from the PR.

[github-actions]

• [P1 blocking] Masked reader failures still credit unread skills — src/utils/tool-call.ts:455: For true; cat --invalid-option <installed-SKILL.md>; true, cat reads nothing, but the final command makes the call successful. Filtering the error leaves Try 'cat --help' for more information., and the helper credits the skill. Codex’s unknown-status response also bypasses this check. Reject unattributed reader failures regardless of the final list status.

• [P2 non-blocking] Escaped newlines bypass the empty-command guard — src/utils/tool-call.ts:315: In echo before\ntrue; \\\n; cat <installed-SKILL.md>, backslash-newline contributes no shell token, so Bash reports a syntax error and never runs cat. The guard nevertheless sets segmentHasToken, while the tokenizer collapses the empty command. With unknown status, the remaining before output causes the skill to be credited.

Earlier findings: Both previously reported exact cases are fixed; the variants above remain.

Testing: The PR description satisfies the requirement with a successful build followed by representative built-CLI hook verification, including a negative case. I only read the diff; I did not run, build, or install anything from the PR.

[github-actions]

No new findings I’m confident are real in the current diff.

Earlier findings: Resolved. Unattributed reader failures are rejected even when a later command masks the failure; repeated separators and escaped-newline empty commands are also rejected.

Testing: The PR description satisfies the requirement: it records a successful build and representative built-CLI E2E verification, covering successful, failed, and malformed reads. Full-suite failures are disclosed.

I only read the specified diff; I did not run, build, or install anything from the PR.

[SaulMoro]

Thanks for finding this. The gap is real. Closing it as superseded by #1262, which fixes the same gap for every tool. Both commits in #1262 credit you as co-author (Co-authored-by: 007Ghz), so you keep the credit for the finding when it merges.

Why a separate PR instead of changes here:

• It is not Codex-only. classifyShellCommand returned plain shell for any command list, and classifyToolCall uses it for every tool's shell call: Claude Bash, Cursor Shell, Pi and OMP bash, CodeBuddy, OpenCode, Copilot. On main, pwd; cat …/SKILL.md records nothing for Claude either. A tool !== 'codex' branch also cuts across [feat] One descriptor per tool, and one resolver for where teamai writes and where the tool reads #1221, which made skill detection per-tool data.

• The shape. The fix belongs in the shared classifier. Each pipeline of a list is classified with the tokenizer and classifySimple that already exist. That needs no second character-level scanner (hasUnsupportedCompoundSyntax), no exit-status tracer (mustReachCommand) and no list of 40 rejected words. That list also rejected cd dir && cat …, which is probably the most common form.

• What carried over from this PR. Two cases you guarded against are handled in fix(shell): count the reads in a shell command list #1262 with simpler rules:

  • A read after || credits nothing.
  • A read that an error line names does not count, now for mixed readers too.

  The cwd guard there fails closed: after any command that may change the directory, only absolute paths count.

The new PR replays real Codex payloads through the built CLI and has a test table over ;, newline, &&, ||, pipelines and cwd changes. Review is welcome there.

[SaulMoro]

@007Ghz thanks again for finding this. You are credited as co-author on both commits of #1262 (Co-authored-by: 007Ghz), so the credit stays with you when it merges.

[007Ghz]

@SaulMoro Thanks for the update and for carrying the fix forward in #1262. I appreciate the co-author credit.