fix(packages/core): add DirsConfig and fix auth dir mismatch#87
Merged
zrosenbauer merged 6 commits intomainfrom Mar 18, 2026
Merged
fix(packages/core): add DirsConfig and fix auth dir mismatch#87zrosenbauer merged 6 commits intomainfrom
zrosenbauer merged 6 commits intomainfrom
Conversation
…login/credential
Auth `login()` and `logout()` hardcoded the store directory to `.<cliName>`,
while `credential()` (passive reads) respected `auth.file({ dirName })` overrides.
This caused tokens written during login to be invisible to credential lookups when
custom dir names were configured.
Introduces `DirsConfig` on `cli()` with separate `local` and `global` directory
names that flow through `ctx.meta.dirs` into the auth subsystem. Auth-level
`dirName` overrides are also supported via `auth({ dirName })`.
- Add `DirsConfig` / `ResolvedDirs` types to `packages/core/src/types/cli.ts`
- Flow `dirs` through runtime → context → auth middleware
- Fix `resolveFromFile` to accept separate `localDirName` / `globalDirName`
- Fix `login()` / `logout()` to use `dirs.global` from context
- Add auth-level `dirName` override in `auth({ dirName })`
- Update all 27 affected test files
Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 884dcf3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
Co-Authored-By: Claude <noreply@anthropic.com>
…lic type exports
Replace `auth({ dirName })` with `auth({ dirs })` to match the DirsConfig
pattern used by `cli()`. Auth dirs now partially merge onto ctx.meta.dirs
instead of replacing both fields with a single string.
Remove DirsConfig and ResolvedDirs from the public API surface — they are
inferrable from the cli() parameter type and don't need explicit export.
Co-Authored-By: Claude <noreply@anthropic.com>
- cli.test.ts: default dirs, custom dirs, partial global/local overrides - auth.test.ts: auth-level dirs override smoke tests - file.test.ts: separate local/global dir resolution, short-circuit behavior - chain.test.ts: dirs passthrough to resolveFromFile Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds configurable directory support for file-backed stores. Two new public types, Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/core/src/cli.ts`:
- Around line 344-356: resolveDirs currently treats only null/undefined as
missing and accepts empty strings, causing config/auth to be written to
project/home roots; update resolveDirs to validate DirsConfig using Zod so
global/local must be non-empty strings: create a Zod schema (e.g., z.object({
global: z.string().min(1).optional(), local: z.string().min(1).optional() }))
and parse/ safeParse the incoming dirs; when a field is missing or fails
validation (empty string), fall back to defaultDir for that field; reference the
resolveDirs function and types DirsConfig/ResolvedDirs when applying this
change.
In `@packages/core/src/context/create-context.ts`:
- Around line 55-57: The code is aliasing options.meta.dirs into ctxMeta (and
thus ctx.meta.dirs), which allows mutations to bleed across contexts because
createRuntime() reuses the same dirs object for every createContext() call; to
fix, stop referencing the runtime dirs by reference and create a fresh copy for
ctxMeta.dirs when building the Meta object in create-context.ts (i.e., copy
options.meta.dirs into a new object instead of assigning it directly) so
downstream mutations don't affect other contexts; check createContext(),
ctxMeta, Meta, and usages of ctx.meta.dirs to ensure no shared mutable object is
returned.
In `@packages/core/src/middleware/auth/auth.ts`:
- Around line 250-267: The resolveAuthDirs function uses an if/else with two
branches; replace that conditional with a ts-pattern match expression (using
match from 'ts-pattern') to return { global: dirName, local: dirName } when
dirName is defined and metaDirs otherwise, keeping the same return type; update
imports to include match from 'ts-pattern' and ensure the pattern checks for
dirName being undefined vs defined so behavior and types are preserved in
resolveAuthDirs.
In `@packages/core/src/middleware/auth/context.ts`:
- Around line 99-100: The login/logout behavior currently only writes/removes
from the global store (createStore with dirs.global and DEFAULT_AUTH_FILENAME),
while file-based resolution (auth/strategies/file.ts) prefers the local store
first, leaving a stale local credential active; update the logic in the
functions that call createStore/save/remove (the login() and logout() flows in
context.ts around DEFAULT_AUTH_FILENAME) to either (A) resolve which store is
effective (check local then global like file.ts) and persist/remove the
credential in that effective scope, or (B) perform the operation in both scopes
(createStore for dirs.local and dirs.global) so local credentials are
overwritten/cleared as well; ensure credential() and authenticated() continue to
read using the same resolution order as the strategy implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3fa170bb-b523-41a9-8b35-d34599cb59e3
⛔ Files ignored due to path filters (1)
.changeset/fix-auth-dirs.mdis excluded by!.changeset/**
📒 Files selected for processing (27)
packages/core/src/cli.tspackages/core/src/compose.test.tspackages/core/src/context/create-context.test.tspackages/core/src/context/create-context.tspackages/core/src/context/decorate.test.tspackages/core/src/context/types.tspackages/core/src/index.tspackages/core/src/middleware/auth/auth-http-chain.test.tspackages/core/src/middleware/auth/auth.test.tspackages/core/src/middleware/auth/auth.tspackages/core/src/middleware/auth/chain.test.tspackages/core/src/middleware/auth/chain.tspackages/core/src/middleware/auth/context.test.tspackages/core/src/middleware/auth/context.tspackages/core/src/middleware/auth/strategies/file.test.tspackages/core/src/middleware/auth/strategies/file.tspackages/core/src/middleware/auth/types.tspackages/core/src/middleware/http/http.test.tspackages/core/src/middleware/icons/icons.test.tspackages/core/src/middleware/typed-middleware.test.tspackages/core/src/runtime/runtime.test.tspackages/core/src/runtime/runtime.tspackages/core/src/runtime/types.tspackages/core/src/test/context.tspackages/core/src/test/types.tspackages/core/src/types/cli.tspackages/core/src/types/index.ts
Writes (login/logout) always target the global (home) directory so credentials are user-scoped. Reads (credential/file strategy) check local → global, allowing project-level overrides. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/core/src/middleware/auth/context.ts (1)
99-103:⚠️ Potential issue | 🟠 Major
login()/logout()can leave the effective credential unchanged when local auth exists.At Line 102 and Line 123, writes/deletes only target
dirs.global, but credential resolution is local → global. A stale local credential can remain authoritative, sologin()/logout()can report success whilecredential()/authenticated()still reflect local state.Proposed fix (align persistence scope with resolution precedence)
- const store = createStore({ dirName: dirs.global }) - const [saveError] = store.save(DEFAULT_AUTH_FILENAME, validatedCredential) + const targetDirs = dirs.local === dirs.global ? [dirs.global] : [dirs.local, dirs.global] + const saveError = targetDirs + .map((dirName) => createStore({ dirName }).save(DEFAULT_AUTH_FILENAME, validatedCredential)[0]) + .find((error): error is Error => error !== null) ... - const store = createStore({ dirName: dirs.global }) - const [removeError, filePath] = store.remove(DEFAULT_AUTH_FILENAME) + const targetDirs = dirs.local === dirs.global ? [dirs.global] : [dirs.local, dirs.global] + const removeResults = targetDirs.map((dirName) => + createStore({ dirName }).remove(DEFAULT_AUTH_FILENAME) + ) + const removeError = removeResults + .map(([error]) => error) + .find((error): error is Error => error !== null) + const filePath = removeResults.find(([, resolvedPath]) => resolvedPath.length > 0)?.[1] ?? ''Also applies to: 122-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/middleware/auth/context.ts` around lines 99 - 103, The login/logout persistence currently only mutates the global store (createStore({ dirName: dirs.global }) and store.save/delete) while credential resolution prefers local → global, so a stale local credential can remain active; update the login/logout logic to perform the same operation in the local scope first and then the global scope (i.e., createStore({ dirName: dirs.local }) then createStore({ dirName: dirs.global }) and save/delete DEFAULT_AUTH_FILENAME in both) so that credential() / authenticated() reflect the effective credential after login/logout; ensure you reference createStore, DEFAULT_AUTH_FILENAME, dirs.local, dirs.global, and the credential()/authenticated() code paths when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/core/src/middleware/auth/context.ts`:
- Around line 99-103: The login/logout persistence currently only mutates the
global store (createStore({ dirName: dirs.global }) and store.save/delete) while
credential resolution prefers local → global, so a stale local credential can
remain active; update the login/logout logic to perform the same operation in
the local scope first and then the global scope (i.e., createStore({ dirName:
dirs.local }) then createStore({ dirName: dirs.global }) and save/delete
DEFAULT_AUTH_FILENAME in both) so that credential() / authenticated() reflect
the effective credential after login/logout; ensure you reference createStore,
DEFAULT_AUTH_FILENAME, dirs.local, dirs.global, and the
credential()/authenticated() code paths when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 544c803e-5324-4330-add2-c9dcad93b756
📒 Files selected for processing (1)
packages/core/src/middleware/auth/context.ts
- Guard against empty/whitespace-only dir strings in resolveDirs() - Freeze dirs object in createContext() to prevent mutation - Clarify logout() intentionally targets global only with expanded comment - Add test for empty string dirs fallback Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Mar 24, 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
1 participant
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
login()/logout()hardcoded the store directory to.<cliName>, whilecredential()respectedauth.file({ dirName })overrides. Tokens written during login were invisible to credential lookups when custom dir names were configured.DirsConfigoncli()with separatelocalandglobaldirectory names (both default to.<name>), flowing throughctx.meta.dirsinto the auth subsystem.auth({ dirName: '.custom' })overrides both local and global dirs for auth operations.API
Files changed
types/cli.ts— newDirsConfig/ResolvedDirstypescli.ts—resolveDirs()helper, passes dirs to runtimecontext/create-context.ts/context/types.ts—dirsonMetamiddleware/auth/auth.ts—resolveAuthDirs(), passes dirs to context + chainmiddleware/auth/context.ts—login()/logout()usedirs.globalmiddleware/auth/chain.ts— strategy chain receives dirsmiddleware/auth/strategies/file.ts— separatelocalDirName/globalDirNamemiddleware/auth/types.ts—dirNameoverride onAuthOptionsTest plan
pnpm typecheckpasses (0 errors)pnpm lintpasses (0 errors, 0 warnings)pnpm formatpassespnpm test --filter=@kidd-cli/corepasses (741/741 tests)