[ousamabenyounes]

Summary

A session judge currently sees neither the team skills delivered to the session nor the skills it loaded. This PR adds the evidence-only slice of #1469: captures retain each delivered skill's description and actual namespace, and evidence.json exposes skills.available and skills.used. The issue explicitly allows this slice to land before the blocked routing-scorer work in #1180.

The capture uses the member's saved delivery selection and checks the delivered skill against source files and the role manifest at that pull's revision. Thus, when ops/ship and web/ship exist, a role switch or a later role-manifest edit cannot relabel an unchanged web copy as ops. When two historical copies have identical content and the delivery record cannot distinguish them, the capture keeps its name and description with namespaceUnknown: true rather than assigning today’s namespace. An own-pull delivery with no skills is recorded as an empty inventory. Descriptions are masked before entering the evidence manifest. Older captures get the skills_unknown gap; a captured checkout with no team skills gets an empty available list. The scorer-file control now receives the session's captures too.

Type of change

• Feature (additive evidence fields; existing capture data stays readable)

Test plan

• npm run build and real CLI E2E: hook-dispatch session-start → score --session → fake judge reads evidence.json with the delivered skill
• npx tsc --noEmit
• Type-aware oxlint --deny-warnings on the changed files
• 106 focused unit tests and all five tests in src/__tests__/e2e/score-session.test.ts pass; the earlier coverage run covered 100% of 126 executable added lines before the final ambiguity guard
• Full suite at the final commit compared with the same upstream base in the same container: 52 identical pre-existing failures, zero new failures; 18,693 base passes versus 18,706 branch passes (+13 tests)

RED → GREEN evidence (selected test log lines)

RED — original upstream base, before the production change:
FAIL session-skill-evidence > lists delivered team skills
  expected manifest.skills to contain available and used; received undefined
FAIL session-skill-evidence > no descriptions recorded
  expected gaps to include skills_unknown
FAIL session-captures > records each delivered skill's description and namespace
  expected teamSkillInfo; received undefined
Test Files 2 failed; Tests 3 failed

RED — review regressions on the first candidate:
FAIL session-captures > same-name namespaces after a role switch without pull
  captured namespace came from the wrong role
FAIL session-captures > same-name namespaces after the role manifest changed
  captured namespace came from today's role mapping, not the delivered revision
FAIL session-captures > empty inventory after the session's own pull
  expected {}; received undefined
FAIL session-captures > identical copies with an ambiguous historical selection
  expected no attribution; received a skill under today's namespace

RED — final review regression:
FAIL session-captures > identical copies with uncertain provenance
  expected ship with namespaceUnknown: true; received an empty inventory
FAIL skill-evidence > preserves a delivered skill with uncertain provenance
  expected namespaceUnknown: true in evidence.json; field was absent

GREEN — final commit:
Test Files 1 passed; Tests 23 passed (capture regressions)
Test Files 6 passed; Tests 106 passed (focused unit tests)
E2E score-session: 5 tests passed, including the real hook → score flow
npx tsc --noEmit: exit 0
npm run build: success
Earlier changed-line coverage: 100% (126 executable added lines, 0 uncovered); final guard has a direct RED → GREEN regression

Full-suite baseline comparison

Upstream base: 52 failed | 18,693 passed | 154 skipped (18,899 total)
Final commit: 52 failed | 18,706 passed | 154 skipped (18,912 total)
Failure-name set difference: 0 new, 0 resolved.

The full-suite failures include tests requiring tools or process behavior absent in this container. For example, the seven env-exec failures reproduce unchanged when that file is run alone on both base and branch. The focused tests, typecheck, build and changed-file lint are green.

Real CLI E2E execution

The E2E harness invokes the built CLI (dist/index.js) with hook-dispatch session-start --tool claude, then score --session <fixture-id> --source claude-code --scorer review-quality. Its fake judge reads the generated evidence.json. The passing assertion checks skills.available contains the delivered review-notes skill with its description, skills.used is [], and skills_unknown is absent.

$ npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/score-session.test.ts
✓ passes delivered and used skills through a real session-start hook and score CLI (#1469)
Test Files  1 passed (1)
     Tests  5 passed (5)

Asserted evidence.json.skills:
{"available":[{"name":"review-notes","description":"Write review notes for a PR."}],"used":[]}

Related issue

Part of #1469. Routing questions, skill_routing settings, generated scorer names and documentation for those settings remain in the follow-up after #1180; this PR does not close #1469.

[github-actions]

• [P1 blocking] Missing feature-level real-CLI verification — PR description. The recorded CLI run is only node dist/cli.js --version; it does not exercise capturing skills or generating evidence. The Code Review Rules require end-to-end / real-CLI verification for runtime changes. Record one representative session flow and confirm its evidence.json contains the expected skills.

• [P2 non-blocking] Namespace resolution uses current repository state — src/skill-use.ts:321. The saved selection restores role IDs, but resolution reads today’s team configuration and role manifest. If that manifest switches a role from web to ops before this checkout pulls again, an unchanged delivered web/ship can be reported as ops/ship. Resolve against the delivered revision or retain namespace provenance in the delivery record.

• [P2 non-blocking] An empty post-pull inventory remains unknown — src/session-flow/captures.ts:170. When a capture starts without a delivery record and its own pull delivers no skills, withTeamSkillInfo discards the newly observed {}. Evidence therefore reports skills_unknown rather than an empty available list. Preserve the empty inventory when a valid delivery record was inspected.

[SaulMoro]

This was generated by AI during triage.

@ousamabenyounes Thanks, the feature works. With the built CLI on this head, I ran a team with both ops/ship and web/ship, a role switch with teamai roles set ops without a pull, then a session and its score. Against #1469:

• The capture keeps the web copy the session actually held.
• skills.used matches the label.
• A later pull leaves the captured description unchanged.
• Captures from main give the skills_unknown gap.
• A team with no skills gives available: [] and no gap.

tsc and lint pass. The three changed test files pass (28) and fail on main. CI is green at this head.

One thing blocks merge.

• [P1 blocking] No real-CLI record of the feature. The body's only built-CLI step is node dist/cli.js --version. "Testing before a PR" in AGENTS.md asks for an end-to-end run of the changed behavior. Please paste one built-CLI run: a session capture, then the scoring path, ending in an evidence.json with skills.available and skills.used. The CI review raised the same point.

Non-blocking:

• [P2 non-blocking] No delivery record at session start. A team with no skills, where the session starts before any delivery record exists, gets skills_unknown rather than available: []. #1469's scope sentence ("a capture that read the delivery record") supports this. The body and criterion 5 read looser, though, so please align one of them.
• [P2 non-blocking] Namespace from the clone's manifest. It is read from the team clone's current manifest (src/skill-use.ts:321), as the CI review says. I could not move the clone without this checkout's pull, so this is not reproduced. Can another workspace on the same machine advance the clone in between? If so, a delivered web/ship could be labelled ops.
• [P2 non-blocking] Unquoted colon in a description. A description such as description: Cut an npm release: bump, changelog, tag, publish. is recorded as "" with no gap. parseFrontmatter drops the valid flag that splitFrontmatter returns. A gap, or a note for the routing slice, would keep later routing from reading an empty description as real.
• [P2 non-blocking] Scorer guide. docs/guide/scorers.md (and zh-CN) do not mention the new skills key and skills_unknown gap that scorer files now see. Fine to defer to the routing slice if you say so.
• [P3 nit] deliveredTeamSkillInfo repeats the walk in deliveredTeamSkills (src/skill-use.ts). One walk would keep a single copy of the rule.

@jeff-r2026 Your call on this one. The evidence-only slice matches what #1469 allows before #1180's routing scorer.

[github-actions]

• [P1 blocking] Feature-level real-CLI verification is still undocumented — PR description. The only recorded CLI invocation is node dist/cli.js --version; the new session-start/score E2E test has no recorded passing run. The Code Review Rules require an end-to-end / real-CLI verification record for runtime changes. Record one representative session flow confirming the expected evidence.json skills.

• [P2 non-blocking] Namespace provenance still depends on today’s manifest for identical copies — src/skill-use.ts:354. If web/ship and ops/ship have identical SKILL.md contents, both historical hashes match. After the manifest redirects the saved role from web to ops, the fallback reports ops although the checkout last received web. The earlier namespace finding is only partially resolved; resolve selection at the delivered revision or retain delivery provenance.

Resolved: The earlier empty post-pull inventory finding is fixed: an observed delivery now preserves teamSkillInfo: {} rather than producing skills_unknown.

Review was read-only; no PR code was run, built, or installed.

[SaulMoro]

This was generated by AI during triage.

@ousamabenyounes Thanks, most of the last round is in. Checked again at d06896ef with the built CLI:

• Real-CLI record: resolved. The body now has a built-CLI run from session start to score, and CI's e2e shards pass at this head.
• No delivery record at session start: resolved. It now gives available: [] with no gap.
• Namespace from the clone's current manifest: partly resolved. With two namespaces holding ship, it now says web where the last head said ops. That holds when SKILL.md has both name: and description:. See the third point below for when it does not.

What the new commit adds:

• [P2 non-blocking, should be fixed here] Session start runs git show for every delivered skill. The loop in src/skill-use.ts (around line 350) reads each skill's source at the pull's revision with getFileContentAtRev, even for names that exist in only one namespace. The session-start hook is synchronous, so members wait for this on every session. With 150 delivered skills the hook went from 0.5–1.1 s to 1.4–2.5 s. The machine was under load, so only the ratio counts. Comparing hashes only for names found in more than one namespace keeps the fix and drops most of the cost. The same line also adds a non-null assertion, deliveredRev!, inside a block that already checked deliveredRev.
• [P2 non-blocking] A deleted skill shows in available but never in used. I reproduced it with a skill the team deleted after the member's last pull. It is still listed as available, but loading it in the session does not add it to used. The previous head left it out of both lists.
• [P2 non-blocking] The hash comparison misses skills that pull rewrites. When pull delivers a SKILL.md, it adds a missing name: (and description:), so the delivered file never hashes the same as the source. For those skills the namespace falls back to today's manifest. I reproduced this. It contradicts the body's provenance sentence, and the e2e fixture skill has no name:. Hashing the rewritten source, as isRecordedFromTeamSkill does, fixes it.
• [P3 nit] Identical copies in two namespaces still resolve through today's manifest, as the CI review says. I agree, but it needs an unlikely setup.

Still open from the last round: an unquoted colon in a description is recorded as "" with no gap, the scorers guide does not mention the new fields, and the skill walk is duplicated. The duplicated walk is what doubles the session-start cost above.

tsc, lint and the three changed test files (29) pass, and CI is green at this head.

@jeff-r2026 Your call. The session-start cost is the item I'd want fixed before this lands.

[github-actions]

• [P2 non-blocking] Ambiguous historical matches still use today’s namespace — src/skill-use.ts:366. When identical web/ship and ops/ship copies both belong to the saved selection’s historical namespaces, roleMatches contains two entries, leaving source undefined. The fallback then trusts today’s entry?.namespace; a manifest edit selecting only ops can still relabel an unchanged delivered web copy. Retain delivery provenance or resolve the winning source entirely at the delivered revision.

Resolved: The single-namespace historical-role scenario and empty post-pull inventory are fixed. The updated description documents a passing real hook → score flow that checks evidence.json, satisfying the required real-CLI testing record.

Review was read-only; no PR code was run, built, or installed.

[github-actions]

• [P2 non-blocking] Ambiguous namespaces erase a known available skill — src/skill-use.ts:371. With identical web/ship and ops/ship copies and both namespaces in the saved selection, this branch drops the unchanged delivered ship entirely. Evidence then reports available: [] without skills_unknown, although the skill and its description are known. Preserve the skill and explicitly represent uncertain namespace provenance rather than reporting an empty inventory.

Resolved: The prior namespace-relabeling and empty post-pull inventory findings are fixed. The description now records a passing real hook → score flow checking evidence.json, satisfying the required testing record.

Review was read-only and limited to the specified diff; no PR code was run, built, or installed.

[github-actions]

No new findings confidently established in the specified diff.

Resolved: The ambiguous-namespace case now preserves the delivered skill and description with namespaceUnknown: true. The earlier namespace-relabeling and empty-inventory findings remain resolved.

Testing: The PR description documents a passing real session-start hook → score CLI flow that checks evidence.json, satisfying the required real-CLI verification record.

Review was read-only and limited to the specified diff; no PR code was run, built, or installed.

[SaulMoro]

This was generated by AI during triage.

@ousamabenyounes Checked again at cad3302c. The new commits fix the CI review's point about identical copies: the built CLI now says web where d06896ef said ops. The points from my last comment are still open:

• [P2 non-blocking, should be fixed here] Session-start cost. Session start still runs one git show per delivered skill. With 150 delivered skills on an idle machine, the hook takes 0.96–1.07 s at this head, against 0.31–0.44 s at e87ad0bc, about 3x. Only names delivered from more than one namespace need the hash comparison.
• [P2 non-blocking] Missing name: line. Skills whose SKILL.md has no name: line still get today's namespace (ops). Pull adds name: on delivery, so the delivered file never hashes like the source. Hash the rewritten source, as isRecordedFromTeamSkill does. This also contradicts the body's provenance sentence.
• [P2 non-blocking] Deleted skill. A skill the team deleted is listed in available but never in used, even when the session loaded it.
• Still open from before: an unquoted colon in a description is recorded as "" with no gap, the scorers guide does not mention the new fields, and the skill walk is duplicated.

New in these commits:

• [P3 nit] A skill delivered through a project namespace now gets namespaceUnknown, where d06896ef gave the correct ops. The delivered namespaces are built from roles only, not roles plus projects.
• [P3 nit] There is a new non-null assertion, deliveredRoleNamespaces!, at src/skill-use.ts:370, and the line that reads the description is duplicated.

Build, tsc, lint and the four changed test files (68) pass, and CI is green at this head.

@jeff-r2026 Your call. The session-start cost is still the item I'd want fixed before this lands.