Skip to content

fix(cli): flag collision, stale bundled themes, and codemod edge cases - #4639

Merged
josephfarina merged 10 commits into
mainfrom
fix/cli-chaos-remaining
Aug 3, 2026
Merged

fix(cli): flag collision, stale bundled themes, and codemod edge cases#4639
josephfarina merged 10 commits into
mainfrom
fix/cli-chaos-remaining

Conversation

@josephfarina

@josephfarina josephfarina commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Summary

The remaining functional defects from chaos-testing the CLI, each a focused
commit with a regression test. Stacked on #4637; review/merge that first.

search / build verbose flag collided with the global --detail

The root program has a value-taking global option --detail <level> (the
component/hook/docs verbosity level). search and build also declared a
boolean --detail for verbose output. Commander matched the global's
value-requiring spec first, so astryx search button --detail failed with
option '--detail <level>' argument missing and the verbose output was
unreachable. Renamed the boolean flag to --verbose; the global is unchanged.
Added a CLI test (the flag had none, which is why the collision went unnoticed).

Stale bundled CLI themes

The themes bundled under packages/cli/assets/templates/themes/ (used by
astryx theme add to scaffold a theme without installing the package) had
drifted from source. The neutral bundle was missing an accessibility fix — a
darker light-mode secondary text color that clears the WCAG AA contrast minimum
— plus a StatusDot color block, so astryx theme add neutral was scaffolding a
theme below AA contrast. Regenerated all seven bundles so they match source,
and added a drift guard test so a future theme edit that forgets to regenerate
is caught in CI.

unwrap-authoring-factories codemod edge cases

The upgrade codemod that rewrites the old create*() authoring calls into plain
objects produced broken output in two cases:

  • Shorthand type: createComponentDoc({name, type}) (where type is a
    variable) printed the invalid {name, 'component'}. Now emits the correct
    type: 'component'.
  • No-argument calls: createConfig() / createComponentDoc() were left in
    place while the import they depend on was removed in the same pass — a
    reference to a deleted binding. Now replaced with the object the factory would
    have returned ({} for config/integration, {type: '…'} for the stamping
    factories).

Upgrade codemod reachability (test only)

Investigated a concern that the new authoring-migration codemods might be
unreachable via astryx upgrade. Confirmed they are reachable: upgrade
targets the installed core version, and core ships alongside the CLI in the
same release, so a released core reaches the codemods (verified end to end).
Added a test that pins this so a future change can't silently strand the
migration. No behavior change.

Investigated, no change needed

A reported light/dark CSS fallback issue turned out to be a false alarm — every
light-dark() / color-scheme construct across core, the theme build, and the
docsite was already correct.

Stale comments (cosmetic)

Fixed three code comments left by the reorg that described the old file layout
or referenced factory functions that were removed. Comment-only.

Note: an earlier draft of this stack included a middle PR guarding the API
against null options/context arguments. It was dropped — the generated types
already make null a compile error for those parameters (options?: {...} is
{...} | undefined), so the plain options = {} default is sufficient and
idiomatic.

Stack

  1. fix(cli): confine user paths, close DoS vectors, and repair broken paths #4637 — confine user paths, close DoS vectors, repair broken paths
  2. fix(cli): flag collision, stale bundled themes, and codemod edge cases #4639 (this PR) — flag collision, stale themes, codemod edges, cleanup

Test plan

  • codemod suite — 17 cases including the two edge cases (green)
  • search --verbose CLI test (green)
  • theme-bundle drift guard — all 7 themes match source (green)
  • upgrade codemod reachability (green)
  • eslint clean on all touched files

Close four path-confinement gaps found by chaos testing:

- theme build --out: options.out was passed to path.resolve with no guard,
  allowing writes outside the project root (../ESCAPE.css). Now uses
  assertWithin (parity with theme add).
- theme build <file>: the input file path was similarly unconfined, allowing
  reads of arbitrary files. Now confined.
- validate-integration manifest roots: components/templates/codemods root
  values from an integration manifest were resolved against the package dir
  with no confinement — a manifest could point codemods at '../OUTCM' and
  the validator would import+execute files outside the package (RCE).
  Now guarded with assertWithin; escapes report a 'root_outside_package'
  validation issue instead of executing.
- Same fix in foundation/integrations/integrations.mjs (the load path used
  by upgrade --integration).
- layout --file: readFileSync accepted any path (including /dev/zero →
  OOM). Now confined + size-capped at 5 MB, rejects non-files.

assertWithin already existed and was used by theme-add, swizzle, template,
init, and upgrade --path — these were the unguarded outliers.
- levenshtein: early-exit when |len(a)-len(b)| > 3 (all callers use
  thresholds ≤5, so the sentinel 999 is safe). Drops search@10k from ~17s
  to <1s while preserving real typo suggestions. Closes the DoS where a
  3k+ char query caused multi-second CPU spins (8,860 lev calls per search).

- XLE parseValue: add depth counter (MAX_VALUE_DEPTH=64) to prevent stack
  overflow on deeply nested attribute values ({a:{a:{a:...}}}). The existing
  MAX_COMPACT_DEPTH guard covers tree nesting but missed parseValue recursion.

- XLE expand: cap repeat count at MAX_REPEAT=10000 to prevent heap OOM on
  B*999999999. The parser accepts any *N but the expander now clamps it.

- layout --file: size-cap (5MB) and reject non-files before readFileSync,
  preventing OOM from /dev/zero (already confined by PR1's assertWithin).
…ed docs (F4/F7/F12)

- docs _adapter: use Object.create(null) for the topic lookup map so
  __proto__/constructor don't bypass the unknown-topic guard and leak
  ERR_INVALID_ARG_TYPE. Same pattern as #912.

- sandbox generate-cli-registry: CLI_SRC repointed from the deleted
  packages/cli/src to packages/cli/clients/cli. The script was silently
  writing a degenerate cliRegistry.ts with all commands '(failed to load)'.

- getting-started + working-with-ai docs: fix the consumer bin path
  from bin/astryx.mjs (deleted) to clients/cli/bin/astryx.mjs (real).
  Consumers copy this into their package.json scripts.
@vercel

vercel Bot commented Aug 1, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview Aug 3, 2026 5:35pm

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 1, 2026
@josephfarina
josephfarina force-pushed the fix/cli-chaos-remaining branch from 020dda4 to f98cf75 Compare August 1, 2026 19:45
@github-actions github-actions Bot added the needs:design-review Affects visuals — Design should review label Aug 1, 2026
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

No new or modified components detected.

Bundle Size Summary

No component packages changed.

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

github-actions Bot added a commit that referenced this pull request Aug 1, 2026
github-actions Bot added a commit that referenced this pull request Aug 1, 2026
@josephfarina josephfarina changed the title fix(cli): remaining chaos-test defects — flag collision, stale themes, codemod edges (F10/F11/U1/U2) fix(cli): flag collision, stale bundled themes, and codemod edge cases Aug 2, 2026
@josephfarina
josephfarina force-pushed the fix/cli-chaos-remaining branch from 71b0ba6 to 992d3e5 Compare August 2, 2026 16:14
@josephfarina
josephfarina changed the base branch from fix/api-robustness-cleanup to fix/security-path-confinement August 2, 2026 16:15
github-actions Bot added a commit that referenced this pull request Aug 2, 2026
josephfarina and others added 4 commits August 3, 2026 17:29
… changeset

The DoS early-exit used |m-n| > 3, but the hook suggester keeps matches at
distance <= 5, so a hook name 4-5 edits away was dropped from suggestions.
Widen the cutoff to > 5 (still O(1), same DoS protection) and pin it with
tests. Add the missing changeset for this consumer-facing CLI fix.
The global `--detail <level>` option (component/hook/docs verbosity) takes a
value, so it shadowed search's and build's boolean `--detail` flag: `astryx
search button --detail` errored with 'option --detail <level> argument
missing' and the verbose import/match output was unreachable. Rename the
boolean flag to `--verbose` so it no longer collides; the global `--detail
<level>` is unchanged. Add a CLI regression test for `search --verbose`
(the flag had no test, which is why the collision went unnoticed).
The bundled themes under packages/cli/assets/templates/themes/ (used by
`astryx theme add` to scaffold without the package installed) had drifted from
source, and nothing guarded against it. The neutral bundle was missing the WCAG
light-mode text-secondary fix (neutral-500 #737373 -> neutral-600 #525252, which
clears AA 1.4.3 on the T95 body) and a StatusDot override block, so
`astryx theme add neutral` scaffolded a theme below AA contrast.

- Regenerate via `pnpm bundle:cli-themes` (verbatim copy of source) so every
  bundle byte-matches its packages/themes/<slug>/src source.
- Make the chocolate source Prettier-clean. It was the one theme source that
  wasn't, so the verbatim bundle copy couldn't stay byte-identical once
  lint-staged reformatted it — the drift guard below would fight formatting
  forever. Clean source keeps copy == source stable.
- Add scripts/check-cli-theme-bundle.test.mjs: fails loudly if any bundled
  theme (or its icons) drifts from source, so a future theme edit that skips
  `pnpm bundle:cli-themes` is caught in CI instead of shipping stale.
…lls (U1/U2)

Two edge cases in the v0.3.0 unwrap-authoring-factories codemod produced broken
output:

- U1 (shorthand `type`): `createComponentDoc({name, type})` where `type` is a
  local binding is a shorthand property. Overwriting only its .value left
  shorthand:true, so it printed `{name, 'component'}` — invalid syntax. Force
  the explicit `type: '<kind>'` form (clear shorthand, reset the key).

- U2 (no-arg call): `createConfig()` / `createComponentDoc()` were left in
  place, but the factory import is removed in the same pass — leaving a dangling
  reference to a deleted binding. Replace a no-arg call with the object the
  factory produced from no input: `{}` for config/integration, `{type: '<kind>'}`
  for the stamping factories.

Add regression tests for both.
josephfarina and others added 3 commits August 3, 2026 17:31
The v0.3.0 authoring codemods are registered at the codemod registry's top
version and only run when installed @astryxdesign/core has reached that version.
Because core ships in the same fixed-version release group as the CLI, a
released core does reach it — but nothing pinned that invariant, so a future
registry entry above the shipped core version would silently strand the
migration (the chaos-test 'codemods unreachable' finding). This test drives
`astryx upgrade` against a project whose installed core is at the registry
latest and asserts an old-surface authoring file is rewritten, plus that
latestVersion tracks the top registry version.
Three JSDoc comments described the pre-reorg layout or the removed create*
factories:
- build-theme.mjs: said the module lives at src/commands/ and resolves the bin
  via ../../bin/astryx.mjs; it is at clients/cli/commands/ and correctly uses
  ../bin/astryx.mjs.
- integration-runner.mjs / runner.mjs: referenced createCodemod /
  createConfigCodemod, which v0.3.0 removed. Codemods are now plain objects
  stamped type: 'code' | type: 'config'.

Comment-only; no behavior change.
@josephfarina
josephfarina force-pushed the fix/cli-chaos-remaining branch from 992d3e5 to f1be37e Compare August 3, 2026 17:31
Base automatically changed from fix/security-path-confinement to main August 3, 2026 21:05
@josephfarina
josephfarina merged commit b7c0b78 into main Aug 3, 2026
20 checks passed
github-actions Bot added a commit that referenced this pull request Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. needs:design-review Affects visuals — Design should review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant