feat(log): expand --verbose diagnostics and surface silent env fallback#183
Merged
feat(log): expand --verbose diagnostics and surface silent env fallback#183
Conversation
…tials When running with --verbose, log.debug() calls now surface: - plapi: every outgoing request URL and method, plus status + response body on error - credentials: which keyring account or fallback file is used for each token lookup - config: the config file path on every read/write - env: the active environment name and resolved platformApiUrl on startup
Contributor
Author
|
Stack: wyattjoh/roomy-arrhinceratops Part of a stacked PR chain. Do not merge manually. |
🦋 Changeset detectedLatest commit: b4d7ed0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Author
|
!snapshot |
📝 WalkthroughWalkthroughThe PR adds extensive debug logging and a new Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Contributor
Snapshot publishednpm install -g clerk@0.8.6-snapshot.v20260420154631
|
- Warn (without --verbose) when the saved environment is not available in the current binary and the CLI silently falls back to production; this surfaces the root cause of 403s on snapshot builds that lack non-production profiles - Always log the active environment in verbose mode, including the production default case where setCurrentEnv is never called - Include env name and target URL in the getAuthToken() debug lines so credential source and request target are visible in one place - Log the profiles source (compile-time, file, or defaults) once per process so it is clear why certain environments are unavailable
Convert run/isPublished/publish in scripts/lib/npm.ts from Bun.spawnSync to async Bun.spawn, then publish the 8 platform packages concurrently via Promise.all. The wrapper package is still published after all platform packages complete.
Contributor
Author
|
!snapshot |
Contributor
Snapshot publishednpm install -g clerk@0.8.6-snapshot.v20260420160518
|
…mid-publish The wrapper publish() call was never awaited. This was harmless when publish was synchronous but became a bug after the parallel publish refactor made it async: the finally block restored the original package.json (with private: true and no optionalDependencies) before npm publish could read the mutated file, so the wrapper clerk package silently failed to publish. Platform packages were unaffected since they use a generated package.json on disk.
Contributor
Author
|
!snapshot |
Contributor
Snapshot publishednpm install -g clerk@0.8.6-snapshot.v20260420162038
|
Switch the changeset prereleaseTemplate from {tag}.v{datetime} to
{tag}.{commit}, and rewrite the full 40-char SHA changesets emits to
the 7-char short form so versions stay readable (e.g.
0.8.6-snapshot.a1b2c3d).
Because commit-based versions are deterministic per commit, re-running
!snapshot on the same commit would produce a version npm rejects as a
duplicate. Bail out of the snapshot job up front by calling
isPublished("clerk", version); the build/sign/test/publish jobs all
depend on this step so they skip entirely instead of burning ~30
minutes of runner time for nothing.
To re-publish the same commit intentionally, use a different tag name
(e.g. `!snapshot retry`), which produces `0.8.6-retry.a1b2c3d`.
…ected The snapshot-build job was the only build-binaries.yml caller missing secrets: inherit, so secrets.ENV_PROFILES was undefined in the reusable workflow. build.ts then skipped the --define CLI_ENV_PROFILES=... step and the resulting binary fell back to hardcoded defaults (production only), which is why `switch-env staging` errors with "Unknown environment" on snapshot installs even though canary works. Matches the pattern already in place for the stable build and canary-build jobs.
Contributor
Author
|
!snapshot |
Contributor
Snapshot publishednpm install -g clerk@0.8.6-snapshot.640414d
|
rafa-thayto
reviewed
Apr 20, 2026
Add lib/fetch.ts exporting loggedFetch(url, { tag, ...init }) which
emits a "<tag>: METHOD url" debug line before the request and, on
non-ok responses, a "<tag>: status METHOD url — body" debug line
(body read from a cloned response so the caller retains ownership).
Migrate every call site that was duplicating this pattern:
- plapi.ts: the six endpoint functions now go through a local
plapiFetch wrapper that adds Bearer auth + Accept headers on top
of loggedFetch. Each public function shrinks to ~4 lines.
- commands/api/bapi.ts: the single bapiRequest swaps fetch for
loggedFetch with tag="bapi".
- lib/token-exchange.ts: both OAuth calls use tag="oauth".
- lib/update-check.ts: fetchLatestVersion uses tag="update-check".
Mock fixup: several test files were detecting mutating requests via
`init?.method ? mutating : fetched`. plapiFetch now always sets
method explicitly (including "GET"), so the truthy check no longer
distinguishes GET from PATCH/PUT. Update the predicates to check
`init.method !== "GET"` explicitly.
Document the pattern in .claude/rules/debug-logging.md (new) with a
pointer from the existing logging.md rule. Future HTTP call sites
must go through loggedFetch; inline log.debug duplicates at call
sites are no longer needed or desired.
…rk, runners Apply the debug-logging rule to the remaining non-HTTP subsystems so --verbose tells the full story when a command fails outside the network path. - auth-server: listening port, callback outcome (success, state mismatch, no code, OAuth error, timeout) - git: cached single-line summary of getGitRepoInfo (toplevel, commonDir, remote) on success; "not a git repository" on failure - autolink: detected-key count + sources, match result (app id and name), no-match branch. Downgrade the listApplications failure log from log.error to log.debug — autolink is a best-effort fallback and shouldn't surface errors to the user; the caller proceeds to the interactive picker. - framework: which dep matched or why nothing matched - runners: detected runners on PATH; yarn dlx probe outcome
- Delete the redundant empty changeset (.changeset/tired-pillows-divide.md).
roomy-arrhinceratops.md already bumps clerk for this branch, and scripts/**
is changeset-exempt, so the empty file was pure noise.
- Make the short-sha substitution regex in scripts/snapshot.ts word-bounded
(\b[a-f0-9]{40}\b) so it stays correct if prereleaseTemplate is later
changed to append a suffix after the commit SHA.
- Hoist profilesSourceLogged next to currentEnvName in lib/environment.ts so
both module-level flags sit above the functions that read them, without
relying on let-hoisting.
…ader coverage The original summary only named the four files touched by the first commit (plapi, credential-store, config, environment). The branch has since grown to include an info-level fallback warning, HTTP-wide loggedFetch migration, and debug coverage for the auth server, git helpers, autolink, framework detection, and runner probing. Rewrite the summary so the changelog entry reflects what actually ships.
wyattjoh
changed the title
Contributor
Author
|
!snapshot |
Contributor
Snapshot publishednpm install -g clerk@0.8.6-snapshot.b4d7ed0
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli-core/src/cli-program.ts`:
- Around line 76-85: The fallback branch logs that it’s falling back to
production but never updates the module-scoped environment, leaving a stale env
active; call setCurrentEnv("production") when envName is invalid (inside the
else path) so the active environment state is updated, then keep the existing
log.debug that uses getPlapiBaseUrl() and list available envs via
getAvailableEnvs() to preserve diagnostic messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ff0da1f5-bbda-4d61-86c8-e50b16311a17
📒 Files selected for processing (25)
.changeset/config.json.changeset/roomy-arrhinceratops.md.claude/rules/debug-logging.md.claude/rules/logging.md.github/workflows/release.ymlpackages/cli-core/src/cli-program.tspackages/cli-core/src/commands/api/bapi.tspackages/cli-core/src/commands/config/push.test.tspackages/cli-core/src/lib/auth-server.tspackages/cli-core/src/lib/autolink.tspackages/cli-core/src/lib/environment.tspackages/cli-core/src/lib/fetch.tspackages/cli-core/src/lib/framework.tspackages/cli-core/src/lib/git.tspackages/cli-core/src/lib/plapi.tspackages/cli-core/src/lib/runners.tspackages/cli-core/src/lib/token-exchange.tspackages/cli-core/src/lib/update-check.tspackages/cli-core/src/test/integration/config-management.test.tspackages/cli-core/src/test/integration/config-put.test.tspackages/cli-core/src/test/integration/dry-run.test.tsscripts/check-release.tsscripts/lib/npm.tsscripts/releaser.tsscripts/snapshot.ts
✅ Files skipped from review due to trivial changes (5)
- .changeset/config.json
- .claude/rules/logging.md
- packages/cli-core/src/lib/git.ts
- packages/cli-core/src/test/integration/config-put.test.ts
- packages/cli-core/src/lib/framework.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .changeset/roomy-arrhinceratops.md
- packages/cli-core/src/lib/plapi.ts
Comment on lines
+76
to
85
| setCurrentEnv(envName); // logs env + platformApiUrl | ||
| } else { | ||
| if (envName) { | ||
| log.warn( | ||
| `Saved environment "${envName}" is not available in this binary. Falling back to production.`, | ||
| ); | ||
| log.warn(`Available environments: ${getAvailableEnvs().join(", ")}`); | ||
| } | ||
| log.debug(`env: active environment is "production" (platformApiUrl=${getPlapiBaseUrl()})`); | ||
| } |
There was a problem hiding this comment.
Fallback branch doesn’t actually set production as active env.
When envName is invalid, this path only logs fallback but never updates the active environment state. Since environment state is module-scoped and mutated via setCurrentEnv(), a reused process can keep a stale non-production env from a previous invocation.
Suggested fix
- if (envName && isValidEnv(envName)) {
- setCurrentEnv(envName); // logs env + platformApiUrl
- } else {
+ if (envName && isValidEnv(envName)) {
+ setCurrentEnv(envName); // logs env + platformApiUrl
+ } else {
if (envName) {
log.warn(
`Saved environment "${envName}" is not available in this binary. Falling back to production.`,
);
log.warn(`Available environments: ${getAvailableEnvs().join(", ")}`);
}
- log.debug(`env: active environment is "production" (platformApiUrl=${getPlapiBaseUrl()})`);
+ setCurrentEnv("production"); // logs env + platformApiUrl
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| setCurrentEnv(envName); // logs env + platformApiUrl | |
| } else { | |
| if (envName) { | |
| log.warn( | |
| `Saved environment "${envName}" is not available in this binary. Falling back to production.`, | |
| ); | |
| log.warn(`Available environments: ${getAvailableEnvs().join(", ")}`); | |
| } | |
| log.debug(`env: active environment is "production" (platformApiUrl=${getPlapiBaseUrl()})`); | |
| } | |
| setCurrentEnv(envName); // logs env + platformApiUrl | |
| } else { | |
| if (envName) { | |
| log.warn( | |
| `Saved environment "${envName}" is not available in this binary. Falling back to production.`, | |
| ); | |
| log.warn(`Available environments: ${getAvailableEnvs().join(", ")}`); | |
| } | |
| setCurrentEnv("production"); // logs env + platformApiUrl | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-core/src/cli-program.ts` around lines 76 - 85, The fallback
branch logs that it’s falling back to production but never updates the
module-scoped environment, leaving a stale env active; call
setCurrentEnv("production") when envName is invalid (inside the else path) so
the active environment state is updated, then keep the existing log.debug that
uses getPlapiBaseUrl() and list available envs via getAvailableEnvs() to
preserve diagnostic messages.
rafa-thayto
approved these changes
Apr 20, 2026
rafa-thayto
approved these changes
Apr 20, 2026
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
2 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.
Summary
Expand
--verbosediagnostics across the CLI so network, credential, and config failures come with enough context to diagnose in one run. Also warns users when a saved environment is not available in the current binary (previously a silent fallback to production) and cleans up a few snapshot release rough edges surfaced along the way.What's new
--verbosecoverage: every outbound HTTP call logs URL, method, status, and response body on error; credential store, config I/O, environment resolution, OAuth callback, git detection, framework detection, autolink, and package-manager runner probing all gain debug lines. All HTTP now goes through a sharedloggedFetchhelper, codified in a new.claude/rules/entry so future call sites can't forget the pattern.0.8.6-snapshot.a1b2c3d) instead of a datetime; the job refuses to re-publish an already-published commit (use!snapshot <tag>to force a re-run); snapshot builds now inherit the env-profile secrets the same way canary and stable do, soswitch-env stagingworks on a snapshot install; platform packages publish in parallel, wrapper still follows sequentially.Test plan
switch-env staging+ an API call on the final snapshot install