test: skill event emission tests

[byapparov]

Summary

• Add tests verifying progressive skill loading events fire correctly
• Covers all 3 event types: skill_discovered, skill_loaded, skill_resource_loaded

Tests

Test	What it verifies
SkillDiscovered emitted per skill on init	Each skill gets a discovery event with name, description, location
SkillDiscovered not emitted without sessionID	No events when init called without sessionID
SkillLoaded emitted on execute	Event fires when model invokes skill tool
SkillResourceLoaded for skill dir files	Event fires when reading a file inside a skill directory
SkillResourceLoaded not emitted for other files	No event for files outside skill directories
No events when no skills exist	Clean behavior with empty skill set

Test plan

• All 6 tests pass locally (bun test test/tool/skill-events.test.ts)
• Existing skill tests unaffected

[github-actions]

PR Review Summary

This PR establishes the planning infrastructure for a CLI hardening project. It adds comprehensive documentation and one test file.

Changes

• 15 new documentation files in .planning/ - project roadmap, requirements, architecture analysis, and execution plans for 31 findings from a 5-engineer code review
• 1 new test file - packages/cli/test/tool/skill-events.test.ts (285 lines)

Findings

Test File Issues (Minor)

1. Type Safety (skill-events.test.ts:111, 240, 269): Uses any[] for event arrays. Should be typed as:

   constevents: Array<{name: string; ... }>=[]

   This is a style issue, not a bug.

Planning Documentation

The execution plans (01-01, 01-02) propose sound fixes:

• Plan 01-01: Correctly identifies the fire-and-forget .catch(() => {}) bug and proposes a Promise.race pattern to handle early prompt failures
• Plan 01-02: Correctly adds process.exit(1) to exception handlers

Security & Reliability

• No security vulnerabilities in the changes
• No bugs or logic errors in the test file
• The documented concerns in .planning/codebase/CONCERNS.md accurately describe existing issues (symlink traversal, credential mutation, process cleanup) that are correctly scoped as future work

Recommendation

Approve - This is a well-structured planning PR. The minor type safety issues in the test file are style nits that don't affect functionality.

---

Reviewed SHA: b4d8379

[github-actions]

Review

This PR adds comprehensive test coverage for skill event emissions. I reviewed the test file for bugs, security issues, logic errors, and reliability concerns.

Findings: No issues found. The tests are well-structured with:

• Proper isolation using temporary directories and environment variable cleanup
• Correct event subscription patterns (subscribe before operation, unsubscribe after)
• Good coverage of both positive and negative cases
• Appropriate use of Instance.provide() for context management

Tests verify:

• SkillDiscovered events emit per skill on init (with sessionID)
• No events emit when init called without sessionID
• SkillLoaded fires when skill tool executes
• SkillResourceLoaded fires when reading files inside skill directories
• SkillResourceLoaded does not fire for files outside skill directories
• Clean behavior when no skills exist

The test coverage looks solid and the implementation patterns are consistent across all test cases.

---

Reviewed SHA: 147b6ab