feat(optimize): /optimize skill — codebase-aware scan tuning + post-scan metrics refine loop (ENG-550)#58
feat(optimize): /optimize skill — codebase-aware scan tuning + post-scan metrics refine loop (ENG-550)#58d-co-white wants to merge 1 commit into
Conversation
|
Claude finished @d-co-white's task in 3m 54s —— View job Claude Code Review — Needs Attention
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| "description": "Optimization is expressed as a trial scan policy referenced via stackhawk.yml app.scanPolicy.name, NOT by mutating the app's own tech flags/plugins.", | ||
| "focus": "commands executed; stackhawk.yml diff" | ||
| }, | ||
| { |
There was a problem hiding this comment.
The description "Exactly one trial scan is run" contradicts SKILL.md step 7, which explicitly allows up to 3 re-scans in the refine loop ("If any change was applied, re-scan and return to step 6 with the new scan id").
A rubric grader counting hawk scan invocations and checking for exactly 1 will false-fail on any optimized run that triggered a concurrency reduction and needed re-validation.
Suggested fix:
"description": "One initial trial scan is run; up to 3 re-scans are allowed during the refine loop. After all refinement is done, the user is asked to promote or discard."
There was a problem hiding this comment.
Fixed in 0abb4e1. Reworded the check: "An initial trial scan runs; any further scans occur only as part of the tiered refine loop (capped at 3 iterations) to verify applied levers — not unbounded re-scanning. After refinement settles, the user is asked to promote or discard." It no longer contradicts SKILL.md step 7.
This comment has been minimized.
This comment has been minimized.
| ], | ||
| "anti_patterns": [ | ||
| "--name [a-z]" | ||
| ], |
There was a problem hiding this comment.
The grading engine (evals/lib/grading.py:47) uses plain substring matching (a in hay), not regex. The literal string --name [a-z] will never appear in any bash command output, so this anti-pattern is dead code — a lowercase policy name like --name optimize_trial_myapp will silently pass the check.
Fix: switch to explicit lowercase-prefix substrings, e.g. "--name optimize", "--name trial", or drop the anti-pattern and rely on the rubric grader for qualitative coverage.
There was a problem hiding this comment.
Fixed in 526cb9a. Removed the inert --name [a-z] anti-pattern (substring grading can't express the regex, so it never fired and gave false confidence) and dropped the overly-broad --name signal. Kept the meaningful policy create --name signal, and updated the description to reflect that the upper-snake (^[A-Z0-9_]+$) enforcement actually lives in hawkop's validate_policy_name (which rejects lowercase).
This comment has been minimized.
This comment has been minimized.
|
Addressed the latest review in
SKILL.md step 7 was aligned to match (cursor rule regenerated; idempotent). No version bump — review fixes on the open PR stay at 1.14.1. |
| }, | ||
| { | ||
| "id": "exclusion-confirmed-not-auto", | ||
| "phase": "any", |
There was a problem hiding this comment.
The exclusion-confirmed-not-auto check uses "type": "command_negative", which causes grading.py:46 to search only bash_only (the list of executed shell commands). But all four anti-patterns here are natural-language output phrases ("auto-applied excludePaths", "silently excluded", etc.) — they will appear in the agent's text output, not in bash command strings.
Result: this check is silent dead-code. It always reports PASS regardless of whether the agent actually auto-applies path exclusions, giving false confidence on the most critical safety rule of the refine loop.
Fix: change "type" to "output_negative" — the grader will then search the full haystack (output text + command trace) where these phrases actually appear:
"type": "output_negative",grading.py already handles output_negative (line 50: if ctype in ("command_negative", "file_content_negative", "output_negative")), so no other change is needed.
There was a problem hiding this comment.
Fixed in b645f1f — changed "type" to "output_negative". Confirmed against grading.py:46 (command_negative searches bash_only, but these anti-patterns are natural-language output phrases that never appear in a command string, so the check was silently always-passing). output_negative routes to the output haystack (line 50 supports it), so the auto-exclusion anti-patterns can now actually fire. Good catch — this was guarding the most safety-critical refine-loop rule.
…550) Rebuilds the optimize plugin + metrics-refine work on top of origin/main (which refactored skills to Anthropic best practices, added skill-authoring, the 600-line SKILL.md CI gate, and per-reference Cursor rules). Conformance to the refactor: - Per-reference Cursor rules: optimize SKILL.md + 4 references now each map to their own .mdc (stackhawk-optimize, -mapping, -trial, -cli, -metrics), matching the new hawkscan/api pattern. - SKILL.md body 92 lines (well under the 600 hard / 500 best-practice limit). - mapping.md no longer cross-links another skill's reference (skill-authoring guideline: references don't link references); points to SKILL.md Companion section. - No env-var interpolations, so the :none YAML convention does not apply. Preserves main's refactor (skill-authoring, hawkscan reorg, dependabot CI bumps) and re-applies optimize's additive edits to shared files (marketplaces, .version-bump.json, generate-cursor-rules.sh, evals/cli.py, skill-evals.yml matrices). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
b645f1f to
15a5fcc
Compare
| "type": "command_executed", | ||
| "signals": [ | ||
| "stackhawk.yml.optimize-bak", | ||
| "cp stackhawk.yml", |
There was a problem hiding this comment.
[warning] The "backup" signal is overly broad — any narration containing the word "backup" (e.g. "I will create a backup before editing") matches since command_executed searches the full haystack (output_text + bash_commands). This can give a false PASS in observe/narration mode without actually running the copy command.
The more specific signals "stackhawk.yml.optimize-bak" and "cp stackhawk.yml" are already present and sufficient. Consider removing "backup" — or replacing it with "optimize-bak" — to avoid spurious passes.
| "type": "command_executed", | ||
| "signals": [ | ||
| "hawkop scan metrics", | ||
| "scan metrics" |
There was a problem hiding this comment.
[warning] "scan metrics" is too broad a signal — it will match agent narration like "the scan metrics showed..." since command_executed uses the full haystack (output text + bash commands). The check can silently pass without hawkop scan metrics ever being executed.
"hawkop scan metrics" on line 48 is already a precise, sufficient signal. Consider removing "scan metrics" or replacing it with "hawkop scan" as a fallback, to avoid false passes in observe/narration mode.
| 2. Restore `stackhawk.yml` from `stackhawk.yml.optimize-bak`; remove the backup file. | ||
|
|
||
| ## Failure handling | ||
| Any error after step "Create 3" → run the full Discard sequence, then report the original |
There was a problem hiding this comment.
[suggestion] The ## Failure handling section covers errors during the Create sequence (after step "Create 3"), but the Promote sequence has no documented failure handling. If the permanent policy create (Promote step 2) fails, the user is left in a recoverable state (trial policy still lives, backup still exists), but an explicit note here would help agents and users know what to do — retry, or fall back to discard.
Something like: "If Promote step 2 fails (permanent policy create errors), the trial policy is still intact on the platform and the yml backup is unchanged — the user can retry Promote with a different name or run Discard."
|
2 participants
Summary
Adds the
optimizeagent skill (invocable as/optimize, discoverable across Claude/Codex/Gemini/Copilot/Cursor/OpenCode) plus its post-scan metrics refine loop. Bumps the repo to v1.14.1./optimizereads a codebase and produces an optimal HawkScan setup, applies it as a non-destructive trial (isolated org scan policy), runs a trial scan, analyzes per-path scan metrics, refines the config, and promotes or discards — with no residue.Part 1 — Pre-scan optimization (the
optimizeplugin)plugins/optimize/skill: SKILL.md + references (mapping,trial-lifecycle,cli-contract).stackhawk.ymlcorrections.hawkop policy create→ S3 hosted asset, referenced byapp.scanPolicy.name); promote = assign as default, discard = delete. App state untouched until promote.Part 2 — Post-scan metrics + tiered refine loop
references/metrics-and-refine.md; SKILL.md workflow gains phases 6–9.hawkop scan metrics <id> --format json(per-path/operation metrics + deterministic signal flags). The skill maps flags → concrete levers; it does not recompute metrics.hawk.scan.concurrentRequestsforrate-limited/timeout-prone); confirm coverage-reducing/input-needing levers (app.excludePathsforheavy-path/slow-path;app.authentication.*forauth-wall). Re-scan to verify; cap 3 iterations; concurrency floored at 1.stackhawk.yml(backed up); discard reverts everything.Dependencies (runtime, gated by preflight)
The skill orchestrates
hawkopcommands that ship separately:hawkop policy get/create/delete/assign— hawkop PR chore(deps): bump actions/checkout from 4 to 7 #41 (https://github.com/stackhawk/hawkop/pull/41)hawkop scan metrics— hawkopscan-path-metricsPreflight checks for these (
hawkop policy create --help,hawkop scan metrics --help); if absent, the affected phase is skipped with guidance to upgrade hawkop. No hawkop release is required to merge this docs/skill PR.Test Plan
generate-cursor-rules.sh→ no diff)optimize: 6 prompts, 5 checks) + registered inevals/cli.py/optimizeend-to-end against a sample app once hawkoppolicy+scan metricsare installed🤖 Generated with Claude Code