Skip to content

Add the bisect repair patch agent skill - #171

Open
rameziophobia wants to merge 2 commits into
ramez/repair-patch-managementfrom
ramez/bisect-repair-skill
Open

Add the bisect repair patch agent skill#171
rameziophobia wants to merge 2 commits into
ramez/repair-patch-managementfrom
ramez/bisect-repair-skill

Conversation

@rameziophobia

Copy link
Copy Markdown
Contributor

Summary

  • add an agent skill for creating and managing bisect repair patches
  • route the workflow through the supported top-level bisect patch commands
  • document verification and history-aware repair behavior

Stack

Depends on the repair-patch management PR. This is the final PR in the bisect repair stack.

Validation

  • workspace build and typecheck passed
  • shaka-perf test suite passed

@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review

This PR is documentation-only (SKILL.md + evals.json, no code changes), so the usual bug/security/performance concerns mostly don't apply. I cross-checked every command, flag, and behavioral claim in the skill doc against the actual implementation (patch-cli.ts, patch-capture.ts, patch-registry.ts) that this stacked PR depends on:

  • All shaka-perf bisect patch <subcommand> invocations and flags (create, edit, update, apply, verify, list, show, remove, --working-tree, --source-commit, --patch-file, --parent, --root, --all-files, --all/--from/--through/--at selectors, --prepare-command/--cleanup-command, --keep-file, --investigate-merges, --check/-R) match the Commander definitions in patch-cli.ts.
  • Claims about capture behavior ("temporary Git index," "source file remains untouched," "immutable source and parent SHAs," root-commit handling via --root) match patch-capture.ts.
  • Claims about verify ("disposable detached worktrees," apply-and-reverse-cleanly-or-already-native) match patch-registry.ts's verifyAtCommit.
  • "Manual apply does not run preparation or cleanup commands" and "Update never changes patch bytes" match the registry's apply/updateMetadata implementations (no prep/cleanup invocation, no capture call).
  • "Do not add bisect.repairs to abtests.config.ts" matches the explicit rejection in config.ts (bisect.repairs is not supported...).
  • "Patch-management commands do not create Git commits" is accurate — no git commit call anywhere in the patch CLI/registry/capture code.

No inaccuracies, internal contradictions, or unsafe guidance found. One very minor nit: the frontmatter adds a compatibility: field that no other skill in .claude/skills/ or .agents/skills/ uses (they all stick to name/description) — harmless, but worth a quick check that it's actually consumed by the skill-loading tooling rather than being inert extra metadata.

Nice, thorough doc — the eval prompts in evals.json also line up correctly with the selector/source flags they're meant to exercise.

@rameziophobia
rameziophobia force-pushed the ramez/bisect-repair-skill branch from 753be4e to ab22b41 Compare August 19, 2026 13:19
@rameziophobia
rameziophobia force-pushed the ramez/bisect-repair-skill branch from ab22b41 to 0d83d41 Compare August 19, 2026 13:20
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown

Reviewed this PR (docs-only: adds the create-bisect-repair-patch agent skill + evals).

I cross-checked every documented CLI command, flag, and behavioral claim in SKILL.md against the actual implementation in packages/shaka-perf/src/compare/bisect/patch-cli.ts, patch-registry.ts, and patch-capture.ts:

  • All subcommands (create, edit, update, apply, verify, list, show, remove) and their flags match the Commander definitions exactly (--working-tree/--source-commit/--patch-file source selection, --parent/--root, --all-files, --all/--from+--through/--at selector exclusivity, --prepare-command/--cleanup-command, --check/--reverse, --keep-file).
  • "Update never changes patch bytes" / "edit replaces bytes only" matches runUpdate vs runEdit.
  • "Manual apply does not run preparation or cleanup commands" matches apply() in patch-registry.ts.
  • "Verification uses disposable detached worktrees" matches the git worktree add --detach + cleanup logic.
  • "Capture uses a temporary Git index... leaves the real index unchanged" matches captureWorkingTreePatch's GIT_INDEX_FILE temp-file approach.
  • "--through alone starts at the session good SHA" matches the goodSha fallback in verify().
  • The bisect.repairs is not supported / manifest-only guidance matches the validation error in config.ts.

evals/evals.json is valid JSON and follows the schema used by the skill-creator skill. Frontmatter (name, description, compatibility) matches the documented optional fields.

No bugs, security issues, or inaccuracies found — the skill faithfully documents the shipped CLI surface. LGTM.

---
name: create-bisect-repair-patch
description: Create, inspect, edit, verify, apply, or remove managed compatibility patches for `shaka-perf bisect`. Use whenever historical candidates need frozen AB tests, build shims, seed-data setup, or other repairs, or whenever a user asks how to manage `bisect-repairs/manifest.json`. Always use the packaged `shaka-perf bisect patch` CLI; `bisect.repairs` and repository-local patch helpers are not supported.
compatibility: Requires Git, Node.js, and a shaka-perf project with twinServers configured.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This skill (and evals/evals.json) is placed under .agents/skills/, but that directory doesn't exist anywhere else in the repo and isn't a recognized skill location. Every other skill lives under .claude/skills/ — that's where Claude Code actually discovers skills, and it's the source-of-truth directory that packages/shaka-perf/scripts/copy-assets.mjs copies into the npm package (see its assets array, which explicitly lists each .claude/skills/<name>dist/skills/<name> pair). docs/for-ai-agents.md also documents skills as living at .claude/skills/ ("Four Claude Code skills under .claude/skills/ (they ship inside the npm package)"). .agents/ in this repo is reserved for CI/agent-workflow tooling (.agents/bin/*, .agents/agent-workflow.yml), unrelated to Claude Code skills.

As written, this skill won't be auto-triggered in this repo (Claude Code won't scan .agents/skills/) and won't ship to consumer projects via shaka-perf init, since it's missing from copy-assets.mjs's assets list and docs/for-ai-agents.md's skill table.

Suggest moving both files to .claude/skills/create-bisect-repair-patch/ and adding the corresponding entry to copy-assets.mjs (and ideally the skills table in docs/for-ai-agents.md).

@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review summary

This PR adds a new agent skill (create-bisect-repair-patch/SKILL.md + evals/evals.json) documenting how to drive the shaka-perf bisect patch CLI. I cross-checked the documented commands, flags, and behavior (selectors, capture semantics, verification via disposable worktrees, resume/snapshot behavior) against the actual implementation on the base branch (patch-cli.ts, patch-registry.ts, patch-capture.ts, README-compare-bisect.md) — the content is accurate and consistent with the real CLI.

One significant issue (left as an inline comment): the skill files are placed under .agents/skills/create-bisect-repair-patch/ instead of .claude/skills/create-bisect-repair-patch/. Every other skill in the repo lives under .claude/skills/, which is the directory Claude Code actually scans for skills and the one packages/shaka-perf/scripts/copy-assets.mjs copies into the published npm package. .agents/ is a separate, pre-existing directory reserved for CI/agent-workflow tooling (.agents/bin/*, .agents/agent-workflow.yml) and has no skills convention. As written, this skill won't be discovered in this repo and won't ship to consumer projects via shaka-perf init.

No security or performance concerns — this PR only adds documentation/markdown and a JSON eval fixture, no executable code.

---
name: create-bisect-repair-patch
description: Create, inspect, edit, verify, apply, or remove managed compatibility patches for `shaka-perf bisect`. Use whenever historical candidates need frozen AB tests, build shims, seed-data setup, or other repairs, or whenever a user asks how to manage `bisect-repairs/manifest.json`. Always use the packaged `shaka-perf bisect patch` CLI; `bisect.repairs` and repository-local patch helpers are not supported.
compatibility: Requires Git, Node.js, and a shaka-perf project with twinServers configured.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This skill is placed under .agents/skills/create-bisect-repair-patch/, but every other skill in this repo — and Claude Code's skill loader itself — lives under .claude/skills/ (see ab-servers, assess-abtest-quality, review-architecture, skill-creator, etc., all at .claude/skills/<name>/SKILL.md). .agents/ here is the machine-generated shakacode/agent-workflows seam directory (.agents/bin/*, agent-workflow.yml) — it has no concept of "skills" and isn't scanned for them.

Concretely, this means the skill as shipped won't be discovered/loaded by Claude Code at all: it's absent from the live skill listing in this very review session (only the 9 .claude/skills/* skills show up). Repo docs reinforce the expected location too — SETUP-twin-servers.md says shaka-perf init "drop[s]... two Claude Code skills under .claude/skills/", and skill-creator's own schema doc (.claude/skills/skill-creator/references/schemas.md) says evals/evals.json is "Located at evals/evals.json within the skill directory" — i.e. under .claude/skills/<name>/, exactly where this content should live.

The SKILL.md/evals.json content itself checks out well against the implemented bisect patch CLI (patch-cli.ts, patch-registry.ts, patch-capture.ts, repair-artifacts.ts) — flags, selectors, verify/apply semantics, manifest snapshot-on-resume behavior, and the bisect.repairs deprecation message all match. This looks like the whole tree should move to .claude/skills/create-bisect-repair-patch/ before merge.

@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review summary

This PR adds a Claude Code skill (SKILL.md) plus evals/evals.json documenting how to drive the shaka-perf bisect patch CLI (create/list/show/update/edit/apply/verify/remove). I cross-checked every command, flag, and behavioral claim in the skill against the actual implementation (patch-cli.ts, patch-registry.ts, patch-capture.ts, repair-artifacts.ts, config.ts) — the content is accurate: source selection (--working-tree/--source-commit/--patch-file), selectors (--all/--from+--through/--at), the temp-index capture isolation, disposable-worktree verification, apply not running prepare/cleanup, update never changing bytes, the bisect-repairs/manifest.json default path, the bisect.repairs deprecation, and the resume-uses-snapshot behavior all match the code.

One blocking issue (left as an inline comment): the skill is added at .agents/skills/create-bisect-repair-patch/, but this repo's Claude Code skills all live at .claude/skills/<name>/ — that's what the skill loader scans, what the repo's own docs point to (SETUP-twin-servers.md), and where skill-creator's own schema doc says evals/evals.json belongs. As shipped, this skill won't be discovered by Claude Code (confirmed empirically — it doesn't appear in this session's live skill listing, only the 9 .claude/skills/* skills do). The tree likely needs to move to .claude/skills/create-bisect-repair-patch/ before this is usable.

No security concerns (docs-only change, no code/CLI behavior modified) and no performance implications.

@rameziophobia
rameziophobia force-pushed the ramez/bisect-repair-skill branch from 0d83d41 to 090a3e7 Compare August 19, 2026 13:24
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown

Reviewed. This PR only adds a new agent skill (.agents/skills/create-bisect-repair-patch/SKILL.md) and its eval suite — no runtime code changes.

I cross-checked every command, flag, and behavioral claim in the SKILL.md against the already-implemented shaka-perf bisect patch CLI (patch-cli.ts, patch-capture.ts, patch-manifest.ts):

  • All subcommands (create, verify, list, show, update, edit, apply, remove) and their flags match the actual Command/Option definitions, including --kind choices (test-harness, build, data, other), the mutually-exclusive selector rule (--all / --from+--through / repeated --at), and --through defaulting to the session good SHA.
  • The "capture uses a temporary Git index and leaves the real index/checkout unchanged" claim matches patch-capture.ts's use of a scratch GIT_INDEX_FILE.
  • The "patch kind is reporting metadata only, never kind-specific runtime behavior" note holds — no switch/kind-based dispatch found in the patch pipeline, consistent with this repo's architecture-review rule against name-keyed dispatch in shared modules.
  • --patch-file import correctly can't take pathspecs or --parent/--root/--all-files, matching the CLI's validation errors.

No bugs, inconsistencies, or security concerns found. Nothing to flag inline.

@rameziophobia
rameziophobia force-pushed the ramez/bisect-repair-skill branch from 090a3e7 to 211e189 Compare August 19, 2026 15:19
@@ -0,0 +1,163 @@
---

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This skill is placed under .agents/skills/, but every other skill in this repo (ab-servers, discover-abtests, setup-docker-servers-for-ab-tests, assess-abtest-quality, troubleshoot-abtest, review-architecture, frontend-design, react-patterns, skill-creator) lives under .claude/skills/. This is the first skill ever added under .agents/skills/ — that directory (from agent-workflow-seam-doctor --init) holds cross-tool CI/workflow config (agent-workflow.yml, bin/setup|build|test|validate), not Claude Code skills.

Confirmed effect: this session's available-skills list (populated from the repo's actual skill directories) does not include create-bisect-repair-patch, so Claude Code cannot discover/invoke it via the Skill tool as written. If the intent is for this to be a normal Claude Code skill (its content — frontmatter shape, references to shaka-perf bisect patch ..., eval format matching skill-creator's evals/evals.json schema — strongly suggests that), it should move to .claude/skills/create-bisect-repair-patch/.

@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review summary

This PR adds a documentation-only skill (SKILL.md + evals/evals.json), no runtime code. I cross-checked every documented shaka-perf bisect patch command, flag, and behavioral claim against the actual implementation on the depended-on branch (ramez/repair-patch-management: patch-cli.ts, patch-registry.ts, patch-capture.ts, repair-runtime.ts, state.ts, config.ts) and it is accurate and well cross-checked — command names, flags, mutual-exclusivity rules (source, selector), the "temporary Git index leaves the real index untouched" claim, the "verification uses disposable detached worktrees" claim, the "manual apply doesn't run prepare/cleanup" claim, the "kind is reporting metadata only" claim, and the "resume uses the immutable manifest snapshot" claim all match the code. The evals/evals.json structure also matches the schema documented in skill-creator's references/schemas.md exactly.

One real issue (posted inline): the skill is added under .agents/skills/create-bisect-repair-patch/, but every other skill in this repo lives under .claude/skills/. .agents/ in this repo is the agent-workflow-seam-doctor-generated cross-tool CI directory, not a skill root. As a result, create-bisect-repair-patch does not appear in this session's discovered skill list — Claude Code can't invoke it from where it's placed. This looks like an incorrect target directory rather than an intentional convention choice, since nothing else about the file (frontmatter shape, eval format) suggests it's meant for a different tool.

No security or performance concerns — this PR ships no executable code.

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