Skip to content
Open
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
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ Requires the `project` (or `read:project`) OAuth scope on the `gh` token; `src/e
`gh` accepts `--label`, `--assignee`, `--reviewer`, `--project`, and the `--add-*`/`--remove-*` variants once per value, so gh-axi must collect _every_ occurrence.
Use `getAllFlags`/`takeAllFlags` plus `pushRepeated`; `getFlag`/`takeFlag` keep only the first occurrence and silently discard the rest, which is the bug that recurred as #55, #57, and #75.
Both collectors reject a dangling (`--label` with nothing after it) or blank (`--label=`) value with a `VALIDATION_ERROR` instead of dropping it.
`getFlag`/`takeFlag` similarly reject a flag-like value (one starting with `--`) rather than treating it as the value, so a dangling single-value flag can't silently swallow the next flag's token.
Pick the collector that matches the surrounding file: `issue.ts` reads args non-destructively (`getAllFlags`), `pr.ts` consumes them (`takeAllFlags`).
When a flag becomes repeatable, mark it `(repeatable)` in that command's `*_HELP` string.

Expand Down
17 changes: 15 additions & 2 deletions src/args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,26 @@ function flagEqualsPrefix(flag: string): string {
return `${flag}=`;
}

/**
* A dangling flag (no value given before the next `--flag` token) must never
* silently swallow that next flag as its own value - doing so both loses the
* next flag and can leak the wrong token into a positional slot downstream.
*/
function rejectFlagLikeValue(value: string, flag: string): void {
if (value.startsWith("--"))
throw new AxiError(`${flag} requires a value`, "VALIDATION_ERROR");
}

/** Get a flag's value from --flag value or --flag=value without modifying args. */
export function getFlag(args: string[], name: string): string | undefined {
const equalsPrefix = flagEqualsPrefix(name);
for (let i = 0; i < args.length; i++) {
const arg = args[i];
if (arg === name) {
if (i + 1 >= args.length) return undefined;
return args[i + 1];
const val = args[i + 1];
rejectFlagLikeValue(val, name);
return val;
}
if (arg.startsWith(equalsPrefix)) {
return arg.slice(equalsPrefix.length);
Expand All @@ -27,6 +39,7 @@ export function takeFlag(args: string[], flag: string): string | undefined {
const arg = args[i];
if (arg === flag) {
const val = args[i + 1];
if (val !== undefined) rejectFlagLikeValue(val, flag);
args.splice(i, 2);
return val;
}
Expand All @@ -53,7 +66,7 @@ export function takeBoolFlag(args: string[], flag: string): boolean {
}

function requireFlagValue(value: string, flag: string): string {
if (value.trim() === "")
if (value.trim() === "" || value.startsWith("--"))
throw new AxiError(`${flag} requires a value`, "VALIDATION_ERROR");
return value;
}
Expand Down
15 changes: 10 additions & 5 deletions src/commands/repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { encode } from '@toon-format/toon';
import type { RepoContext } from '../context.js';
import { ghJson, ghExec } from '../gh.js';
import { AxiError } from '../errors.js';
import { getFlag, hasFlag } from '../args.js';
import { getFlag, hasFlag, takeFlag, takeBoolFlag } from '../args.js';
import {
field,
lower,
Expand Down Expand Up @@ -167,21 +167,26 @@ async function forkRepo(args: string[], ctx?: RepoContext): Promise<string> {
}

async function listRepos(args: string[], ctx?: RepoContext): Promise<string> {
// Consume value-taking flags before scanning for positionals, so a bare
// flag value (e.g. the "200" in --limit 200) can never be mistaken for
// the optional owner positional when no owner is given.
const limit = takeFlag(args, '--limit') ?? '30';
const visibility = takeFlag(args, '--visibility');
const language = takeFlag(args, '--language');
const archived = takeBoolFlag(args, '--archived');

const positionals = args.filter((a) => !a.startsWith('--'));
const owner = positionals[1]; // optional

const limit = getFlag(args, '--limit') ?? '30';
const ghArgs = [
'repo', 'list',
'--json', 'name,description,visibility,primaryLanguage,stargazerCount,updatedAt',
'--limit', limit,
];
if (owner) ghArgs.splice(2, 0, owner); // insert owner after 'list'
const visibility = getFlag(args, '--visibility');
if (visibility) ghArgs.push('--visibility', visibility);
const language = getFlag(args, '--language');
if (language) ghArgs.push('--language', language);
if (hasFlag(args, '--archived')) ghArgs.push('--archived');
if (archived) ghArgs.push('--archived');

const repos = await ghJson<Record<string, unknown>[]>(ghArgs);
const isEmpty = repos.length === 0;
Expand Down
2 changes: 1 addition & 1 deletion src/commands/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ function takeViewFlagValue(args: string[], flag: string): string | undefined {
const present = args.includes(flag);
const value = takeFlag(args, flag);
if (!present) return undefined;
if (!value || value.startsWith("--")) {
if (!value) {
throw new AxiError(`Missing value for ${flag}`, "VALIDATION_ERROR");
}
return value;
Expand Down
14 changes: 14 additions & 0 deletions test/args.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ describe("getFlag", () => {
expect(getFlag(["--state", "open", "--repo"], "--repo")).toBeUndefined();
});

it("throws instead of swallowing an adjacent flag as the value", () => {
expect(() =>
getFlag(["--repo", "--state", "open"], "--repo"),
).toThrow("--repo requires a value");
});

it("does not modify the args array", () => {
const args = ["--repo", "cli/cli"];
getFlag(args, "--repo");
Expand Down Expand Up @@ -68,6 +74,14 @@ describe("takeFlag", () => {
expect(val).toBe("val");
expect(args).toEqual(["--other"]);
});

it("throws instead of swallowing an adjacent flag as the value", () => {
const args = ["--limit", "--visibility", "public"];
expect(() => takeFlag(args, "--limit")).toThrow(
"--limit requires a value",
);
expect(args).toEqual(["--limit", "--visibility", "public"]);
});
});

describe("hasFlag", () => {
Expand Down
45 changes: 45 additions & 0 deletions test/commands/repo.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,51 @@ describe('repoCommand', () => {

expect(result).toContain('showing first 10');
});

it('does not mistake the --limit value for the owner positional when no owner is given', async () => {
mockedGhJson.mockResolvedValue([]);

await repoCommand(['list', '--limit', '10'], ctx);

expect(mockedGhJson).toHaveBeenCalledWith([
'repo', 'list',
'--json', 'name,description,visibility,primaryLanguage,stargazerCount,updatedAt',
'--limit', '10',
]);
});

it('passes an explicit owner positional through to gh repo list', async () => {
mockedGhJson.mockResolvedValue([]);

await repoCommand(['list', 'octo', '--limit', '10'], ctx);

expect(mockedGhJson).toHaveBeenCalledWith([
'repo', 'list', 'octo',
'--json', 'name,description,visibility,primaryLanguage,stargazerCount,updatedAt',
'--limit', '10',
]);
});

it('throws instead of misreading an adjacent flag as the --limit value', async () => {
await expect(
repoCommand(['list', '--limit', '--visibility', 'public'], ctx),
).rejects.toThrow('--limit requires a value');
expect(mockedGhJson).not.toHaveBeenCalled();
});

it('does not mistake --visibility or --language values for the owner positional', async () => {
mockedGhJson.mockResolvedValue([]);

await repoCommand(['list', '--visibility', 'public', '--language', 'TypeScript'], ctx);

expect(mockedGhJson).toHaveBeenCalledWith([
'repo', 'list',
'--json', 'name,description,visibility,primaryLanguage,stargazerCount,updatedAt',
'--limit', '30',
'--visibility', 'public',
'--language', 'TypeScript',
]);
});
});

describe('clone', () => {
Expand Down
2 changes: 1 addition & 1 deletion test/commands/run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ describe("runCommand", () => {
runCommand(["view", "100", "--job", "--log"], ctx),
).rejects.toMatchObject({
code: "VALIDATION_ERROR",
message: "Missing value for --job",
message: "--job requires a value",
});
expect(mockedGhExec).not.toHaveBeenCalled();
expect(mockedGhJson).not.toHaveBeenCalled();
Expand Down
Loading