Skip to content

Remove --json from the app security commands until they follow the JSON output contract - #8807

Merged
nickwesselman merged 6 commits into
mainfrom
app-security/no-json
Oct 6, 2026
Merged

nickwesselman merged 6 commits into
mainfrom
app-security/no-json

Conversation

@jek

@jek jek commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

The --json output of the shopify app security commands doesn't follow docs/cli/json-output.md. For example, its fields are snake_case and check and review reuse the result file schemas. Converting it is too big a change for launch. Releasing the current shape and converting later would break a public contract, but adding --json later only adds output. So this removes --json for launch. The commands are exempt from the typed JSON output requirement until they're converted.

WHAT is this pull request doing?

  • Removes --json and jsonOutputSchema from check, record, review and clean, along with their JSON encoders, fixtures and tests. The services still return typed results separately from the terminal presenters, so the conversion can add new encoders on top of them.
  • Adds the five app security commands, including instructions, to a new section of json-output-command-exceptions.js, so unhiding them doesn't fail the command-json-output lint rule.
  • Drops error.details from record rejections and from invalid results file errors, since only JSON error documents read it. The terminal still lists every error. Without the details assignment, no-error-factory-functions rejected the error factories, so they're now abort…(): never helpers like the ones in app-security-selection.ts.
  • Removes --json from the coding-agent instructions and the command descriptions, so agents following them don't pass a nonexistent flag. The instructions now tell agents to report from review, the only combined view of the results, instead of combining the results files themselves, and explain --blocking as the pass/fail gate.
  • Fixes two places where agents got wrong directions:
    • agent-checks.json told agents to pipe their findings to a bare shopify app security record, which records to the default configuration's results when check ran with --config, --client-id, --without-app-config or from another directory. It now points them at the exact command from their instructions.
    • review's next steps said running check "also updates" agent-findings.json, which it never does. With findings, the agent step now depends on the agent findings: a deeper review when there are none, the existing older-results step when they're older than the scan, and how to refresh them otherwise.

Machine-readable results are still in deterministic-findings.json, agent-checks.json and agent-findings.json in the results directory.

The first commit is the removal. The second updates the agent instructions, the descriptions and oclif.manifest.json. The third tells agents to report from review. The rest fix the directions above.

How to manually test your changes?

In a local app:

  1. pnpm shopify app security check --path <app> --json exits 2 with a Nonexistent flag: --json error document on stdout, like other commands without --json, such as app env show. --help doesn't list --json.
  2. pnpm shopify app security check --path <app> --skip-instructions shows the report and writes the result files to <app>/.shopify/app-security/shopify.app/.
  3. pnpm shopify app security check --path <app> --list-files prints one path per line.
  4. echo '{}' | pnpm shopify app security record --path <app> rejects the document and lists every error.
  5. pnpm shopify app security review --path <app> shows the combined results.
  6. pnpm shopify app security instructions --path <app> prints instructions that don't mention --json and tell the agent to report from review.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

jek added 2 commits October 6, 2026 10:54
The output doesn't follow docs/cli/json-output.md yet, and adding --json
later is non-breaking where changing a released shape isn't. The services
keep returning typed results separately from their presenters, and the
commands are exempt from the typed JSON output lint rule until they adopt it.
The agent instructions no longer suggest --json, so agents following them
don't fail on a nonexistent flag.
@jek
jek requested review from a team as code owners October 6, 2026 18:01
@github-actions github-actions Bot added the no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. label Oct 6, 2026
@jek

jek commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

/snapit

1 similar comment
@jek

jek commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

/snapit

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

🫰✨ Thanks @jek! Your snapshot has been published to npm.

Built from c2615d9844b758635f96fc6a2a960a63eb05e4db. Workflow run.

Test the snapshot by installing your package globally:

pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20261006184401

Caution

After installing, validate the version by running shopify version in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

Without --json, review's output is the only combined view of the results.
The agent instructions now say so, warn that its boxes wrap long paths and
commands, and explain what --blocking does.
@jek
jek requested a review from jplhomer October 6, 2026 19:15
@jek

jek commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

/snapit

jek added 2 commits October 6, 2026 12:22
agent-checks.json told agents to pipe their findings to a bare
`shopify app security record`. When check ran with --config, --client-id,
--without-app-config or from another directory, that wrote the findings to
the wrong results directory without warning.
Review's next steps said running check "also updates" agent-findings.json,
which check never does. Agents now report from review, so say that the
agent runs check and records its findings again.
@jek

jek commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

/snapit

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

🫰✨ Thanks @jek! Your snapshot has been published to npm.

Built from e47a169f1388a09e70c167b4fee3c0a69d445191. Workflow run.

Test the snapshot by installing your package globally:

pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20261006193249

Caution

After installing, validate the version by running shopify version in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

With findings, review now asks for a deeper review when the agent hasn't
recorded findings, leaves refreshing to the older-results step when its
findings are older than the scan, and otherwise says how to refresh them.
@jek

jek commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

/snapit

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

🫰✨ Thanks @jek! Your snapshot has been published to npm.

Built from a30a448c03ac1cce11ea8a4fd9c74a9d0ebdafe5. Workflow run.

Test the snapshot by installing your package globally:

pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20261006195249

Caution

After installing, validate the version by running shopify version in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

@nickwesselman
nickwesselman added this pull request to stack #8809 October 6, 2026 20:04
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

⚠️ Potential Breaking Changes Detected

This PR contains changes that may break the existing contract.

@shopify/dev_experience — this PR contains breaking changes that require coordination for the next major release.

🏳️ Removed Flags

The following flags were removed from existing commands:

Command Flag
app:security:check --json
app:security:clean --json
app:security:record --json
app:security:review --json

@jplhomer jplhomer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worked locally 👍

@nickwesselman
nickwesselman added this pull request to the merge queue Oct 6, 2026
Merged via the queue into main with commit b6defe8 Oct 6, 2026
29 of 30 checks passed
@nickwesselman
nickwesselman deleted the app-security/no-json branch October 6, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants