feat: migrate to @mobb.ai/cli (new Bugsy) + CI contract tests - #38
Open
antonychiu2 wants to merge 14 commits into
Open
feat: migrate to @mobb.ai/cli (new Bugsy) + CI contract tests#38antonychiu2 wants to merge 14 commits into
antonychiu2 wants to merge 14 commits into
Conversation
Allow omitting report-file in the review (gh-fixer) action so the Mobb CLI can run its own internal SAST scan (opengrep) instead of requiring an external SARIF/JSON report. -f is appended only when report-file is provided.
Allow omitting scanner alongside report-file. --scanner is appended only when the input is non-empty, so scan-and-fix mode (no external report) no longer requires a scanner declaration.
…ed args Bugsy now ships as @mobb.ai/cli — a prebuilt standalone binary, same code and version stream as mobbdev, but no longer tied to the user's Node version. The CLI's review command hard-requires -f, --scanner and --ch (verified against 1.4.57: omitting them yields "Missing required arguments: f, scanner, ch, commit-hash"). Combined with the previous `eval ... || true`, that failure was silent: the step went green, fix-report-url was empty, the status step was skipped by its startsWith guard, and the user saw a passing check with no fix report. - switch to npx --yes @mobb.ai/cli@latest, bump setup-node v3.6.0 -> v4 - validate report-file/scanner/commit-hash/pr-number up front with an actionable ::error:: message; GitHub does not reliably enforce `required:` for composite actions - validate scanner against the CLI's own --scanner choices - add fail-on-error input (default false, preserving the historical non-blocking behaviour) and always emit an ::error:: annotation on non-zero exit instead of swallowing it - echo the CLI's stdout, which command substitution was hiding entirely - add src-path (-p) and polling inputs - quote $OUT in the tr pipeline so output containing * cannot glob - fix "GitaHub Token" typo organization-id is deliberately not wired here: the CLI rejects it on review with "Unknown argument: org". Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs
Switch to npx --yes @mobb.ai/cli@latest (standalone binary, same 1.4.57 code as
mobbdev) and bump setup-node v3.6.0 -> v4.
New inputs wired to flags the current CLI supports:
- polling -> --polling (for runners behind proxies/firewalls blocking WSS)
- create-one-pr-> --create-one-pr, appended only alongside --auto-pr, with a
fail-fast check when set without auto-pr
- src-path -> -p, for monorepos
- scan-only -> --scan-only
Also:
- guard the status step with startsWith(..., 'https://'), matching the review
action, so it no longer posts a status with an empty target_url when URL
extraction fails
- echo the CLI's stdout, which command substitution was hiding entirely
- quote $OUT in the tr pipeline so output containing * cannot glob
- fix "GitaHub Token" typo
Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs
--pr-id was previously appended only inside the commit-directly branch, so an --auto-pr run in a pull request context never told Mobb which PR the fixes belong to. Pass it whenever a PR context exists and either auto-pr or commit-directly is enabled. Kept as an isolated commit so it can be dropped if the coupling to commit-directly was intentional. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs
This repo had no real CI signal: both existing workflows always passed report-file, so scan-and-fix, diff-aware, auto-pr and commit-directly were never exercised, and nothing checked that the flags the actions emit still exist in the CLI. Review-mode tests live in their own files (added separately) so a review regression shows up as its own red check. lint.yml - actionlint over the workflows, with -shellcheck= since shellcheck floods on the deliberate eval pattern (see reverted PRs #31/#33/#35) - .github/scripts/validate-actions.py for the composite action.yml files, which actionlint cannot lint: it parses them as workflows and fails on the missing on/jobs sections. The script bash -n's every run block, checks that every inputs.<name> reference is declared and every declared input used, and checks the README examples only pass real inputs -- which immediately caught the documented-but-nonexistent auto-commit input. cli-smoke.yml (no secrets, runs on fork PRs, no Mobb quota) - binary-ubuntu: required leg proving the platform binary resolves and runs. @mobb.ai/cli is a ~3 KB launcher that resolves a binary from optionalDependencies rather than downloading one, so it hard-fails on musl/Alpine, win-arm64, and wherever optional deps are skipped. - binary-other-os: macOS/Windows, continue-on-error, informational only. - analyze-contract: asserts analyze still documents every flag action.yml passes, and that -f stays optional so scan-and-fix keeps working. - review-contract: asserts review still documents the flags review/action.yml passes and still requires f, scanner and ch. This is the standing alarm for the mismatch this branch fixes. test-analyze-args.yml - asserts the exact command the root action builds for each input combination via the MOBB_ACTION_DRY_RUN seam, including the negative case of create-one-pr without auto-pr. No API calls. test-analyze-e2e.yml - real runs over the repo's vulnerable fixtures: scan-and-fix, diff-aware, and a manual-only auto-pr leg. Asserts fix-report-url is a real URL. main.yml - checkout/setup-node v3 -> v4, skip on fork PRs instead of failing red on missing secrets, and assert fix-report-url starts with https:// Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs
Review mode gets its own workflows so that when it breaks the failing check says "review" rather than hiding inside an analyze-mode failure. test-review-e2e.yml - CodeQL -> ./review with fail-on-error: true, so a CLI failure propagates instead of producing the green-check-with-no-fix-report that the previous `eval ... || true` allowed. Asserts fix-report-url is a real URL. - daily schedule, so a CLI-side regression surfaces with no open PR - a secret-free negative job asserting that omitting report-file fails the step and produces no fix-report-url test-review-args.yml - asserts the command review/action.yml builds (-f, --scanner, --ch, --pr, --github-token) plus pass-through of mobb-project-name, src-path and polling - negative cases: no report-file, no scanner, unsupported scanner, and no pull_request context review.yml - kept as the reference example users copy; bumped checkout v3 -> v4 and codeql-action v2 -> v3 (both flagged by actionlint as too old to run), added the fork-PR guard and the permissions CodeQL needs Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs
…rements - fix the scan-and-fix example: it passed `auto-commit: true`, which no action has ever declared, so it was silently dropped. The real input is `commit-directly`. The new metadata validator now catches this class of bug. - add a Requirements section naming @mobb.ai/cli and documenting the Alpine/musl, win-arm64 and --no-optional caveat, with `npx mobbdev@latest` as the fallback for those runners - document the new inputs: create-one-pr, src-path, polling, fail-on-error - add a Review action section: the README previously framed the repo as having two modes without mentioning that review mode requires report-file and scanner and only runs on pull_request events. The CLI hard-requires all of them, so this is now stated plainly, along with a worked example and a note that organization-id is rejected on review. - add a Versioning section noting the v1 tag currently lags the v1.x tags - drop the specific scanner-engine name from the scan-and-fix description; the internal SAST implementation is in flux scan-only is deliberately left undocumented while Mobb SAST settles. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs
This reverts commit b85c453.
Testing the generated commands against @mobb.ai/cli 1.4.57 surfaced two constraints the action did not respect: 1. "--create-one-pr and --commit-directly cannot be provided at the same time". The action would happily pass both. Now rejected up front with an explanation of why they conflict. 2. "--pull-request flag requires --commit-directly to be provided as well". This is why --pr-id was originally coupled to commit-directly, so the preceding commit that promoted --pr-id to plain --auto-pr runs was wrong and has been reverted. The assertion in test-analyze-args.yml is inverted to match: --pr-id must NOT appear on an --auto-pr-only run. Also adds cli-smoke checks that track both constraints. They emit notices rather than failing, since if the CLI relaxes a constraint nothing breaks - the action's guard just becomes unnecessarily strict and should be revisited. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs
The previous caveat was partly wrong. Verified against the published 1.4.57 linux-x64 package and by running it in containers: - the binary is dynamically linked (interpreter /lib64/ld-linux-x86-64.so.2, needs libc.so.6 and libstdc++.so.6) and requires glibc >= 2.28, so RHEL/CentOS 7 and Ubuntu 18.04 are excluded too. That was not documented. - on Alpine, npm still installs the glibc linux-x64 package, because the platform packages carry no npm `libc` constraint. The failure therefore surfaces as `spawnSync .../mobbdev ENOENT` at spawn time. There is no helpful "install mobbdev instead" message, contrary to what the previous wording claimed. - on glibc < 2.28 the error is `mobbdev: not found`. In both cases the file exists; the ELF interpreter or libc does not. - the documented fallback, `npx mobbdev@latest`, was confirmed working on node:20-alpine. Replaces the prose caveat with a platform-support table and the real error signatures, so someone hitting either error can recognise it. Also notes that an Alpine container job cannot use this action regardless, since actions/setup-node installs official nodejs.org builds and has no musl variant; those users should invoke the CLI directly. Adds a cli-smoke musl-fallback job that runs in node:20-alpine and asserts the documented fallback still works, so the escape hatch cannot rot. It also emits a notice if @mobb.ai/cli ever starts working on musl. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs
Running the lint workflow locally through `act` surfaced a real failure, not an
emulation artifact:
error: externally-managed-environment
× This environment is externally managed
`python3 -m pip install pyyaml` cannot install into the system Python on modern
Ubuntu (PEP 668), which is what ubuntu-latest now is. Add actions/setup-python
so pip has an interpreter it owns.
Verified: both lint jobs now pass under act.
Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs
Reframes the platform section around what the action supports: GitHub-hosted ubuntu runners, self-hosted Linux with glibc 2.28+, and the platforms the CLI publishes binaries for. Drops the table of unsupported environments and the error-signature notes. Keeps one neutral line pointing at `npx mobbdev@latest` for other environments, so anyone outside the supported set still has a path forward. Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs
Trims this branch back to the migration itself, per review feedback.
review/action.yml
- restore the original logic: conditional -f and --scanner appends, and
`eval ... || true`. Removes the fail-fast validation of report-file,
scanner, commit-hash and pr-number, the scanner allowlist, and the
fail-on-error input. Bugsy reports these conditions itself.
action.yml
- remove the create-one-pr/auto-pr and create-one-pr/commit-directly input
validation for the same reason.
both
- remove the src-path input. These actions always run in GitHub CI, where the
source is checked out at the workspace root, so pointing the CLI at an
arbitrary local path is not a case worth supporting.
- remove the MOBB_ACTION_DRY_RUN test seam and the mobb-command output. CI
should exercise the real thing; API usage is not a constraint here.
workflows
- delete test-analyze-args.yml and test-review-args.yml, which existed only to
assert the dry-run command string.
- test-analyze-e2e.yml now covers the combinations for real: scan-and-fix,
polling, scan-only, diff-aware, and a manual-only auto-pr leg.
- test-review-e2e.yml drops fail-on-error and the negative job. The review
action keeps its non-blocking behaviour, so the fix-report-url assertion is
what turns a CLI problem into a red check in CI.
- cli-smoke.yml drops the checks that only existed to justify the removed
guards, and the musl fallback job.
README
- point everything at v1.1, the active release tag, and drop the discussion of
other tags.
- remove src-path and fail-on-error.
Two small fixes are kept in both actions: quoting $OUT in the tr pipeline so
CLI output containing * cannot glob, and the startsWith('https://') guard on
the root action's status step, which the review action already had.
Claude-Session: https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Migrates both composite actions from
npx mobbdev@latestto the new Bugsy home,@mobb.ai/cli— same Bugsy code and version stream (both at1.4.57), but shipped as a prebuilt standalone binary so the CLI no longer depends on the runner's Node version.Context: new home for Bugsy and prod deploy + testing.
The package swap itself is one line per action. Most of this PR is the three things that came out of actually testing it.
Finding 1: review mode's scan-and-fix could never work, and failed silently
The two commits already on
feat/review-scan-and-fixmadereport-fileandscanneroptional so review mode could do scan-and-fix. Verified against1.4.57, the CLI'sreviewcommand hard-requires them:--ch/--commit-hashis required too — it's hidden from--helpbut still enforced.Worse,
review/action.ymlusedOUT=$(eval $MobbExecString || true), so that failure was silent: step green,fix-report-urlempty, status step skipped by itsstartsWithguard, user sees a passing check and no fix report.Per discussion the inputs stay optional, but the action now validates up front and fails with an actionable message pointing at the root action for scan-and-fix. A new
fail-on-errorinput (defaultfalse, preserving today's non-blocking behaviour) makes CLI failures blockable, and a non-zero exit now always produces an::error::annotation instead of vanishing. The CLI's stdout is also echoed — command substitution was swallowing it entirely.organization-idis deliberately not wired into review: the CLI answersUnknown argument: org.Finding 2: two auto-pr flag constraints the action violated
Feeding the generated commands to the real CLI turned up:
--create-one-pr and --commit-directly cannot be provided at the same time— the action would pass both. Now rejected up front.--pull-request flag requires --commit-directly to be provided as well— this is why--pr-idwas coupled tocommit-directly. I'd initially "fixed" that coupling as a bug; it's CLI-enforced, so that commit is reverted in this PR.Finding 3: packaging caveat worth knowing before
v1moves@mobb.ai/cliis a ~3 KB launcher that resolves@mobb.ai/cli-<os>-<arch>(~137 MB) fromoptionalDependenciesand spawns it — it does not download at runtime. So it hard-fails on musl/Alpine,win-arm64, and anywhere optional deps are skipped (--no-optional,omit=optionalin an.npmrc); its own error text says to fall back tomobbdev. Documented in the README, and the smoke matrix is what will catch it.v1tag still points at a July-2023 commit while the README tells everyone to use@v1, so merging alone changes nothing for existing users. Suggest cuttingv1.3at merge and only repointingv1once the scheduledcli-smokerun has been green for a few days — repointing swaps the CLI package for every consumer at once.New inputs
Root action:
polling,create-one-pr,src-path,scan-only. Review action:polling,src-path,fail-on-error.scan-onlyis wired but intentionally undocumented — per Anatoli and Kirill it's tied to the unreleased Mobb SAST work, so it's ready without being announced.Tests
The repo had no real CI signal: both existing workflows always passed
report-file, so scan-and-fix, diff-aware,auto-prandcommit-directlywere never exercised, and nothing checked the CLI flag contract. Review mode gets its own files so a review regression is its own red check, and ubuntu is the required leg everywhere.lint.ymlvalidate-actions.pyfor the composite files — actionlint can't lint those, it parses them as workflows and failscli-smoke.ymlbinary-ubuntu(required) proves the platform binary resolves and runs; macOS/Windows arecontinue-on-errorinformational legs;analyze-contractandreview-contractassert every flag each action emits still exists, thatanalyze -fstays optional, and thatreviewstill requiresf/scanner/chtest-analyze-args.ymlcreate-one-prwithoutauto-pr,create-one-prwithcommit-directly)test-review-args.ymlmain.ymlfix-report-urlis a real URLtest-analyze-e2e.ymlauto-prlegtest-review-e2e.ymlfail-on-error: true, daily schedule, and a secret-free negative job asserting a missingreport-filefails instead of going greenvalidate-actions.pybash -ns everyrun:block, checks everyinputs.<name>is declared and used, and checks README examples only pass real inputs — which immediately caughtauto-commit: truein the README, an input no action has ever declared, silently dropped at runtime all this time. Nowcommit-directly.Secret-gated jobs skip on fork PRs rather than failing red.
Verified locally before pushing
actionlintclean across all workflowsvalidate-actions.pycleanMOBB_ACTION_DRY_RUN=trueacross 16 input combinations — all passNot done here
No de-
evalrefactor — PRs #31/#33 attempted that and #35 reverted them for breaking deployments. Only additive validation.Sibz/github-status-action@v1is unmaintained and still on a floating tag; replacing it is a separate change.Next
Point a test repo at
mobb-dev/action@feat/mobb-ai-cli-migrationto exerciseauto-pr,create-one-pr,commit-directlyand review mode (GH Fixer) — the paths CI can't safely assert because they write PRs.https://claude.ai/code/session_011tjj4uQmBrSMYKrDjCbHGs