Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ This file provides guidance to coding agents when working with code in this repo
- Run `pnpm lint` to lint the code using Biome
- Run `pnpm format` to auto-fix formatting issues with Biome
- Run `pnpm type` to check TypeScript types
- Run `pnpm spec` to run only the Vitest tests
- Run `pnpm unit` to run only the Vitest tests
- Run `pnpm exec vitest run path/to/test.ts` to run a single test

## Formats
Expand All @@ -34,7 +34,7 @@ This file provides guidance to coding agents when working with code in this repo

## Specs

- Place unit tests in `<module>.spec.ts` files and don't add useless comments like "Arrange", "Act", "Assert"
- Place unit tests in `<module>.unit.ts` files and don't add useless comments like "Arrange", "Act", "Assert"

## Docs

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"lefthook": "2.1.1",
"npm-check-updates": "18.0.1",
"semantic-release": "24.2.9",
"tempy": "3.2.0",
"typescript": "5.9.2",
"vitest": "4.0.16",
"vitest-polly": "1.4.0"
Expand Down
5 changes: 4 additions & 1 deletion pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pnpm-workspace.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
dangerouslyAllowAllBuilds: true
packages:
- uptask
- terminal
- website
File renamed without changes.
146 changes: 146 additions & 0 deletions terminal/actions/command/create.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import { Command } from "commander"
import { helpConfiguration } from "../../helpers/program.ts"
import type { Function } from "../../models/function.ts"
import type { Parameter } from "../../models/parameter.ts"

/**
* Create a Commander command from a Function, wiring options to dynamic import and execution.
*/
export function createCommand(func: Function): Command {
const cmd = new Command(func.name).configureHelp(helpConfiguration)
if (func.description) cmd.description(func.description)

const argumentParams: typeof func.parameters = []
let hasVariadic = false

for (const [i, param] of func.parameters.entries()) {
if (i === 0 && (param.type === "string[]" || param.type === "number[]")) {
hasVariadic = true
argumentParams.push(param)
const bracket = param.required
? `<${param.name}...>`
: `[${param.name}...]`
cmd.argument(bracket, param.description || "")
} else if (
!hasVariadic &&
(param.type === "string" || param.type === "number") &&
param.required
) {
argumentParams.push(param)
cmd.argument(`<${param.name}>`, param.description || "")
} else if (param.type === "object" && param.properties?.length) {
registerObjectOptions(cmd, param.properties)
} else {
registerOption(cmd, param)
}
}

cmd.action(async (...actionArgs: unknown[]) => {
const options = actionArgs.at(-2) as Record<string, unknown>
const positionalValues = actionArgs.slice(0, argumentParams.length)
const args = func.parameters.map(param => {
const argIndex = argumentParams.indexOf(param)
if (argIndex !== -1) {
const val = positionalValues[argIndex]
if (param.type === "number") return Number(val)
if (param.type === "number[]")
return (val as unknown[]).map(v => Number(v))
return val
}
if (param.type === "object" && param.properties?.length) {
return buildObject(param.properties, options)
}
return options[param.name] ?? param.default
})
Comment on lines +53 to +54
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

Required non-positional params can pass through as undefined.

Line 55 returns option/default directly without enforcing param.required, so required option-backed params can silently become undefined at invocation time.

🔧 Proposed fix
-      return options[param.name] ?? param.default
+      const value = options[param.name] ?? param.default
+      if (param.required && value === undefined) {
+        throw new Error(`Missing required option: ${param.name}`)
+      }
+      return value
📝 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
return options[param.name] ?? param.default
})
const value = options[param.name] ?? param.default
if (param.required && value === undefined) {
throw new Error(`Missing required option: ${param.name}`)
}
return value
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@terminal/actions/command/create.ts` around lines 55 - 56, The current return
of options[param.name] ?? param.default in the param resolution logic allows
required option-backed params to be undefined; update the resolution (the
mapping that uses options[param.name], param.default and param.required) to
explicitly enforce param.required by throwing a descriptive error when both
options[param.name] and param.default are undefined (or null) for a required
param, otherwise return the provided option or default as before; ensure you
reference the param.required flag and include the param.name in the thrown error
message so callers know which required parameter is missing.

const mod = (await import(func.path)) as Record<
string,
(...args: unknown[]) => unknown
>
const fn = mod[func.name]
if (!fn) throw new Error(`Function ${func.name} not found in ${func.path}`)
await fn(...args)
})

return cmd
}

function buildObject(
properties: Parameter[],
options: Record<string, unknown>,
): Record<string, unknown> {
const obj: Record<string, unknown> = {}
for (const prop of properties) {
if (prop.type === "object" && prop.properties?.length) {
obj[prop.name] = buildObject(prop.properties, options)
} else {
obj[prop.name] = options[prop.name] ?? prop.default
}
}
return obj
}

function registerObjectOptions(cmd: Command, properties: Parameter[]) {
for (const prop of properties) {
if (prop.type === "object" && prop.properties?.length) {
registerObjectOptions(cmd, prop.properties)
} else {
registerOption(cmd, prop)
}
}
}

function registerOption(cmd: Command, param: Parameter) {
const flag = camelToKebab(param.name)
const description = param.description || ""

if (param.type === "string") {
if (param.required) {
cmd.requiredOption(`--${flag} <value>`, description)
} else {
cmd.option(`--${flag} <value>`, description, param.default as string)
}
} else if (param.type === "number") {
if (param.required) {
cmd.requiredOption(`--${flag} <value>`, description, Number)
} else {
cmd.option(
`--${flag} <value>`,
description,
Number,
param.default as number,
)
}
} else if (param.type === "boolean") {
cmd.option(`--${flag}`, description, param.default as boolean | undefined)
} else if (param.type === "string[]") {
if (param.required) {
cmd.requiredOption(`--${flag} <value...>`, description)
} else {
const defaultVal = (param.default ?? []) as string[]
cmd.option(`--${flag} <value...>`, description, defaultVal)
}
} else if (param.type === "number[]") {
const coerce = (v: string, prev: number[] | undefined) => {
const list = prev ?? []
list.push(Number(v))
return list
}
if (param.required) {
cmd.requiredOption(`--${flag} <value...>`, description, coerce)
} else {
const defaultVal = (param.default ?? []) as number[]
cmd.option(`--${flag} <value...>`, description, coerce, defaultVal)
}
} else if (param.type === "object") {
const parse = (v: string) => JSON.parse(v) as unknown
if (param.required) {
cmd.requiredOption(`--${flag} <json>`, description, parse)
} else {
cmd.option(`--${flag} <json>`, description, parse, param.default)
}
}
}

function camelToKebab(str: string): string {
return str.replace(/[A-Z]/g, m => `-${m.toLowerCase()}`)
}
Loading
Loading