Repository navigation
Conversation
fix(release): adds release workflow
## [1.9.1-next.1](v1.9.0...v1.9.1-next.1) (2026-07-09) ### Bug Fixes * **release:** adds back github token ([cd34c52](cd34c52)) * **release:** adds defaults ([fdac965](fdac965)) * **release:** adds dependency of test job ([3cd41cf](3cd41cf)) * **release:** adds new release workflows ([9350a16](9350a16)) * **release:** adds permissions to action ([d66921b](d66921b)) * **release:** brings testing job into release action ([94551bd](94551bd)) * **release:** fixes commitlint issues ([ac51e58](ac51e58)) * **release:** fixes typo in workflow ([53d1443](53d1443)) * **release:** makes release config common js ([ccefcad](ccefcad)) * **release:** removes extraneous test that was causing failure in build not local ([687bbd6](687bbd6)) * **release:** removes PR action and corrects branch for testing push ([5a1059a](5a1059a)) * **release:** replaces inline testing with test workflow chaining ([d5b77b7](d5b77b7)) * **release:** stop committing build output and drop unused deps ([d64ac2c](d64ac2c)), closes [#45](#45) * **release:** updates branches for workflow for main and next ([725d29c](725d29c)) * **release:** updates workflow away from quotations as per the github docs ([c6dbd1a](c6dbd1a))
Updating workflow to run integration on pull request to main
* adding workflow test on pull-request * updating with only main branch for test * Trying out --yes on kickstart:kill * removed promotional logging for dotenvx * camel-> kebab case, tests * package-lock version update * Address PR review feedback - import-generate: detect deprecated flags in --flag=value form, not just bare --flag - kickstart-install: replace setTimeout-chained install steps with sequential awaited steps so errors propagate through try/catch and ordering is deterministic; also await createKickstart (was previously fire-and-forget) - utils: confirmOrExit now requires both stdin and stdout to be TTYs before treating the session as interactive, and normalizes confirmation input (trims whitespace, accepts y/yes case-insensitively) * Add test coverage for confirmOrExit and kickstart:kill - utils.ts: extract isConfirmationAccepted() as a pure, exported function so the accept/reject decision logic can be unit tested directly without simulating a real TTY - kickstart-kill.ts: export action() and add an injectable deps parameter (isDockerInstalled, confirmOrExit, spawn) so tests can exercise the confirmation gating without touching real docker or exiting the process - add __tests__/utils.test.js covering isConfirmationAccepted and the yes-bypass / non-interactive TTY-detection paths of confirmOrExit - add __tests__/commands/kickstart-kill.test.js covering docker-not-installed, CLI_DIR mismatch, --yes bypass, and confirm-rejected gating paths - wire both new test files into the test and test:unit npm scripts * Fix confirmOrExit silently proceeding when process.exit is mocked/deferred Previously, the rl.question callback called process.exit(0) on decline but had no return statement, so resolve() ran unconditionally afterward. In production this was masked because process.exit halts execution synchronously, but in any environment where exit is mocked or deferred (e.g. tests), a declined confirmation would be silently treated as accepted, letting the caller proceed with the risky operation. - extract handleConfirmationAnswer(answer, resolve, reject): resolves on accept, exits + rejects on decline, so the promise can never silently resolve when exit doesn't actually happen - confirmOrExit now passes both resolve and reject into handleConfirmationAnswer - add 3 tests in __tests__/utils.test.js covering accept, decline, and the decline-with-mocked-exit case that reproduces the original bug * Add test coverage for import:generate deprecated-flag detection - extract getDeprecatedFlagUsage(argv) as a pure, exported function so the deprecation-detection logic is testable without mocking process.argv or console.warn - export DEPRECATED_FLAGS for use in tests - add __tests__/commands/import-generate.test.js covering: no deprecated flags used, bare --flag and --flag=value forms detected, multiple deprecated flags detected together, new kebab-case form not flagged, and that both the deprecated and current flag spellings populate the same underlying Commander option property - wire the new test file into the test and test:unit npm scripts * Fix crypto.randomUUID global usage and confirmOrExit non-interactive gap - kickstart-install.ts: import randomUUID from node:crypto explicitly instead of relying on the global WebCrypto object, matching the convention already used elsewhere in the codebase - utils.ts: confirmOrExit() now throws after errorAndExit() in the non-interactive path, mirroring the fix already applied to handleConfirmationAnswer in the interactive path. In production this is a no-op since process.exit(1) halts synchronously first, but in any environment where exit is mocked/deferred, the promise now rejects instead of silently resolving and letting the caller proceed with the risky operation - update the three non-interactive tests in __tests__/utils.test.js to assert.rejects, which now actually exercises the fixed behavior * fix(ci): remove stale duplicate pull_request trigger from test workflow The rebase onto next carried forward an old commit that added a second pull_request trigger (scoped to branches: main) to what was then integration-tests.yml. That file has since been renamed to test.yaml on next, which already has its own unscoped pull_request trigger. The duplicate key is invalid YAML (most parsers, including GitHub Actions', silently keep only the last occurrence), which risked the main-scoped trigger silently overriding the intended unscoped one and breaking CI for PRs targeting next. Removed the stale block. next takes priority over main going forward, so a main-specific trigger no longer serves any purpose here. File is now byte-identical to next's original. * fix(test): pin integration test FusionAuth image to 1.69.2 The integration test fixture pinned fusionauth/fusionauth-app:latest, a floating tag. This made the integration test's pass/fail status depend on whatever FusionAuth happened to publish as latest at run time, independent of anything in this repo's history. Pin to 1.69.2 (current release) for reproducible test runs. Confirmed passing against a clean container/volume state. The kickstart:install command's own docker-compose.yml template (src/resources/kickstart/fusionauth/docker-compose.yml), which gets copied into end users' projects, intentionally remains on :latest so new installs always get the current FusionAuth release. * test: cleaned up tests and brought in validator lib for email validation * chore: remove CONTRIBUTING.md Content will be migrated into README.md separately. * fix: exit with non-zero status on kickstart:install failure, fix typo The outer catch block only logged the error and let the command return successfully. A failed file copy, kickstart-file write, rename, or environment update would produce an error message while the CLI still exited with status 0, masking failures from scripts/CI that check the exit code. Set process.exitCode = 1 in that path. Also fix a JSDoc typo: intial -> initial. * test: replaced mock-fs with real temp dir-based file testing --------- Co-authored-by: Mark Robustelli <137117976+mark-robustelli@users.noreply.github.com>
andrewpai
reviewed
Oct 7, 2026
* adding workflow test on pull-request
* updating with only main branch for test
* Trying out --yes on kickstart:kill
* camel-> kebab case, tests
* package-lock version update
* Address PR review feedback
- import-generate: detect deprecated flags in --flag=value form, not just bare --flag
- kickstart-install: replace setTimeout-chained install steps with sequential
awaited steps so errors propagate through try/catch and ordering is
deterministic; also await createKickstart (was previously fire-and-forget)
- utils: confirmOrExit now requires both stdin and stdout to be TTYs before
treating the session as interactive, and normalizes confirmation input
(trims whitespace, accepts y/yes case-insensitively)
* Add test coverage for confirmOrExit and kickstart:kill
- utils.ts: extract isConfirmationAccepted() as a pure, exported function so
the accept/reject decision logic can be unit tested directly without
simulating a real TTY
- kickstart-kill.ts: export action() and add an injectable deps parameter
(isDockerInstalled, confirmOrExit, spawn) so tests can exercise the
confirmation gating without touching real docker or exiting the process
- add __tests__/utils.test.js covering isConfirmationAccepted and the
yes-bypass / non-interactive TTY-detection paths of confirmOrExit
- add __tests__/commands/kickstart-kill.test.js covering docker-not-installed,
CLI_DIR mismatch, --yes bypass, and confirm-rejected gating paths
- wire both new test files into the test and test:unit npm scripts
* Add test coverage for import:generate deprecated-flag detection
- extract getDeprecatedFlagUsage(argv) as a pure, exported function so the
deprecation-detection logic is testable without mocking process.argv or
console.warn
- export DEPRECATED_FLAGS for use in tests
- add __tests__/commands/import-generate.test.js covering: no deprecated
flags used, bare --flag and --flag=value forms detected, multiple
deprecated flags detected together, new kebab-case form not flagged,
and that both the deprecated and current flag spellings populate the
same underlying Commander option property
- wire the new test file into the test and test:unit npm scripts
* checkpointing application create - incomplete
* checkpoint
* Added next steps.
* code review updates
* Potential fix for pull request finding 'Use “does not exist” for grammatical agreement'
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding 'Update wrapper reference to executeApplicationCreate'
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Address PR review feedback
- Fix inverted localhost/container-IP fallback order in integration
test setup's auth-readiness check
- Gate CORS system-configuration mutation behind --yes/confirmOrExit
per the Risky Operations Policy
- Make --name optional; required only for --profile, preserved from
--data JSON unless explicitly overridden
- Document that applicationId/clientId are intentionally identical
(FusionAuth never accepts clientId as input)
- Remove NODE_ENV-conditional exit from executeApplicationCreate so
it always returns a result per its documented contract; thread the
raw error through to the CLI wrapper for field-level error detail
* fix: remove duplicate pull_request trigger key in test.yaml
A prior commit's small addition to the now-retired integration-tests.yml
(a scoped 'pull_request: branches: [main]' trigger) got merged by git's
rename-detection into next's test.yaml during the rebase onto next,
producing invalid YAML with two 'pull_request:' keys in the same 'on:'
block. test.yaml's existing bare 'pull_request:' trigger already covers
all PRs unconditionally, making the scoped duplicate redundant regardless.
* fix: address additional PR review feedback
- Attribute error prefix only to actual createApplication failures,
not CORS setup failures that occur before it's ever called; add
regression tests asserting the message content for both cases
- Stop the kickstart:install spinner on any build-step failure
(fs.cpSync, createKickstart, rename, env write), not just success,
so a failed install doesn't leave its animation interval running
- Document the new application:create command in README.md
* fix: preserve structured FusionAuth errors through rawError
- catch blocks around retrieveSystemConfiguration/patchSystemConfiguration/
createApplication now attach the original rejection (typically a
FusionAuth ClientResponse carrying fieldErrors/generalErrors) as
Error.cause via a small wrapError() helper, instead of discarding it
when adding human-readable context
- the outer catch in executeApplicationCreate unwraps that cause for
ApplicationCreateResult.rawError, so errorAndExit/reportError's
dedicated ClientResponse/field-error formatting actually receives the
structured error instead of a generic wrapper Error
- add a regression test asserting rawError is the original
ClientResponse-shaped object, not an instance of Error
- fix README's --data example to show --name as optional, matching the
actual (preserve-JSON-name-unless-overridden) behavior
* fix: tolerate empty response bodies in integration test helper
makeApiRequest() called response.json() unconditionally, which throws
on successful-but-empty-body responses (e.g. DELETE /api/application
returns 200 with no body). This caused deleteApplication()'s soft
delete to throw before the hard delete ever ran, and the error was
silently swallowed by afterEach's try/catch, leaving every test
application behind. Read the body as text first and only parse it as
JSON when non-empty.
Also remove CONTRIBUTING.md — its content is superseded by the
Contributing section already in README.md (picked up from next).
* docs: remove dangling CONTRIBUTING.md references
CONTRIBUTING.md was removed since its content is superseded by
README.md's Contributing section. Update the two remaining comments
that referenced it to describe the confirmOrExit()/--yes gating
requirement directly instead of pointing at a file that no longer
exists.
* test: reduce duplication in application-create.test.js
- Extract REDIRECT_URI constant for the repeated callback URL literal
- Extract spaOptions()/webappOptions() helpers (mirroring the
integration test file's baseOptions() pattern) to replace the
repeated {...BASE_OPTIONS, profile, redirectUri} boilerplate
- Extract mockCompliantSystemConfig() for the repeated
already-compliant GET /api/system-configuration nock registration
- Merge 'clientSecret is absent for spa profile' and 'result contains
name, applicationId, and clientId' into one test — they used
identical mocks/options and only differed in which result fields
they asserted on
No behavioral changes; same assertions, same coverage.
* test: reduce duplication in application-create.integration.test.js
- Extract REDIRECT_URI constant for the repeated callback URL literal
- Extract spaOptions()/webappOptions() helpers wrapping the existing
baseOptions() factory, mirroring the unit test file's pattern
- Remove redundant explicit tenantId: TENANT_ID in the tenant-header
regression test — baseOptions() already defaults to that value
- Extract assertCorsHeadersConfigured() for the repeated CORS-headers
verification block (spa + native), and apply the enabled:true check
to the native test too, which previously lacked it
No change in test count or intent; same regression coverage, plus one
small strengthening (CORS enabled check now applies to native too).
* fix: prevent leaked FusionAuth integration test containers
Root cause: startFusionAuthContainer()'s pre-start cleanup used
`docker compose ps -q`, which only lists running/restarting
containers. A container left in a stopped (but not removed) state by
a prior interrupted run was invisible to this check, so cleanup was
skipped and the following `docker compose up -d` failed with
'Conflict: container name already in use'. Reproduced this directly
(stopped the db container mid-run, confirmed `ps -q` missed it while
`ps -aq` found it) before and after the fix.
- Use `docker compose ps -aq` so stopped containers are detected
- Stop silently swallowing a failure from the actual `docker compose
down -v` teardown once containers are confirmed to exist — let it
propagate instead of continuing into a doomed `up -d`
- Add SIGINT/SIGTERM handlers that attempt teardown before exiting,
so a manual Ctrl+C (e.g. during the health-check wait) doesn't skip
after()/t.after() and leak a container. Verified with a live
foreground SIGINT: handler fires, containers are removed, process
exits cleanly
- Remove `restart: unless-stopped` from the three services in the
test-only docker-compose.yml — on this ephemeral fixture it only
risked containers resurrecting themselves after a crash instead of
staying stopped
- Hoist the repeated composeDir computation to a shared COMPOSE_DIR
module constant
* build: use glob patterns instead of per-file lists in test scripts
Replace the manually-maintained list of every test filename with glob
patterns scoped to each test directory, so new test files are picked
up automatically without a package.json edit (verified: dropping a
scratch test file into __tests__/commands/ changed the count from 134
to 135 with zero script changes).
Kept unit/integration as separate steps rather than fully adopting
next's single bare `node --test` (which auto-discovers every
*.test.js file with no args) because that would run our two
Docker-dependent integration tests concurrently by default — verified
with a throwaway reproduction (two files racing on the same TCP port)
that Node's test runner runs multiple files in parallel processes
unless told otherwise. Our two integration tests share the same
docker-compose project/container names/ports, so concurrent execution
would be flaky at best.
- test:unit now globs each unit-test directory instead of naming
every file
- test:integration now globs __tests__/integration/**/*.test.js in a
single invocation with --test-concurrency=1, forcing sequential
execution (verified serial, no port conflicts) instead of two
separate node invocations chained by &&
- test is now just test:unit && test:integration
- Kept NODE_ENV=test in the script rather than dropping it — apply.ts's
executeAction() still relies on it being set to avoid calling
process.exit() on its error path, and apply.integration.test.js
doesn't set it itself
* fix: add --authorized-origin-url to the system CORS allowlist
ensureCorsHeaders() enabled CORS and added the required DPoP headers
to systemConfiguration.corsConfiguration.allowedHeaders, but never
touched corsConfiguration.allowedOrigins — a separate, required field
per FusionAuth's own CORS configuration docs. --authorized-origin-url
was only being copied into application.oauthConfiguration.
authorizedOriginURLs (the hosted-pages iframe/X-Frame-Options
allowlist), which is a different setting entirely. Net effect: the
command reported CORS as configured, but a spa's actual cross-origin
browser requests to the API would still be blocked unless the system
allowedOrigins already happened to include that origin.
- ensureCorsHeaders() now also accepts the authorized origins and
merges any missing ones into allowedOrigins, using exact
(case-sensitive) matching and skipping entirely when allowedOrigins
already contains '*'
- the confirmation-gate decision and prompt message now account for
origin changes too, not just headers/enabled — a missing origin
alone (with headers/enabled already compliant) now correctly
triggers confirmation, closing a gap where it would have silently
skipped the patch entirely
- applies to both spa and native profiles, consistent with how
headers are already handled for both
- added unit tests mirroring the existing header-merge coverage
(added when missing, no-op when present or when allowedOrigins
contains '*', confirmation gate covers origin-only changes) and
fixed one pre-existing test whose mock needed updating now that
origins are actually checked
- added a live integration test asserting the real system
configuration's allowedOrigins after a create with
--authorized-origin-url
* fix: address Copilot's latest review findings
- Disable telemetry during local test runs: add FUSIONAUTH_TELEMETRY=false
to test:unit/test:integration, mirroring how CI's workflow already sets
it. Verified src/.fa/config.json (gitignored, but a real artifact of
this gap) was being created and real analytics events were being sent
to PostHog on every local test run; confirmed it's no longer created
after this change. Also hardened telemetry.test.js's 'tests for
logEvent' describe block to explicitly delete FUSIONAUTH_TELEMETRY in
beforeEach rather than relying on test declaration order to leave it
unset for the one test that requires that -- it previously only passed
by coincidence (same latent fragility already present in CI, which sets
this var the same way)
- Validate --profile against the known profile keys before indexing
profileDefaults in executeApplicationCreate(). Direct library callers
bypass Commander's .choices() validation; an invalid value previously
silently spread "undefined" into an empty object and proceeded to
create an application with none of the advertised security defaults
while still reporting success
- Fix executeApplicationCreate()'s doc comment to accurately describe
that confirmOrExit() (invoked via ensureCorsHeaders() for spa/native
profiles) can still terminate the process for non-interactive callers
without yes=true, or decliners -- consistent with this project's
established Risky Operations convention elsewhere (kickstart-kill.ts).
No behavior change, just making the contract honest
- Update AGENTS.md's stale "No test framework - tests not implemented"
line to point at the actual node:test-based suite and npm scripts
* fix: handle --redirect-uri/--logout-url/--authorized-origin-url in --data mode
--data mode previously silently ignored these three flags entirely --
Commander accepted them, but the custom-mode branch never referenced
redirectUri/logoutUrl/authorizedOriginUrl at all, so passing any of
them with --data had no effect while the command still reported
success.
This broke the policy already established (and documented in code
comments) for --name earlier in this PR: --data provides "full custom
control," and any CLI flag with a corresponding JSON field is an
optional override -- it only takes effect when explicitly passed,
otherwise the JSON's own value is left untouched. --application-id
and --tenant-id already follow this pattern unconditionally in both
modes; --name follows it specifically in --data mode. These three
flags now do too.
Explicitly out of scope: this does not call ensureCorsHeaders() or
otherwise mutate system-wide CORS configuration in --data mode --
that remains --profile (spa/native) only, consistent with --data
mode's "caller owns their own infrastructure config" principle.
Added unit tests covering: JSON values preserved when the flags are
omitted, each flag overriding its corresponding JSON field when
explicitly provided, and confirming no system-configuration call is
made in --data mode even when --authorized-origin-url is passed.
* style: pass --env-file .env.test to all docker compose teardown calls
docker compose down -v / ps -aq were missing --env-file .env.test,
unlike the up -d call, which already passed it. Verified this
doesn't currently cause the failure Copilot's review described
(KICKSTART_FILE_PATH isn't actually referenced anywhere in
docker-compose.yml, and docker compose down -v --dry-run without
--env-file still exits 0 with only "variable not set" warnings) --
this exact code path has also torn down real containers successfully
many times already in this session's testing. Still worth fixing for
consistency with up -d and to silence the warnings; also removes any
doubt if the compose file ever adds a variable reference that down/ps
genuinely need to resolve correctly.
* fix: profile validation accepted inherited Object properties
`profile in profileDefaults` checks the full prototype chain, not just
own properties, so values like 'toString', 'constructor', and
'__proto__' incorrectly passed validation. profileDefaults['toString']
then resolves to the inherited Function (not undefined), and
{...profileDefaults['toString']} silently spreads to {} — reaching the
exact "empty defaults, no security profile or CORS applied" bug this
validation was added to prevent, just via a different vector than the
original invalid-string case already covered by tests.
Switched to Object.keys(profileDefaults).includes(profile), which only
considers own enumerable keys. Added a regression test confirming it's
rejected, and verified it reproduces (fails) without the fix.
Also updated the README's --profile example to mention
--authorized-origin-url and when it's needed, since enabling CORS
headers alone doesn't add any origin to the system allowlist.
* fix: restrict automatic CORS configuration to --profile spa only
CORS is purely a browser-enforced mechanism; native apps don't make
requests through a browser's CORS preflight/enforcement at all, so
FusionAuth's system-wide CORS allowlist has no effect on them.
--profile native was unnecessarily requiring system-configuration
permissions, prompting for --yes, and mutating a global security
setting (CORS enabled/headers/allowed origins) for no actual benefit.
Removed native from the ensureCorsHeaders() trigger condition, updated
all related comments/JSDoc/CLI help text, and flipped the native
integration test to assert system CORS configuration is left untouched
(comparing against the captured baseline) instead of asserting it was
configured. Added a dedicated unit test confirming native does not
call /api/system-configuration at all, mirroring the existing webapp
test. Verified via a full local docker-based integration run.
Also fixed a misleading comment on defaultRefreshTokenPolicy: it
described timeToLiveInSeconds as "the per-profile difference" in the
refresh token policy, but that field is actually the access token
(JWT) lifetime, set separately in jwtConfiguration — not part of the
refresh token usage/expiration policy at all.
* fix: resolve kickstart resources dir correctly when run from source
`npm start` runs src/index.ts directly via tsx, skipping the build's
copy-files step. kickstart-install.ts read resource files from
`${__dirname}/resources/...`, which only exists post-build (resources
get copied to dist/commands/resources alongside the compiled command);
in the source tree, resources actually live at src/resources, one
level up from src/commands. As a result, `npm start -- kickstart:install`
failed with a missing resource path.
Added resolveResourcesDir(), which checks the dist layout first
(__dirname/resources) and falls back to the src layout
(__dirname/../resources), throwing a clear error if neither exists.
Verified both layouts resolve correctly (manually, and via a new unit
test), and confirmed the full build + unit + integration suite still
passes.
* fix: restore build-first npm start script
package.json's "start" script was "node --import=tsx src/index.ts" on
this branch, but next's canonical value is
"npm run build && node dist/index.js" — this was an incorrect merge
conflict resolution during the earlier rebase onto next, which
dropped the build step and caused the resource-path regression fixed
in 52ae42e. That commit's resolveResourcesDir() fallback remains as a
defensive improvement for any other run-from-source scenario, but this
restores the actual root cause: npm start building and running from
dist/, matching next and ensuring resources are always copied before
the CLI needs them.
* fix: validate --data JSON is a non-null, non-array object
JSON.parse can return null, arrays, or primitives, but parseData() cast
the result straight to Application unchecked. Traced the actual failure
modes: --data 'null' crashed downstream with an opaque
"Cannot read properties of null (reading 'id')" TypeError; arrays and
primitives silently passed through property assignments and produced
nonsensical API payloads sent to FusionAuth, surfacing as confusing
server-side errors instead of a clear client-side validation message.
Added a shape check right after JSON.parse, throwing a clear Error
consistent with parseData()'s other validation errors. Added three
unit tests covering null/array/primitive --data input.
Also fixed two stale comments:
- A test comment claiming confirmOrExit() "never exits the process
itself" — this directly contradicted the JSDoc on
executeApplicationCreate (and the earlier fix in 7bb5071): production
calls CAN still exit via confirmOrExit(); only this specific test's
mocked process.exit turns that into a returned result.
- The resolveResourcesDir() JSDoc (from 52ae42e) describing its src/
layout fallback as "npm start running this file via tsx", which my
very next commit (bd81c93, restoring the build-first start script)
made inaccurate. Reworded to describe direct source execution
generically, independent of npm start.
* docs: clarify --authorized-origin-url CORS scope in --help text
The old text ('for CORS') implied this flag configures cross-origin
API access in every mode, but system CORS is only touched for
--profile spa. In --data, native, and webapp modes it only sets
application.oauthConfiguration.authorizedOriginURLs, a separate
application-level allowlist. Reworded to make the scope explicit.
* fix: add Content-Type to required CORS headers, dedupe error output
Content-Type is only CORS-safelisted for application/x-www-form-urlencoded,
multipart/form-data, or text/plain -- not application/json. A SPA sending
JSON would still fail preflight after this command reported CORS as
"configured", since Content-Type wasn't in the guaranteed header set.
Added it to REQUIRED_CORS_HEADERS and updated the comments that
described these as "DPoP-related" headers (Content-Type is about JSON
bodies not being safelisted, not DPoP specifically). Updated test
fixtures that previously hardcoded the old 3-header "fully compliant"
set, and the integration test's local REQUIRED_CORS_HEADERS constant,
so they continue to validate the correct full set. Verified via a full
local docker-based integration run.
Also fixed unwrapError() printing the same error message twice.
Direct, never-wrapped Errors (e.g. parseData()'s validation errors)
have no distinct .cause, so unwrapError() fell back to returning the
same Error object as rawError -- which errorAndExit()/reportError()
then printed a second time via its generic 'message' in error branch.
Reproduced this empirically before and after the fix. unwrapError()
now returns undefined in that case, while still preserving a
genuinely-wrapped error's distinct cause, or a rejection that was
never an Error at all (e.g. a raw ClientResponse-shaped object).
Added a regression test asserting rawError is undefined for a direct
validation error.
* test: exercise all resolveResourcesDir() branches, including dist/
The existing test imported src/commands/kickstart-install.js via tsx,
so __dirname was always .../src/commands for the whole test run --
meaning the dist-layout branch (and the error-throw path) had zero
coverage, despite the test's comment claiming both layouts were
covered.
Added a baseDir parameter to resolveResourcesDir() (defaulting to the
real __dirname, so production behavior is unchanged) so tests can
exercise all three outcomes -- dist found, src fallback found, neither
found -- against controlled, synthetic temp directories instead of
depending on the real repo's build state.
Also added a separate test that imports the actual compiled
dist/commands/kickstart-install.js and verifies resolveResourcesDir()
resolves correctly against the real build output, confirming the
copy-files build step actually produces a working dist/commands/resources
directory. Skips gracefully (not fails) when dist/ hasn't been built
yet, so test:unit still works without requiring a build first --
meaningful in CI, which always builds before testing.
* fix: resolve container ID via Compose, fix path encoding for spaces
Two previously-missed findings from the same review, neither acted on
across two review cycles -- not a deliberate decision, just missed.
CONTAINER_NAME hard-coded Docker Compose's default generated container
name ('{project}-{service}-{index}'). If COMPOSE_PROJECT_NAME is set,
the real container name differs, both docker inspect calls silently
fail (caught by empty catch blocks), and the bridge-IP fallback this
PR added is defeated without any visible error. Replaced with
resolveContainerId(), which resolves the real ID via
`docker compose ps -q fusionauth`, independent of naming conventions.
Verified end-to-end via a full local docker-based integration run --
the bridge-IP fallback message still appears correctly, confirming
the dynamic resolution works.
COMPOSE_DIR used new URL(...).pathname, which leaves special characters
like spaces percent-encoded (e.g. '%20') rather than decoding them --
not a valid filesystem path component. Verified empirically that
fileURLToPath() correctly decodes it instead. Also replaced the
`cd ${COMPOSE_DIR} && ...` string-concatenation pattern (5 call sites)
with execAsync(cmd, { cwd: COMPOSE_DIR }), avoiding shell-quoting
issues with the path entirely rather than just moving them around.
* fix: dedupe authorized origins, clean up test temp directories
Duplicate --authorized-origin-url values were only ever compared
against the pre-existing system CORS allowlist, not against each
other, so passing the same origin twice wrote a duplicate entry into
the system-wide CORS configuration. Deduped the supplied origins via
[...new Set(authorizedOrigins)] before filtering. Verified empirically
before/after, and added a regression test asserting the origin appears
exactly once in the PATCH payload.
Also fixed resolveResourcesDir()'s unit tests leaking temp directories
on every run -- mkTempDir() created a real directory under the OS temp
dir in each of the 4 tests but never removed any of them. Confirmed
this was a real, accumulating leak: found 16 leftover directories from
prior test runs still on disk. Now tracks created dirs and removes
them in afterEach; verified a fresh run leaves zero behind.
* fix: exit immediately on repeated termination signals during teardown
Once a SIGINT/SIGTERM listener is registered, Node no longer applies
its default "a second Ctrl+C just kills the process" behavior on its
own -- the listener has full responsibility. The early `return` on a
repeated signal while handlingTerminationSignal was already true meant
that if forceTeardown()'s `docker compose down -v` call hung (it has
no timeout, unlike every other network call in this file), every
subsequent Ctrl+C/SIGTERM was silently swallowed, leaving no way to
interrupt the process short of `kill -9` from another terminal.
Verified both the bug and the fix with a standalone repro harness
simulating a permanently-hung teardown: the old logic left the process
running indefinitely after a second SIGINT; the new logic force-exits
with the expected code (130/143) immediately. Also ran the full
integration suite to confirm no regression in normal (non-hung)
teardown.
* fix: show real error messages instead of "[object Object]"
Root cause: the FusionAuth SDK rejects with a ClientResponse instance,
which does NOT extend Error. All three wrapError() call sites in
application-create.ts built their message via
"e instanceof Error ? e.message : String(e)" -- since e is never
instanceof Error for a real SDK rejection, this always fell to
String(e), which for a plain class instance produces the literal
"[object Object]". Reproduced the exact reported repro (creating an
application with a duplicate name) character for character before
fixing, and confirmed the new test fails against the old code.
Added describeError() to utils.ts, reusing the existing
isClientResponse()/isErrors() guards to correctly unpack
fieldErrors/generalErrors, a nested network-level Error, or fall back
to "HTTP <statusCode>" -- instead of duplicating that logic locally.
Hardened isClientResponse()/isErrors() to not throw on null/undefined
input, now that they're used more broadly via describeError(). Used
describeError() at all three wrapError() sites.
Also replaced reportError()'s final fallback (JSON.stringify via
toJson) with util.inspect, which handles circular references and
non-JSON-serializable values gracefully instead of throwing or
silently dropping them, for genuinely unknown error shapes.
Added betaWarning() to application:create's action() (matching the
existing convention used by all kickstart:* commands), updated the
README's new Applications heading to "(beta)", and added blank-line
spacing before the success message and before the Next Steps box per
review feedback.
* fix: remove redundant error detail printed twice on CLI failure
f54f856 fixed "[object Object]" by baking describeError()'s full
detail into wrapError()'s message, which became ApplicationCreateResult
.error. But reportError() already does its own unwrapping of rawError
to print structured fieldErrors/generalErrors detail -- so once .error
also carried that same detail, the CLI printed it twice (once as the
flattened message, once via reportError's own per-field breakdown).
The redundancy was a symptom of putting a presentation-layer concern
(a fully flattened, human-readable string) into a data field. Reverted
wrapError()'s message construction to short, static, per-stage labels
("Error creating application", etc.) with no appended detail -- this
also means the original [object Object] bug can no longer occur by
construction, since the message no longer attempts to coerce the
rejection into a string at all. The real detail now flows through
rawError alone, and reportError()'s existing (unchanged) formatting
handles it correctly for both single- and multi-error cases, with no
new branching logic needed anywhere.
describeError() is now unused in production code (confirmed via
repo-wide search) -- removed it from utils.ts along with its tests.
Updated the one test that asserted on .error's flattened detail to
instead assert structurally on rawError.exception, which is more
robust and doesn't couple the test to any particular string-formatting
choice.
Verified against the real compiled CLI with both a single-field-error
and a multi-field-error mocked response: single case now prints
exactly one line of detail (previously two identical lines); multi
case correctly prints each field error on its own line, matching the
original pre-regression behavior.
---------
Co-authored-by: Mark Robustelli <137117976+mark-robustelli@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Andy Pai <8798244+andrewpai@users.noreply.github.com>
Co-authored-by: Andy Pai <8798244+andrewpai@users.noreply.github.com>
Co-authored-by: Andy Pai <8798244+andrewpai@users.noreply.github.com>
Contributor
Author
|
@andrewpai other than the comments with conversation, everything is resolved, including cleaning this up with commits from the next branch. Let's look at merging this into next-setup today to also get next-setup into parity with next. Can you give it another look? |
andrewpai
reviewed
Oct 9, 2026
| await assert.rejects(() => action(APP_ID, {...BASE_OPTIONS, prop: ["something"]})) | ||
| const result = await executeUpdateAction(APP_ID, {...BASE_OPTIONS, prop: ["something"]}) | ||
| await assert.equal(result.success, false) | ||
| // await assert.rejects(() => executeUpdateAction(APP_ID, {...BASE_OPTIONS, prop: ["something"]})) |
| .argument('<id>', "The FusionAuth Application ID to update") | ||
| .option('-d, --data <file>', "Apply changes from a named file of JSON that matches the API body for an application update (ignores other flags)") | ||
| .option('--redirect-url <redirectUrl>', 'Oauth2.0 Authorized URL') | ||
| .option('-p, --prop <prop...>', 'Updates a single property from the application --prop name="My New Name" or --prop oauthConfiguration.authorizedOriginURLs="http://localhost:9011" ') |
Contributor
There was a problem hiding this comment.
If you remove the ... from <...prop> then we try to run a map operation on something that is no longer an array and you get the error options.prop.map is not a function. I think we should be able to remove the ... and remove the mapping code since we'll no longer have multiple values on a single --prop option.
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
4 participants
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.
Preview version of the application:update