Skip to content

chore(harness): /commit-sweep + nightly automation (#461 A2) - #516

Merged
thomasluizon merged 2 commits into
mainfrom
feature/461-b2-commit-sweep
Jul 15, 2026
Merged

chore(harness): /commit-sweep + nightly automation (#461 A2)#516
thomasluizon merged 2 commits into
mainfrom
feature/461-b2-commit-sweep

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

Part of #461, bundle 2 (A2). Rebased onto main after #514 merged (supersedes #515, which GitHub closed when its stacked base branch was deleted).

Adds:

  • .claude/skills/commit-sweep/SKILL.md — report-only cross-commit regression sweep over the last N commits (or --since) across both repos; six-check incident rubric; ranked report to .claude/audits/commit-sweep.md.
  • .github/workflows/commit-sweep.yml — nightly (0 5 * * *) + workflow_dispatch; dual public checkout; claude-code-action@v1 + CLAUDE_CODE_OAUTH_TOKEN; upserts a commit-sweep-labeled tracking issue + artifact.
  • CLAUDE.md pointer line.

Harness-only: no product code, no parity/i18n.

🤖 Generated with Claude Code

Add a report-only cross-commit, cross-repo regression sweep over a window
of recent `main` commits in BOTH repos, plus the nightly automation that
runs it and surfaces findings.

- .claude/skills/commit-sweep/SKILL.md: reads the last N commits (default
  10) or `--since <when>` across both repo checkouts, applies a six-check
  incident rubric holistically (migration DDL dup -> 42P07, dropped RN
  pins, analyzer-silent-locally ORBIT0001/0002, cross-repo contract drift,
  parity/i18n drift, stale-artifact QA), and writes one ranked report to
  .claude/audits/commit-sweep.md plus a `.status` sidecar. Never remediates.
  Roots resolve from ORBIT_UI_ROOT/ORBIT_API_ROOT with local defaults.
- .github/workflows/commit-sweep.yml: nightly cron + workflow_dispatch;
  checks out both public repos (main at root for skill discovery, orbit-api
  into ./orbit-api); runs claude-code-action@v1 with the subscription OAuth
  token; event-branched prompt (schedule -> --since 24h, dispatch -> -n N);
  plain run: steps upsert a single commit-sweep-labeled tracking issue on
  findings and always upload the report artifact + a job-summary line.
- CLAUDE.md: one pointer line for /commit-sweep.

Harness-only: no product code, no parity/i18n mirror.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
orbit-ui-mobile-web Ignored Ignored Jul 15, 2026 11:52am

Request Review

Comment thread .github/workflows/commit-sweep.yml Outdated

The two repo roots come from the `ORBIT_UI_ROOT` and `ORBIT_API_ROOT` env vars (already set for this job); resolve each root from those, exactly as the skill's Phase 1 describes.

Run exactly this command: ${{ github.event_name == 'schedule' && '/commit-sweep --since "24 hours ago"' || format('/commit-sweep -n {0} {1}', inputs.commits, inputs.scope) }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[High] Manual workflow_dispatch invocation uses a -n flag the skill's parser never defines — silently drops custom commit-count/scope overrides

This builds /commit-sweep -n {commits} {scope} (e.g. /commit-sweep -n 25 api). SKILL.md Phase 1 never defines a -n flag — it only recognizes a bare numeric first token as N, --since <when> for time mode, and a trailing non-numeric/non---since token as scope (matching argument-hint: <N=10 | --since <when>> <repo-scope: both|ui|api>).

Walking the grammar against -n 25 api: -n is non-numeric and isn't --since, so it gets consumed as the scope candidate — but -n doesn't match both|ui|web|mobile|api|backend. N falls back to its default of 10 (never sees 25), and 25/api are never consumed by any token slot.

Effect: any manual run with non-default commits/scope inputs silently ignores the overrides and runs with N=10/scope=both — no error, just wrong behavior. Only the nightly --since "24 hours ago" path is unaffected.

Fix: emit the format the skill actually parses — format('/commit-sweep {0} {1}', inputs.commits, inputs.scope) — or add a -n flag to the skill's Phase 1 grammar.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review: PR #516 — chore(harness): /commit-sweep + nightly automation (#461 A2)

Scope: PR #516 in thomasluizon/orbit-ui-mobile
Recommendation: NEEDS WORK

Summary

Harness-only PR (no product code): adds .claude/skills/commit-sweep/SKILL.md (a new report-only cross-commit/cross-repo sweep skill), .github/workflows/commit-sweep.yml (nightly + on-demand trigger), and a one-line CLAUDE.md pointer. No apps/*, orbit-api, or packages/shared files touched, so every gated subagent (parity, i18n, contract, security-reviewer, design) and the backward-compat guard are N/A. Close reading of the two new files against each other surfaced one real functional bug (the workflow's manual-dispatch invocation uses a flag the skill's own parser never defines) and one real-but-narrow CI script-injection anti-pattern.

Findings

Critical

None.

High

[High] Manual workflow_dispatch invocation uses a `-n` flag the skill's parser never defines — silently drops custom commit-count/scope overrides
· dimension: 1. Correctness
· location: .github/workflows/commit-sweep.yml:31 (the "Run exactly this command: ..." prompt line) vs .claude/skills/commit-sweep/SKILL.md Phase 1 "Resolve roots + parse args"
· issue: The workflow constructs the manual-dispatch invocation as `format('/commit-sweep -n {0} {1}', inputs.commits, inputs.scope)` — e.g. `/commit-sweep -n 25 api`. SKILL.md Phase 1 never defines a `-n` flag anywhere (verified via full-file grep): it only recognizes a bare numeric first token as N, `--since <when>` for time mode, and "a non-numeric, non-`--since` first token is treated as the scope." Walking that literal grammar against `-n 25 api`: `-n` is non-numeric and isn't `--since`, so it's consumed as the scope candidate — but `-n` matches none of `both|ui|web|mobile|api|backend`. `N` falls back to its 10-default (never sees `25`); `25`/`api` are never consumed by any token slot (no third-token handling exists). The frontmatter `argument-hint: <N=10 | --since <when>> <repo-scope: both|ui|api>` confirms bare-token syntax was intended, not `-n <N>`.
· risk: Any manual workflow_dispatch run with non-default `commits`/`scope` inputs silently ignores the overrides and runs with N=10/scope=both instead — no error, just wrong behavior. The nightly cron path (`--since "24 hours ago"`) is unaffected since it matches the grammar correctly; only the on-demand path this PR advertises is broken for its one configurable use case.
· fix: Either add a `-n` flag to SKILL.md's Phase 1 grammar, or fix the workflow to emit the format the skill actually parses: `format('/commit-sweep {0} {1}', inputs.commits, inputs.scope)`.
· reference: CLAUDE.md rule 1 (root cause — spec and caller must agree); dimension 1 Correctness

Medium

[Medium] GitHub Actions script-injection anti-pattern — LLM-written file content interpolated directly into a `run:` block via `${{ }}` instead of `env:` indirection
· dimension: 12. Security
· location: .github/workflows/commit-sweep.yml — "Write the job summary" step, `status="${{ steps.status.outputs.status }}"`
· issue: `steps.status.outputs.status` traces back to `.claude/audits/commit-sweep.status`, written by the unattended Claude agent step (driven by a prompt built from commit messages/diffs in the swept window). The prior step only runs the value through `tr -d '[:space:]'`, which strips whitespace but not `;`, `"`, `` ` ``, `$()`, `#`, `${IFS}` — a whitespace-free payload survives untouched and, once substituted as literal text before the shell parses it, breaks out of the `status="..."` assignment.
· risk: The agent's `Bash(git:*),Bash(mkdir:*),Bash(ls:*)` tool allowlist gives zero protection here — the exploit needs no Bash, only `Write` (explicitly permitted), so a successfully prompt-injected agent could write shell metacharacters instead of one of the three sanctioned words, and the later, non-agent-controlled `run:` step executes them with runner command execution. This step carries no explicit secret in its own `env:`, so immediate blast radius is runner RCE + reachable `id-token: write` OIDC, not direct `GITHUB_TOKEN` abuse — hence Medium: the precondition chain (get a commit past branch-protection review, then jailbreak the unattended agent) is real but non-trivial.
· fix: Add `env: SWEEP_STATUS: ${{ steps.status.outputs.status }}` to the step and reference `"$SWEEP_STATUS"` in the script body instead of direct interpolation. Trivial, standard defense-in-depth.
· reference: OWASP / CWE-78; GitHub Actions script-injection best practice; dimension 12 Security "Injection"

Low / Info

None (signal-gated out per rubric).

Subagents

Agent Verdict
parity-checker N/A — no apps/web/**/apps/mobile/** changed
i18n-syncer N/A — no user-facing strings or i18n JSON changed
contract-aligner N/A — no packages/shared/src/types/*/endpoints.ts; orbit-api not touched (not verifiable in CI regardless — sibling repo not checked out)
security-reviewer N/A — orbit-api not touched (Security dimension #12 was still walked directly above)
design-reviewer N/A — no apps/web/**, apps/mobile/**, or orbit-landing-page/src/** UI file changed

Validation

Phase 6 (/validate) skipped per CI-wrapper convention — this PR runs Build / Unit Tests / SonarCloud as separate required checks.

Deferred — N/A dimensions & files not verdicted

  • Dimension 4 (Comment policy): N/A — no .ts/.tsx/.cs source files changed.
  • Dimensions 6–7 (Type safety, console.log): N/A — no application code changed.
  • Dimension 8 (DESIGN.md/AI-slop): N/A — no apps/* UI files changed.
  • Dimensions 9 (Parity), 10 (i18n), 11 (Contract drift/backward-compat), 13 (Backend hard rules): N/A — confirms the PR body's own "harness-only" claim.
  • Dimension 14 (FEATURES.md parity): N/A — internal tooling automation, not a user-facing feature/screen/tool/gating change.
  • All three changed files (SKILL.md, commit-sweep.yml, CLAUDE.md) received a full verdict above.

What's good

  • Tight tool-scoping on the unattended CI agent — no Edit tool, so it structurally cannot modify repo code even under prompt injection.
  • Consistent root-resolution design: ORBIT_UI_ROOT/ORBIT_API_ROOT env vars exactly match the skill's documented fallback behavior when unset.
  • The scheduled (--since "24 hours ago") path is correctly formatted against the skill's grammar — only the manual-override path has the bug.
  • .status sidecar as a single machine-readable token is a clean way for the workflow to branch on outcome.
  • The CLAUDE.md pointer line accurately reflects the skill's actual behavior.

Recommendation

Fix both findings before merge: (1) align the workflow's manual-dispatch command format with the skill's actual argument grammar, and (2) route steps.status.outputs.status through env: indirection. Both are small, mechanical fixes; the overall design is sound.

…v-indirect status (#461 A2 review)

- workflow_dispatch emitted `/commit-sweep -n {N} {scope}` but the skill's Phase 1 parser
  defines no `-n` flag (positional grammar: numeric first token = N, trailing token = scope).
  Emit `/commit-sweep {N} {scope}` so manual commit-count/scope overrides actually apply. (review High)
- job-summary step interpolated the agent-written status sidecar via `${{ }}` straight into a
  shell `run:`; route it through `env: SWEEP_STATUS` and reference `$SWEEP_STATUS` to close the
  GitHub Actions script-injection vector. (review Medium)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@thomasluizon

Copy link
Copy Markdown
Owner Author

Addressed both findings in 05e1692:

  • High: dropped the bogus -n dispatch flag — the workflow now emits /commit-sweep {N} {scope}, matching the skill's positional grammar (numeric first = N, trailing token = scope), so manual workflow_dispatch overrides actually apply.
  • Medium: routed the agent-written status sidecar through env: SWEEP_STATUS and reference $SWEEP_STATUS in the job-summary run: block, closing the GitHub Actions script-injection vector.

@sonarqubecloud

Copy link
Copy Markdown

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review: PR #516 — chore(harness): /commit-sweep skill + nightly commit-sweep.yml (#461 A2)

Recommendation: APPROVE

Summary

Harness-only PR: adds the report-only /commit-sweep skill (.claude/skills/commit-sweep/SKILL.md, new), its nightly + on-demand GitHub Actions workflow (.github/workflows/commit-sweep.yml, new), and a one-line CLAUDE.md pointer bullet. No apps/*, no orbit-api, no packages/shared touched — parity, i18n, and contract-drift surfaces are all out of scope for this diff.

A prior automated review on this PR (CHANGES_REQUESTED) flagged two real issues:

  1. [High] The manual workflow_dispatch invocation used a -n flag the skill's own argument parser never defines, silently dropping custom commit-count/scope overrides.
  2. [Medium] steps.status.outputs.status was interpolated directly into a run: block via ${{ }} instead of env: indirection — a GitHub Actions script-injection anti-pattern reachable if the unattended sweep agent were prompt-injected into writing shell metacharacters to its .status sidecar.

Commit 05e16921 fixes both, and both fixes are verified present in this diff:

  • The dispatch prompt now emits /commit-sweep {N} {scope} (bare positional args matching the skill's actual grammar — no more bogus -n flag).
  • The job-summary step now routes the value through env: SWEEP_STATUS: ${{ steps.status.outputs.status }} and references "$SWEEP_STATUS" in the script body, closing the injection vector.

A fresh full-diff pass found no new Critical/High issues.

Findings

Critical: None.
High: None.
Medium: None.

Low / Info:

  • .github/workflows/commit-sweep.yml grants the agent a blanket Bash(git:*) in allowedTools, broader than the sibling claude-review.yml's explicit read-only git allowlist. This is deliberate and justified — Phase 3 check 1 of the skill needs git grep, which a narrower allowlist would block — and the blast radius is contained by permissions: contents: read (a git push is rejected server-side) plus the ephemeral runner. No action needed.

Grounding spot-check

Cross-referenced the skill's incident-rubric evidence pointers against the real repo state on origin/main: apps/mobile/scripts/fix-hoisting.js exists, and the named pins (hermes-compiler, memoize-one, regenerator-runtime, promise, react-native-worklets) are all present in apps/mobile/package.json / root overrides. No fabricated grounding.

Subagents

All five gated N/A — diff touches no apps/web, apps/mobile, orbit-api, packages/shared/src/types, i18n JSON, or UI files.

Validation

N/A per CI adaptation — Build / Unit Tests / SonarCloud run as separate required checks; no source code changed.

Deferred

DESIGN.md/AI-slop, Parity, i18n, Contract-drift/backward-compat guard, Backend hard rules, FEATURES.md parity — all N/A, surface not touched by this diff. All three changed files (SKILL.md, commit-sweep.yml, CLAUDE.md) received a verdict; nothing silently skipped.

Severity Count
Critical 0
High 0
Medium 0
Low / Info 1

@thomasluizon
thomasluizon merged commit de6c02a into main Jul 15, 2026
20 checks passed
@thomasluizon
thomasluizon deleted the feature/461-b2-commit-sweep branch July 15, 2026 11:59
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