Skip to content

feat(core): extract config into opt-in middleware with layered resolution#155

Open
zrosenbauer wants to merge 1 commit intomainfrom
feat/config-middleware
Open

feat(core): extract config into opt-in middleware with layered resolution#155
zrosenbauer wants to merge 1 commit intomainfrom
feat/config-middleware

Conversation

@zrosenbauer
Copy link
Copy Markdown
Member

Summary

Closes #154

  • Extract config loading from core runtime into opt-in middleware (@kidd-cli/core/config)
  • Remove ctx.config, CliConfig, CliConfigOptions, ConfigType, and TConfig generic from core types
  • Add config() middleware factory with single-mode (cwd) and layered-mode (global > project > local deep merge)
  • Module augmentation adds ctx.config only when middleware is imported — keeping builds lean for CLIs that don't need config
  • Add ./config entrypoint to package.json and tsdown config

Breaking change

ctx.config is no longer available by default. CLI authors must use the config middleware:

import { config } from '@kidd-cli/core/config'
import type { ConfigType } from '@kidd-cli/core/config'

declare module '@kidd-cli/core' {
  interface CommandContext {
    readonly config: ConfigType<typeof configSchema>
  }
}

cli({
  middleware: [config({ schema: configSchema })],
})

For layered resolution (global > project > local):

config({ schema: configSchema, layers: true })

Test plan

  • pnpm typecheck passes
  • All 1060 tests pass (81 test files)
  • @kidd-cli/cli (downstream) typechecks clean
  • Formatting clean
  • Manual verification: importing @kidd-cli/core does NOT pull in c12
  • Manual verification: config middleware loads and validates config correctly

…tion (#154)

Move config loading out of the core runtime into `@kidd-cli/core/config`
middleware. Config is no longer baked into CommandContext — it is added
via module augmentation when the middleware is imported.

The new middleware supports:
- Single-mode: loads config from cwd (existing behavior)
- Layered mode: global > project > local resolution with deep merge

Co-Authored-By: Claude <noreply@anthropic.com>
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 4, 2026

🦋 Changeset detected

Latest commit: 7f264f6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@kidd-cli/core Minor
@kidd-cli/cli Patch

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

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 4, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
oss-kidd Ignored Ignored Apr 4, 2026 1:51am

Request Review

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 4, 2026

Merging this PR will not alter performance

✅ 2 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing feat/config-middleware (7f264f6) with main (da87a23)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 4, 2026

📝 Walkthrough

Walkthrough

This PR refactors the CLI's configuration system from a built-in, upfront-loading model to a middleware-based approach. The TConfig generic parameter is removed from core context types (CommandContext, ScreenContext), CLI options, and runtime signatures. A new config middleware module is added at packages/core/src/middleware/config/ that handles layered config resolution (global → project → local), deep merging, and validation via Zod schema. Config loading is no longer performed in createRuntime; instead, the middleware orchestrates it. All test utilities are updated to remove TConfig generics. A package export entry ./config is added to expose the new middleware publicly.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly describes the main change: extracting config into optional middleware with layered resolution support.
Description check ✅ Passed Description provides comprehensive overview of changes, breaking changes, usage examples, and test plan status.
Linked Issues check ✅ Passed PR comprehensively addresses #154 objectives: implements layered resolution (global > project > local), exposes layer metadata on ctx.raw.configLayers, supports deep merging, and makes it opt-in via middleware.
Out of Scope Changes check ✅ Passed All changes are in scope: core runtime/context/type removals, new config middleware with layered resolution, module augmentation, and package export additions.
Docstring Coverage ✅ Passed Docstring coverage is 92.31% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/config-middleware

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/core/src/test/types.ts (1)

13-24: ⚠️ Potential issue | 🟠 Major

createTestContext() / runHandler() no longer have a safe path for config-aware handlers.

After importing @kidd-cli/core/config, CommandContext is augmented with config, but Lines 13-24 and 80-83 no longer let tests seed that field. packages/core/src/test/context.ts, Lines 24-49, still builds the context through createContext(), and packages/core/src/context/create-context.ts, Lines 15-51, do not attach config. That means handler tests can typecheck against ctx.config and then read undefined at runtime. Please keep a config/decorated override path or add a middleware-aware test helper before removing the old config option.

Also applies to: 31-45, 80-83

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/test/types.ts` around lines 13 - 24, The TestContextOptions
interface (in types.ts) and the test helpers createTestContext() / runHandler()
lack a way to seed a config property, so tests that import `@kidd-cli/core/config`
get ctx.config === undefined at runtime; update TestContextOptions to include an
optional readonly config?: ConfigType (or a generic decorated field), and adjust
createTestContext(), runHandler(), and the test context builder in
packages/core/src/test/context.ts to pass that config value through into the
object created by createContext() (or add a middleware-aware override that
attaches config before returning the context) so handlers can safely read
ctx.config during tests; reference the symbols TestContextOptions,
createTestContext, runHandler, and createContext 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.

Inline comments:
In `@packages/core/src/middleware/config/config.ts`:
- Around line 24-25: The code returns early in the "no configs found" and
"merged-validation failure" paths without populating the layered-mode provenance
field, so ensure ctx.raw.configLayers is always assigned before any early
return: set ctx.raw.configLayers to the appropriate metadata (e.g., an empty
array or the partial layer list you computed) in the "no configs found" path and
right before returning on merged-validation failure so callers still receive
provenance; update the same behavior wherever configLayers may be skipped (the
other early-return branches that check layered mode) and keep the symbol
ctx.raw.configLayers as the single source of truth.
- Around line 65-69: The current checks conflate loadError with “no config” and
silently return an empty config; instead, change the logic around client.load()
so that loadError (or any validation/merged-schema error) is surfaced (throw or
propagate the error) rather than falling back to Object.freeze({}), and only
fall back to an empty config when there is no error and the result is genuinely
absent. Update the branches that use the pattern (loadError || !result) —
including the places that call client.load(), inspect loadError and result
separately, throw or pass the load/validation error up (or call the existing
error-handling path), and only call decorateContext(ctx, 'config',
Object.freeze({})) when result is undefined/null with no loadError.
- Around line 129-135: The code tries to defineProperty('configLayers') on a
frozen ctx.raw (created by createContext()), causing a TypeError; fix by either
adding configLayers to ctx.raw before createContext() freezes it, or stop
mutating ctx.raw and instead attach the metadata to an extensible object (for
example ctx.configLayers or ctx._configLayers) so you do not call
Object.defineProperty on a non-extensible object; update the logic around
createContext()/ctx.raw and the property access sites to read from the new
location (ctx.configLayers or the pre-freeze assignment) so JSDoc promise is
satisfied.

In `@packages/core/src/middleware/config/types.ts`:
- Line 36: Replace the inline import('zod').ZodTypeAny usage with a proper
top-level type import: add "import type { ZodTypeAny } from 'zod';" at the top
of the file and change the generic constraint in ConfigMiddlewareOptions<TSchema
extends import('zod').ZodTypeAny> to ConfigMiddlewareOptions<TSchema extends
ZodTypeAny>; also update any other occurrences in this file that use
import('zod').ZodTypeAny (the two additional spots flagged) to use the imported
ZodTypeAny instead.
- Around line 69-79: The current declaration exposes CommandContext.config as
DeepReadonly<Record<string, unknown>> which conflicts with consumer
redeclarations using ConfigType<typeof configSchema>; change the exported
augmentation surface to a registry pattern instead: add and export an interface
named ConfigContextRegistry (or similar) in the config types module and update
the CommandContext declaration to type config as
ConfigContextRegistry['config']; update the docs to instruct consumers to
augment ConfigContextRegistry (using ConfigType<typeof configSchema>) rather
than redeclaring CommandContext.config so consumer-specific typed configs will
merge cleanly with the library declaration (refer to symbols: CommandContext,
ConfigType, ConfigContextRegistry, configSchema).

In `@packages/core/src/types/utility.ts`:
- Around line 59-61: The inline import in the InferSchema conditional type
should be replaced with a type-only import to satisfy consistent-type-imports:
add a top-level type-only import "import type { ZodType } from 'zod';" and
update the conditional to use ZodType directly (i.e. change "TSchema extends
import('zod').ZodType<infer TOutput>" to "TSchema extends ZodType<infer
TOutput>"). Keep the fallback AnyRecord and ensure the import is type-only so no
runtime require is introduced.

---

Outside diff comments:
In `@packages/core/src/test/types.ts`:
- Around line 13-24: The TestContextOptions interface (in types.ts) and the test
helpers createTestContext() / runHandler() lack a way to seed a config property,
so tests that import `@kidd-cli/core/config` get ctx.config === undefined at
runtime; update TestContextOptions to include an optional readonly config?:
ConfigType (or a generic decorated field), and adjust createTestContext(),
runHandler(), and the test context builder in packages/core/src/test/context.ts
to pass that config value through into the object created by createContext() (or
add a middleware-aware override that attaches config before returning the
context) so handlers can safely read ctx.config during tests; reference the
symbols TestContextOptions, createTestContext, runHandler, and createContext
when making the change.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7ca997a0-39be-4381-a43e-05cc9a927c73

📥 Commits

Reviewing files that changed from the base of the PR and between da87a23 and 7f264f6.

⛔ Files ignored due to path filters (1)
  • .changeset/config-middleware.md is excluded by !.changeset/**
📒 Files selected for processing (25)
  • packages/core/package.json
  • packages/core/src/cli.test.ts
  • packages/core/src/cli.ts
  • packages/core/src/command.ts
  • packages/core/src/context/create-context.test.ts
  • packages/core/src/context/create-context.ts
  • packages/core/src/context/types.ts
  • packages/core/src/index.ts
  • packages/core/src/middleware/config/config.ts
  • packages/core/src/middleware/config/index.ts
  • packages/core/src/middleware/config/types.ts
  • packages/core/src/runtime/runtime.test.ts
  • packages/core/src/runtime/runtime.ts
  • packages/core/src/runtime/types.ts
  • packages/core/src/test/command.ts
  • packages/core/src/test/context.test.ts
  • packages/core/src/test/context.ts
  • packages/core/src/test/handler.ts
  • packages/core/src/test/middleware.ts
  • packages/core/src/test/types.ts
  • packages/core/src/types/cli.ts
  • packages/core/src/types/command.ts
  • packages/core/src/types/index.ts
  • packages/core/src/types/utility.ts
  • packages/core/tsdown.config.ts
💤 Files with no reviewable changes (6)
  • packages/core/src/cli.test.ts
  • packages/core/src/index.ts
  • packages/core/src/test/context.test.ts
  • packages/core/src/test/command.ts
  • packages/core/src/types/index.ts
  • packages/core/src/runtime/runtime.test.ts

Comment on lines +24 to +25
* When layered mode is enabled, per-layer metadata is available on `ctx.raw.configLayers`.
*
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

configLayers is skipped on the early-return paths.

The JSDoc says layered mode exposes ctx.raw.configLayers, but both the “no configs found” path and the merged-validation failure path return before that metadata is assigned. Consumers lose provenance exactly when they need it most.

Also applies to: 106-109, 122-124

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/middleware/config/config.ts` around lines 24 - 25, The code
returns early in the "no configs found" and "merged-validation failure" paths
without populating the layered-mode provenance field, so ensure
ctx.raw.configLayers is always assigned before any early return: set
ctx.raw.configLayers to the appropriate metadata (e.g., an empty array or the
partial layer list you computed) in the "no configs found" path and right before
returning on merged-validation failure so callers still receive provenance;
update the same behavior wherever configLayers may be skipped (the other
early-return branches that check layered mode) and keep the symbol
ctx.raw.configLayers as the single source of truth.

Comment on lines +65 to +69
const [loadError, result] = await client.load()

if (loadError || !result) {
decorateContext(ctx, 'config', Object.freeze({}))
return
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Load/validation failures are being treated as “no config found.”

These branches conflate loadError with !result, and merged-schema failures also fall back to {}. That silently ignores malformed config files instead of surfacing them, and in layered mode it can drop a broken higher-precedence layer from the merge with no user-visible failure.

Also applies to: 116-124, 150-154

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/middleware/config/config.ts` around lines 65 - 69, The
current checks conflate loadError with “no config” and silently return an empty
config; instead, change the logic around client.load() so that loadError (or any
validation/merged-schema error) is surfaced (throw or propagate the error)
rather than falling back to Object.freeze({}), and only fall back to an empty
config when there is no error and the result is genuinely absent. Update the
branches that use the pattern (loadError || !result) — including the places that
call client.load(), inspect loadError and result separately, throw or pass the
load/validation error up (or call the existing error-handling path), and only
call decorateContext(ctx, 'config', Object.freeze({})) when result is
undefined/null with no loadError.

Comment on lines +129 to +135
// Attach layer metadata to raw — use defineProperty to extend the frozen raw object
Object.defineProperty(ctx.raw, 'configLayers', {
configurable: false,
enumerable: true,
value: Object.freeze(layerResults),
writable: false,
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

fd config.ts | grep -E "packages/core"

Repository: joggrdocs/kidd

Length of output: 165


🏁 Script executed:

fd create-context.ts | grep -E "packages/core"

Repository: joggrdocs/kidd

Length of output: 101


🏁 Script executed:

cat -n packages/core/src/middleware/config/config.ts | sed -n '120,145p'

Repository: joggrdocs/kidd

Length of output: 907


🏁 Script executed:

cat -n packages/core/src/context/create-context.ts

Repository: joggrdocs/kidd

Length of output: 6388


🏁 Script executed:

rg "configLayers" packages/core/src --context 3

Repository: joggrdocs/kidd

Length of output: 1421


🏁 Script executed:

cat -n packages/core/src/middleware/config/config.ts | sed -n '80,136p'

Repository: joggrdocs/kidd

Length of output: 2371


🏁 Script executed:

python3 <<'EOF'
# Verify JavaScript behavior: Object.defineProperty on frozen object
import subprocess
result = subprocess.run(['node', '-e', '''
const obj = Object.freeze({ argv: [] });
try {
  Object.defineProperty(obj, "newProp", { 
    value: 123,
    writable: false,
    enumerable: true,
    configurable: false
  });
  console.log("SUCCESS: property added");
} catch (e) {
  console.log("ERROR:", e.message);
}
'''], capture_output=True, text=True)
print(result.stdout)
print(result.stderr)
EOF

Repository: joggrdocs/kidd

Length of output: 123


Object.defineProperty() cannot add configLayers to the frozen ctx.raw object.

createContext() freezes ctx.raw at line 107, making it non-extensible. Calling Object.defineProperty() at line 130 throws TypeError: Cannot define property configLayers, object is not extensible when valid layered config is found. The JSDoc at line 9 promises ctx.raw.configLayers in layered mode, but this implementation cannot fulfill it. Either populate the property before freezing raw, or attach the metadata to an extensible surface.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/middleware/config/config.ts` around lines 129 - 135, The
code tries to defineProperty('configLayers') on a frozen ctx.raw (created by
createContext()), causing a TypeError; fix by either adding configLayers to
ctx.raw before createContext() freezes it, or stop mutating ctx.raw and instead
attach the metadata to an extensible object (for example ctx.configLayers or
ctx._configLayers) so you do not call Object.defineProperty on a non-extensible
object; update the logic around createContext()/ctx.raw and the property access
sites to read from the new location (ctx.configLayers or the pre-freeze
assignment) so JSDoc promise is satisfied.

*
* @typeParam TSchema - Zod schema type used to validate the loaded config.
*/
export interface ConfigMiddlewareOptions<TSchema extends import('zod').ZodTypeAny> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use a proper type import for Zod here.

Lines 36, 81, and 82 currently fail the Code Standards check because consistent-type-imports forbids import() type annotations in this repo.

Suggested fix
+import type * as z from 'zod'
+
 import type { ConfigFormat } from '@/lib/config/types.js'
 import type { DeepReadonly } from '@/types/index.js'
@@
-export interface ConfigMiddlewareOptions<TSchema extends import('zod').ZodTypeAny> {
+export interface ConfigMiddlewareOptions<TSchema extends z.ZodTypeAny> {
@@
-export type ConfigType<TSchema extends import('zod').ZodType> = DeepReadonly<
-  import('zod').infer<TSchema>
->
+export type ConfigType<TSchema extends z.ZodType> = DeepReadonly<z.infer<TSchema>>

Also applies to: 81-82

🧰 Tools
🪛 GitHub Check: Code Standards & Tests

[failure] 36-36: typescript-eslint(consistent-type-imports)
import() type annotations are forbidden.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/middleware/config/types.ts` at line 36, Replace the inline
import('zod').ZodTypeAny usage with a proper top-level type import: add "import
type { ZodTypeAny } from 'zod';" at the top of the file and change the generic
constraint in ConfigMiddlewareOptions<TSchema extends import('zod').ZodTypeAny>
to ConfigMiddlewareOptions<TSchema extends ZodTypeAny>; also update any other
occurrences in this file that use import('zod').ZodTypeAny (the two additional
spots flagged) to use the imported ZodTypeAny instead.

Comment on lines +69 to +79
* Use this in a `declare module` block to type `ctx.config`:
*
* ```ts
* import type { ConfigType } from '@kidd-cli/core/config'
*
* declare module '@kidd-cli/core' {
* interface CommandContext {
* readonly config: ConfigType<typeof configSchema>
* }
* }
* ```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In TypeScript declaration merging, can two merged interfaces declare the same non-function property name with different but compatible types, or must the property types be identical?

💡 Result:

No, in TypeScript declaration merging, two merged interfaces cannot declare the same non-function property name with different types, even if compatible. The types must be identical, or the compiler issues an error.

Citations:


🏁 Script executed:

cat -n packages/core/src/middleware/config/types.ts | head -100

Repository: joggrdocs/kidd

Length of output: 4021


🏁 Script executed:

rg -i "contextregistry|ConfigContextRegistry" --type ts --type tsx

Repository: joggrdocs/kidd

Length of output: 85


🏁 Script executed:

rg -i "contextregistry|ConfigContextRegistry" -t ts

Repository: joggrdocs/kidd

Length of output: 40


🏁 Script executed:

rg "declare module '@kidd-cli/core'" -A 5 -t ts

Repository: joggrdocs/kidd

Length of output: 3702


🏁 Script executed:

rg "CommandContext" -A 3 examples/advanced/src/index.ts -t ts

Repository: joggrdocs/kidd

Length of output: 120


🏁 Script executed:

cat -n examples/advanced/src/index.ts | grep -A 10 -B 10 "CliConfig"

Repository: joggrdocs/kidd

Length of output: 625


🏁 Script executed:

rg "interface CliConfig|export interface CliConfig|type CliConfig" -t ts

Repository: joggrdocs/kidd

Length of output: 153


🏁 Script executed:

rg "interface CliConfig|export interface CliConfig" packages/core -t ts

Repository: joggrdocs/kidd

Length of output: 40


🏁 Script executed:

rg "CliConfig" packages/core/src/context -t ts -B 2 -A 2

Repository: joggrdocs/kidd

Length of output: 40


🏁 Script executed:

rg "export interface CliConfig|declare.*CliConfig" packages/core/src -t ts

Repository: joggrdocs/kidd

Length of output: 40


🏁 Script executed:

cat -n packages/core/src/context/types.ts | head -80

Repository: joggrdocs/kidd

Length of output: 2916


🏁 Script executed:

cat -n packages/core/src/context/types.ts | sed -n '100,150p'

Repository: joggrdocs/kidd

Length of output: 1483


🏁 Script executed:

cat -n packages/core/src/context/types.ts | tail -100

Repository: joggrdocs/kidd

Length of output: 3962


Documentation augmentation path conflicts with actual declaration type.

Lines 71-79 instruct consumers to redeclare CommandContext.config using ConfigType<typeof schema>, but lines 89-96 declare the property as DeepReadonly<Record<string, unknown>>. TypeScript declaration merging requires identical property types. Following the documented path will fail with a type error because ConfigType<TSchema> produces specific typed records (e.g. DeepReadonly<{name: string}>) which do not match the generic DeepReadonly<Record<string, unknown>> declaration.

The suggested registry pattern fixes this by making the declaration type flexible:

+export interface ConfigContextRegistry {}
+
+export type ResolvedConfig =
+  keyof ConfigContextRegistry extends never
+    ? DeepReadonly<Record<string, unknown>>
+    : ConfigContextRegistry[keyof ConfigContextRegistry]
+
 declare module '@kidd-cli/core' {
   interface CommandContext {
-    readonly config: DeepReadonly<Record<string, unknown>>
+    readonly config: ResolvedConfig
   }
 }

Consumers then augment ConfigContextRegistry from @kidd-cli/core/config instead of redeclaring CommandContext.config, making the documented pattern actually work.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/middleware/config/types.ts` around lines 69 - 79, The
current declaration exposes CommandContext.config as DeepReadonly<Record<string,
unknown>> which conflicts with consumer redeclarations using ConfigType<typeof
configSchema>; change the exported augmentation surface to a registry pattern
instead: add and export an interface named ConfigContextRegistry (or similar) in
the config types module and update the CommandContext declaration to type config
as ConfigContextRegistry['config']; update the docs to instruct consumers to
augment ConfigContextRegistry (using ConfigType<typeof configSchema>) rather
than redeclaring CommandContext.config so consumer-specific typed configs will
merge cleanly with the library declaration (refer to symbols: CommandContext,
ConfigType, ConfigContextRegistry, configSchema).

Comment on lines +59 to +61
export type InferSchema<TSchema> = TSchema extends import('zod').ZodType<infer TOutput>
? TOutput
: AnyRecord
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n packages/core/src/types/utility.ts | sed -n '50,70p'

Repository: joggrdocs/kidd

Length of output: 482


🏁 Script executed:

head -70 packages/core/src/types/utility.ts | tail -20

Repository: joggrdocs/kidd

Length of output: 620


🏁 Script executed:

rg "import.*zod" packages/core/src/types/utility.ts -i

Repository: joggrdocs/kidd

Length of output: 145


🏁 Script executed:

head -30 packages/core/src/types/utility.ts

Repository: joggrdocs/kidd

Length of output: 1099


🏁 Script executed:

rg "import.*from.*zod" packages/core/src/types/utility.ts

Repository: joggrdocs/kidd

Length of output: 40


🏁 Script executed:

rg "from\s+['\"]zod['\"]" packages/core/src --type ts -A 2 | head -30

Repository: joggrdocs/kidd

Length of output: 1826


🏁 Script executed:

rg "ZodType" packages/core/src --type ts | head -10

Repository: joggrdocs/kidd

Length of output: 976


🏁 Script executed:

# Check if ZodType is exported from zod
npm list zod 2>/dev/null || echo "checking package.json for zod version"

Repository: joggrdocs/kidd

Length of output: 143


🏁 Script executed:

# Verify the eslint rule configuration
cat -n .eslintrc.json 2>/dev/null || cat -n .eslintrc.js 2>/dev/null || echo "no eslint config found"

Repository: joggrdocs/kidd

Length of output: 80


🏁 Script executed:

# Check if there's a tsconfig for consistent-type-imports
find . -name "tsconfig.json" -o -name ".eslintrc*" | head -5 | xargs grep -l "consistent-type-imports" 2>/dev/null || echo "Searching for rule config"

Repository: joggrdocs/kidd

Length of output: 83


🏁 Script executed:

rg "consistent-type-imports" . --type json --type js | head -5

Repository: joggrdocs/kidd

Length of output: 125


🏁 Script executed:

cat .oxlintrc.json

Repository: joggrdocs/kidd

Length of output: 4233


🏁 Script executed:

# Check if ZodType exists and can be imported
rg "ZodType" packages/core/src/stories/schema.ts -B 2 -A 2 | head -20

Repository: joggrdocs/kidd

Length of output: 763


🏁 Script executed:

# Verify the exact nature of the rule violation by checking TypeScript documentation
python3 << 'EOF'
# The consistent-type-imports rule flags:
# 1. import() in type position without 'type' keyword - these should be type-only imports
# 2. Mixing type and value imports from same module
print("Rule analysis:")
print("- import('zod').ZodType in conditional type (type position)")
print("- This should be flagged by consistent-type-imports rule")
print("- Solution: Add 'import type { ZodType } from \"zod\"'")
print("- Then use 'ZodType' directly in the extends clause")
EOF

Repository: joggrdocs/kidd

Length of output: 294


Replace inline import('zod') with a type-only import.

Line 59 violates typescript/consistent-type-imports (error-level rule in .oxlintrc.json). Use a static type-only import and reference ZodType directly.

Fix
+import type { ZodType } from 'zod'
+
 // ---------------------------------------------------------------------------
 // Generic type utilities
 // ---------------------------------------------------------------------------
@@
-export type InferSchema<TSchema> = TSchema extends import('zod').ZodType<infer TOutput>
+export type InferSchema<TSchema> = TSchema extends ZodType<infer TOutput>
   ? TOutput
   : AnyRecord
🧰 Tools
🪛 GitHub Check: Code Standards & Tests

[failure] 59-59: typescript-eslint(consistent-type-imports)
import() type annotations are forbidden.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/types/utility.ts` around lines 59 - 61, The inline import
in the InferSchema conditional type should be replaced with a type-only import
to satisfy consistent-type-imports: add a top-level type-only import "import
type { ZodType } from 'zod';" and update the conditional to use ZodType directly
(i.e. change "TSchema extends import('zod').ZodType<infer TOutput>" to "TSchema
extends ZodType<infer TOutput>"). Keep the fallback AnyRecord and ensure the
import is type-only so no runtime require is introduced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(config): ordered config resolution with global > project > local merging

1 participant