chore(harness): Stage 4 (/investigate + rules) + Stage 5 (WORKFLOW.md) + Stage-3 audit-workflow smoke fixes - #438
Conversation
…ixes Stage 4 (the two in-scope items; /second-opinion deferred — opencode is now present, so it is unblocked for a follow-up): - New /investigate skill: a gated incident runbook (Sentry issue -> Render deploy/logs -> Postgres rows read-only -> localize via csharp-lsp -> root cause -> minimal fix behind a human gate). Pinned to claude-fable-5 (effort medium): the runbook is mechanical multi-hop tracing, the one judgment step sits behind a human gate that catches a mis-diagnosis before any edit, and since model: applies only for the current turn the fix IMPLEMENTATION lands on the session model (Opus) post-approval. - Proactive tooling-defaults rule added at ~/.claude/rules/tooling-defaults.md (native auto-load; lives outside the repo, not in this commit). Stage-3 conversion smoke test (/audit-security api) found + fixed 5 bugs in the audit workflow: 1. Named workflow resolution is unavailable in this build -> repoint all 5 audit/prod-readiness skills to the scriptPath form. 2. autocrlf gives the workflow .mjs CRLF, which the Workflow approval dialog rejects as control characters -> add a .gitattributes LF gate on .claude/workflows/*.mjs. 3. The top-level Workflow tool delivers args as a JSON string -> normalize at the script boundary in both workflows. 4. scope:api under-covered security (the isApiSurface substring heuristic dropped injection/secrets/rate-limit/error-leakage) -> explicit repos tags on the security surfaces + a tier-bounded, <=6-gap completeness critic. 5. The default-refuted skeptic gate was bypassed for ~all findings (finders emit CRITICAL/HIGH while isSerious matched the Tier 1/Tier 2 vocabulary) -> isSerious is now rank()-based; dropped the dead KIND.*.verify config. Also: gitignore .claude/audits/ (generated reports, alongside reviews/reports/sweeps). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nsmhto1amnzXEmbSvuJgw9
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
Review: PR #438 — chore(harness): /investigate runbook + Stage-3 audit-workflow smoke fixes
Recommendation: APPROVE
Summary
This PR is internal harness tooling only — no apps/, packages/shared, or orbit-api files are touched (11 files: 4 trivial SKILL.md text swaps, one new SKILL.md runbook, .claude/research/harness-upgrade-plan.md, .gitattributes, .gitignore, and the two workflow engines audit.mjs / prod-readiness.mjs). All five bug fixes described in the PR body (scriptPath fallback, CRLF .gitattributes gate, args-as-JSON-string normalization, security surface repos tagging, vocabulary-agnostic isSerious) check out against the actual diff: dead code (isUiSurface, cfg.verify, verifySet) is fully removed with zero dangling references, and the parsedArgs normalization is applied consistently everywhere args?. used to appear. One Medium-severity scoping inconsistency survived review in the new repos: 'both' tagging.
Findings
Critical: None
High: None
Medium
[MEDIUM] `authz-isolation` tagged repos:'both' but its `where` text is API-only, producing self-contradictory finder prompts for scope=ui/web/mobile
· dimension: 1. Correctness
· location: .claude/workflows/audit.mjs:80 (line number as of the merged PR head)
· issue: The security surface `{ label: 'authz-isolation', repos: 'both', where: 'orbit-api controllers + every CQRS query/command handler in src/Orbit.Application — ...' }` was newly tagged `repos: 'both'` by this diff, but unlike the other four surfaces this PR widened (`injection`, `secrets-config`, `ratelimit-ai-abuse`, `error-web-auth`), its `where` text was never updated to mention any web/mobile-side concern (e.g. Server Action / BFF authz checks). Contrast with the sibling `ai-mcp-scoping` surface, which is equally API-only in its `where` text and was correctly left as `repos: 'api'` — and note the PR's own bug list (body, bullet 4) names only `injection`/`secrets`/`rate-limit`/`error-leakage` as the surfaces that were wrongly dropped under `scope='api'`; `authz-isolation` isn't among them, since under the old `isApiSurface` substring check it already matched `orbit-api` in its `where` text and was already correctly scoped.
· risk: `resolveSurfaces('security', 'ui'|'web'|'mobile'|'frontend')` will now include `authz-isolation` in every UI-scoped security audit. `finderPrompt` will render "Objective: security audit of the 'authz-isolation' surface in orbit-ui-mobile" alongside "Where to look: orbit-api controllers + every CQRS query/command handler in src/Orbit.Application" — a self-contradictory instruction to the Haiku finder. Best case it wastes an agent call and returns nothing useful; worst case it wanders into unrelated territory or reports a false "clean" for a surface it never meaningfully checked in the UI repo, undermining the exact scope-coverage fix this PR sets out to make.
· fix: Either revert to `repos: 'api'` (matches its pre-existing correct behavior and the PR's own stated bug list), or — if UI-side authz coverage (Server Actions / BFF routes forwarding to the API, per rubric dimension 12) is genuinely intended — update `where` to name that explicitly, the same way `error-web-auth` calls out "web auth cookie flags" and "mobile token storage" alongside its API-side language.
· reference: rubric.md dimension 1 (Correctness); PR body §"Stage-3 smoke test" bullet 4 (the surface this diff was meant to fix)
Low / Info: None posted (per the rubric's signal gate).
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | N/A — no apps/web/** or apps/mobile/** files changed |
| i18n-syncer | N/A — no user-facing strings or packages/shared/src/i18n/*.json changed |
| contract-aligner | N/A — neither repo's contract surface (packages/shared/src/types/*, endpoints.ts, orbit-api DTOs) changed |
| security-reviewer | N/A — no orbit-api code changed |
| design-reviewer | N/A — no apps/*/orbit-landing-page UI files changed |
Validation
| Check | Result |
|---|---|
| Lint | N/A — no file in any workspace's lint scope changed (.claude/workflows/*.mjs isn't part of any turborepo workspace's TS/JS lint target) |
| Type check | N/A — same reason |
| Tests | N/A — same reason |
| Build (api) | N/A — orbit-api untouched |
| CI (Build / Unit Tests / SonarCloud) | Skipped per workflow instructions — this PR runs these as separate required checks |
Deferred — N/A dimensions & files not verdicted
- Dimension 8 (DESIGN.md/AI-slop), 9 (Parity), 10 (i18n), 11 (Contract drift + backward-compat), 12 (Security), 13 (Backend hard rules), 14 (FEATURES.md parity) — all N/A: the diff's file list contains only
.claude/*,.gitattributes,.gitignore; none of these dimensions' triggering surfaces (apps/*,packages/shared/*,orbit-api/*,FEATURES.md) are touched. - Cross-repo dimensions (contract-aligner, orbit-api side of the backward-compat guard) are not verifiable in this CI job —
orbit-apiis not checked out here — but are also not applicable since no orbit-api files changed in this diff. - All 11 changed files were read and given a verdict — nothing deferred at the file level.
What's good
- The
isSeriousrewrite (rank(f.severity) <= 1in.claude/workflows/audit.mjs, using the pre-existingrank()helper) correctly fixes the described skeptic-bypass bug — preserves the exact same critical/high (or tier-1/tier-2) semantics as the old per-kindverifylists, just vocabulary-agnostically. - Dead code from the refactor (
isUiSurface,cfg.verify,verifySet) is cleanly and completely removed — zero dangling references left. - The
args-as-JSON-string normalization (typeof args === 'string' ? JSON.parse(args) : args || {}) is applied consistently at every formerargs?.call site in bothaudit.mjsandprod-readiness.mjs, including the?? 2default and nestedloop?.maxDryRoundsaccess. - New
.claude/skills/investigate/SKILL.mdis well-structured: explicit read-only-until-gate phases, a hard human gate before any file edit, and evidence-pinned capture fields per phase — consistent with the rest of the harness's verification-first conventions.
Recommendation
Merge is safe as-is (no Critical/High). Fix the one Medium (authz-isolation scoping) whenever convenient — either in a quick follow-up or folded into this PR before merge, since it's a one-line change (repos: 'api' revert, or a where-text broadening) with no other consequences.
…net pin) Fable 5 is disabled 2026-07-12; pinning a skill to a model that gets retired makes it error or silently fall back. Dropped the model: pin entirely so /investigate inherits the session model (Opus 4.8 at xhigh) instead — best-model diagnosis for a low-frequency high-stakes runbook, and no hardcoded model id left to rot. Model routing belongs on high-volume leaves (the Haiku audit finders, 40+ per run), not a once-per-incident skill. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nsmhto1amnzXEmbSvuJgw9
…ign pattern Adds the top-level slice-vs-campaign decision (litmus: converging-metric finish line = campaign; known diff = slice), a Campaign pattern section (assess via workflow -> fix a batch via /implement -> PR -> /clear -> re-run across fresh sessions, with the #243 worked example), and the plan->implement fresh-session split for cross-repo/risky slices. /execute stays the default for slices; the doc now teaches when NOT to reach for it. Bundled into this PR rather than a separate one per the minimum-PRs preference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nsmhto1amnzXEmbSvuJgw9
|
Two follow-on changes since the original description:
|
|
There was a problem hiding this comment.
Review: PR #438 — chore(harness): Stage 4 (/investigate + rules) + Stage 5 (WORKFLOW.md) + Stage-3 audit-workflow smoke fixes
Recommendation: APPROVE
Summary
Internal harness tooling only — no apps/, packages/shared, or orbit-api files touched. This is an update to a PR I reviewed previously (commit 8d77eb9d, APPROVED). The diff since that review is documentation-only: WORKFLOW.md's Stage-5 slice-vs-campaign router rewrite, notes/checkbox updates in .claude/research/harness-upgrade-plan.md, and a 2-line frontmatter removal in .claude/skills/investigate/SKILL.md dropping the model: claude-fable-5 / effort: medium pin (the subject of the separate, already-landed commit 2f3223a1 — sound given Fable's disablement date noted in the research doc). .claude/workflows/audit.mjs, .claude/workflows/prod-readiness.mjs, and the four audit-* SKILL.md files are byte-identical to what I reviewed last time.
Findings
Critical: None
High: None
Medium (pre-existing, unchanged since last review — not new, not blocking)
[MEDIUM] `authz-isolation` tagged repos:'both' but its `where` text is API-only, producing self-contradictory finder prompts for scope=ui/web/mobile
· location: .claude/workflows/audit.mjs:80
· issue: { label: 'authz-isolation', repos: 'both', where: 'orbit-api controllers + every CQRS query/command handler in src/Orbit.Application ...' } — its `where` never mentions a web/mobile-side concern, unlike the sibling `ai-mcp-scoping` surface (equally API-only, correctly tagged `repos: 'api'`).
· risk: `resolveSurfaces('security', 'ui'|'web'|'mobile')` includes this surface in every UI-scoped security audit, handing the Haiku finder a self-contradictory prompt (UI objective, API-only "where to look").
· fix: `repos: 'both'` → `repos: 'api'`, or broaden `where` to name a real UI-side authz concern (Server Actions / BFF routes forwarding to the API), the way `error-web-auth` does.
Verified unchanged: git diff 8d77eb9d..HEAD -- .claude/workflows/audit.mjs is empty — this file received zero edits in this review cycle, so the finding is carried forward, not new, and consistent with the prior APPROVE.
Low / Info: None (per the rubric's signal gate).
What's good (new since last review)
WORKFLOW.md's Stage-5 rewrite is a clean, well-organized restructure: the slice-vs-campaign router (Step 1) with a concrete litmus test ("converging metric" vs "known diff"), the new Campaign pattern section grounded in the #243 worked example, and the plan→implement fresh-session split — all internally consistent with the existing slice ladder, which is preserved intact.- Dropping the
/investigatemodel/effort pin is a good call given the noted 2026-07-12 Fable disablement risk; the rationale (low-frequency/high-stakes judgment work doesn't benefit from a cheap-tier pin the way high-volume audit leaves do) is sound and well-documented in the research plan.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | N/A — no apps/web/** or apps/mobile/** changed |
| i18n-syncer | N/A — no i18n surface changed |
| contract-aligner | N/A — no contract surface changed |
| security-reviewer | N/A — no orbit-api code changed |
| design-reviewer | N/A — no UI files changed |
Validation
CI (Build / Unit Tests / SonarCloud) runs as separate required checks per workflow instructions — skipped here. No lint/type/test-covered source changed (.claude/* tooling, .mjs workflow scripts, and docs only).
Deferred — N/A dimensions
DESIGN.md/AI-slop, Parity, i18n, Contract-drift/backward-compat, Backend hard rules, FEATURES.md parity — all N/A, no triggering surface in this diff. Cross-repo dimensions requiring orbit-api (not checked out in this job) are not applicable here since no orbit-api files changed.
Recommendation
Safe to merge as-is. The one Medium (authz-isolation scoping in audit.mjs:80) is pre-existing and unchanged in this cycle — fix whenever convenient, no urgency.



Stage 4 (partial — the two in-scope items)
Per
.claude/research/harness-upgrade-plan.md./second-opinionis intentionally deferred, but note opencode is now present (v1.14.48) so its "deferred until opencode is back" condition is void — it's unblocked for a follow-up session./investigateskill — a gated production-incident runbook: Sentry issue → Render deploy/logs → Postgres rows (read-only) → localize via csharp-lsp → root cause → minimal fix behind a human gate (read-only until the gate). Pinned toclaude-fable-5/effort: mediumbecause the runbook is dominated by mechanical multi-hop tracing; the one judgment step sits behind a human gate that catches a mis-diagnosis before any edit; and sincemodel:applies only for the current turn, the fix implementation lands on the session model (Opus) post-approval. Verified the MCP servers it uses are live (Sentry orgthomasluizon✓, Render ✓, csharp-lsp ✓).~/.claude/rules/tooling-defaults.md(native.claude/rules/auto-load; global, so not in this PR's diff).Stage-3 smoke test:
/audit-security apifound + fixed 5 workflow bugsThe Stage-3 conversion (audit skills → deterministic Haiku workflows) was never run live. Running one real audit surfaced five real bugs — the actual value of the smoke test:
Workflow({name:'audit'})→ "Available: (none)") → repointed all five/audit-*+/prod-readinessskills to thescriptPathform.autocrlf=truegives the working-tree.mjsCRLF, which the Workflow approval dialog rejects as control characters →.gitattributesLF gate on.claude/workflows/*.mjs(durable across re-checkout).argsas a JSON string, not an object →args?.kindwas undefined → normalize at the script boundary in both workflows.scope:apiunder-covered security —isApiSurfaceinferred repo from a substring match on each surface'swhere, silently dropping injection/secrets/rate-limit/error-leakage → explicitrepos: 'api'|'ui'|'both'tags + a tier-bounded, ≤6-gap completeness critic.CRITICAL/HIGHwhileisSeriousmatched theTier 1/Tier 2vocabulary via exact set-membership →isSeriousis nowrank()-based (vocabulary-agnostic); dropped the deadKIND.*.verifyconfig.Also: gitignore
.claude/audits/(generated reports, alongsidereviews//reports//sweeps/).Note on the audit's findings
The smoke run completed (43 Haiku agents, 0 errors, 4 critic rounds, 184 raw findings) but predates fix #5, so its findings were not skeptic-verified — they're captured as an explicit validation artifact (gitignored, not committed) with an unverified caveat. The billing-webhook concurrency cluster it surfaced is a real lead worth a dedicated orbit-api hardening session and a clean re-run.
Scope
Single-repo — orbit-api was only read (via csharp-lsp + the audit), never modified.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Nsmhto1amnzXEmbSvuJgw9