Skip to content

Optimize the skill + extract resolve-session to CLI#5

Merged
narghev merged 2 commits into
mainfrom
optimize-the-skill
May 10, 2026
Merged

Optimize the skill + extract resolve-session to CLI#5
narghev merged 2 commits into
mainfrom
optimize-the-skill

Conversation

@narghev
Copy link
Copy Markdown
Owner

@narghev narghev commented May 10, 2026

Summary

Two phases of work, two commits:

Phase 1 — 07b78a4 Simplify the skill: ~30% prose reduction on both askdiff/SKILL.md and askdiff-dev/SKILL.md (Steps 1–4 kept in sync). No behavior change. Drops dead branches (e.g. the (none …) troubleshooting paragraph for a code path that now throws), collapses 13-line footgun blocks into 3-line inline notes, removes "tell the user" enumerations that just restate what's already in the bash output.

Phase 2 — f18342c Extract keyword session search to resolve-session CLI subcommand: moves Step 4c's inline mktemp/find/grep -cFf bash into a typed CLI subcommand (packages/cli/src/resolve-session.ts), keeping the public-facing skill behavior identical but making the search testable and shell-safe.

Bugs found and fixed during testing

  • Commander v14 --cwd overlap: parent and subcommand both declared -c, --cwd, so commander silently routed the flag to the parent — the subcommand always saw process.cwd(). Read from program.opts() instead.
  • Step 4b prefix-glob in zsh: existing skill bash used shopt -s nullglob (bash-only) and ${matches[0]} (zsh is 1-indexed, so empty). Replaced with find + while read + for "${matches[@]}" — works in both shells.
  • Dev-skill tsx invocation: the CLI's static import { PROTOCOL_VERSION } from "@askdiff/protocol" failed under ESM because the protocol package is CJS. Lazy-loaded inside runServer so resolve-session never triggers protocol load.
  • Skill prose regression: I had said "pass --diff-file only for description-based diffs" — original bash always extracted paths. Reverted to "always pass; omit --sha/--branch for working-tree."

Test plan

  • pnpm lint clean
  • pnpm test 111 passing (was 89; +17 unit, +5 integration including a tsx-path regression guard)
  • pnpm run build clean (CLI bundle 32K, skill.md with both ASKDIFF_VERSION lines pinned)
  • Smoke tests against real ~/.claude-personal/projects/<this>/*.jsonl for 9 flag combinations
  • Edge cases on tmpdir fixtures: missing dir, empty needles, special chars in keywords
  • Built CLI: node packages/cli/dist/index.js resolve-session … works
  • Dev path: pnpm --filter askdiff exec tsx src/index.ts resolve-session … works
  • Walked 9 natural-language inputs through Steps 1–4 manually (working tree, last commit, branch comparison, vague description w/ commit-ladder, explicit prefix in zsh after fix, full UUID, keyword + working-tree, etc.)
  • End-to-end skill invocation is intentionally not run from this branch (would open a browser tab); reviewer can validate by running /askdiff and /askdiff-dev after merge.

Stats

before after
askdiff/SKILL.md 493 lines 406 lines (-18%)
askdiff-dev/SKILL.md 496 lines 425 lines (-14%)
tests 89 111
50-line mktemp/find/grep bash inline extracted to resolve-session.ts

Suggested release

v0.4.0 (minor) — adds public CLI subcommand resolve-session, includes two user-facing bug fixes for the /askdiff session <prefix> and /askdiff-dev keyword-search paths.

🤖 Generated with Claude Code

narghev and others added 2 commits May 9, 2026 11:04
Adds `askdiff resolve-session` (typed options, JSON output, fixture
tests) and replaces the inline find/grep bash that motivated the
"Do not simplify" footgun list.

Also fixes two pre-existing bugs found during testing:
- Commander v14 routes `--cwd` to the parent program when both parent
  and subcommand declare it, so the resolve-session subcommand always
  saw the default. Read cwd from `program.opts()` instead.
- Step 4b prefix-glob used `shopt -s nullglob` (bash-only, silently
  fails in zsh) and `${matches[0]}` (zsh arrays are 1-indexed). Use
  `find` + `while read` + iteration via `"${matches[@]}"`.

Lazy-loads `@askdiff/protocol`/`@askdiff/server` inside `runServer` so
`tsx src/index.ts resolve-session …` works in dev (the protocol
package is CJS and ESM-static-importing it failed under tsx).

Tests: 89 → 111 (+17 unit, +5 CLI integration including a tsx-path
regression guard). Lint clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@narghev narghev merged commit d2e27c3 into main May 10, 2026
1 check passed
@narghev narghev deleted the optimize-the-skill branch May 10, 2026 11:44
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.

1 participant