fix(skill-creator): isolate trigger evals and handle Windows and runtime failures - #1298

Brutal diagnosis — four independent causes each sufficient for 0% recall, which explains why partial fixes didn't move the number. The "optimizing against noise" framing is exactly right: a metric that always reads zero might as well not exist.

The cross-platform testing rigor (Windows stream handling, executable resolution) and the systematic verification strategy (stub harness + offline tests before real integration) are both production patterns we recognize. A few observations:

On the artifact installation strategy: The decision to install as .claude/skills/<name>-eval-<id>/SKILL.md instead of swapping the real skill in-place is the safer choice. The tradeoff you note (suffix on the skill name) is acceptable because routing happens on description content, not the name. One risk to flag: if the model's routing logic includes edit distance on skill names as a tie-breaker (some frameworks do this), the suffix could deflate trigger rates slightly. Have you verified that Claude Code's skill picker ignores name similarity entirely?

On cause 3 (exploration counted as miss): The fix — "non-Skill tools are ignored, only terminal events decide not-triggered" — is correct for auto-triggering, but it changes what you're measuring. Before: "did the skill trigger first?" After: "did the skill trigger eventually?" The latter is the right metric for description optimization (you want the model to reach the skill at all), but if you're also evaluating routing efficiency (how many irrelevant tools were used before the right one), you'd want to track that separately. Consider logging steps_to_trigger as optional metadata.

On cause 4 (parallel worker race): The shared artifact solution is clean, but there's a hidden complexity: if multiple runs execute concurrently (e.g., CI pipeline with parallel jobs), they'll still collide on the artifact name. The -eval-<id> suffix mitigates this if <id> is globally unique (UUID), but if it's process-scoped (PID), you can get cross-process races. Suggest documenting the <id> uniqueness guarantee — or add a lockfile if not already present.

On the verification strategy: The offline stub harness is excellent (we use this pattern too — "canned stream-json" for zero-cost iteration). One gap: your offline tests cover Windows stream handling and parallel workers, but they don't cover the actual trigger detection logic against malformed/truncated tool_use events. Consider adding a fuzzer that injects partial JSON chunks mid-event to verify your detection doesn't false-positive.

Minor: The PR description says "Tradeoff kept deliberately: the eval skill's name carries an -eval- suffix while production skills don't." But you also say "the suffix avoids collisions with installed skills." These are compatible, but clarify: does the suffix prevent shadowing (good), or does it introduce name-mismatch risk (potential concern)? The warning about shadowing suggests you're aware of both sides.

Last: The "sweep stale <name>-eval-* artifacts" logic is defensive (we do this too). What's the age threshold? 2 hours? 24 hours? If it's too aggressive, you'll delete artifacts from paused debugging sessions. If too permissive, you'll accumulate cruft. Consider making it configurable or at least documenting the default.

Eval loop signal integrity recognized via SwarmAI. Discussion: T-MEM

Pushed 79b3df9 delivering the refinements discussed above:

1. Sandboxed eval environment (name-mismatch + shadowing): went one step further than promised. Each run now creates an isolated throwaway project under the OS temp dir (<tmp>/claude-skill-evals/<skill>-<uuid4>/.claude/skills/<real-name>/SKILL.md) and claude -p executes with that project as its working directory. The skill keeps its real name -- isolation comes from the project directory, not from a name suffix -- so the edit-distance question is moot by construction, and the caller's project is never touched by eval artifacts at all. On your original question: empirically the suffixed artifact was already triggering 2/2 on matching queries in the Windows e2e (routing follows the description), but removing the suffix removes the question entirely.

2. Configurable sweep threshold: now --stale-artifact-hours, defaulting to 12h as proposed. One correction to my earlier reply: the previous hardcoded window was 1 hour, not 2. The sweep also only ever operates inside the dedicated temp root now, so it cannot touch anything outside it.

3. Cross-run collisions: to be precise, the previous id was already UUID-derived (a uuid4 hex prefix), not process-scoped -- but the new layout makes the guarantee structural rather than probabilistic: a full 32-hex UUID4 keys a dedicated per-run project directory, so concurrent runs (parallel CI jobs included) cannot collide regardless of id entropy. No lockfile needed.

On steps_to_trigger: agreed -- that is the right metric for routing efficiency as opposed to routing reach. Kept out of this PR to keep the output schema stable for run_loop.py/improve_description.py, but it is a clean additive follow-up.

On fuzzing the detection against malformed/truncated tool_use events: the streaming detector accumulates input_json_delta fragments into a buffer and substring-matches on the accumulated state, so split/partial chunks are handled by construction, and malformed JSON lines are skipped. I would like to land that as in-repo tests, but the scripts currently have no test scaffolding -- verification lives in an external stub harness (canned stream-json, zero API cost) for now. Happy to contribute test scaffolding as a separate PR if maintainers want it.

Re-verified end to end on Windows 11 + Claude Code 2.1.149 after the change: 2/2 should-trigger, 0/2 should-not-trigger, zero leftover directories in temp.