Skip to content
4 changes: 2 additions & 2 deletions .github/workflows/managed-images.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ jobs:
CANDIDATE_SHA: ${{ github.event.pull_request.head.sha }}
# Retains the reviewed discovery-permission repair and the current
# managed-image security inventory. The previous staging source pinned
# Vim 9.2.0782, which cannot satisfy the candidate's 9.2.0858 contract.
STAGING_QA_SOURCE_SHA: af2a73f0d6ce8f08a2975560f376470387c535d0
# libssh2 nemoclaw1, which cannot satisfy the candidate's nemoclaw2 contract.
STAGING_QA_SOURCE_SHA: ce96811ddb418ad01c040521a1fe912b5bcb405e
STAGING_QA_BASE_IMAGE: nemoclaw-deepagents-code-base:staging-31396519688
STAGING_QA_FINAL_IMAGE: nemoclaw-managed-pr/langchain-deepagents-code-staging-qa
steps:
Expand Down
152 changes: 152 additions & 0 deletions scripts/checks/no-defaulted-dependent-flags.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

/**
* Guard oclif flag definitions against combining `default` with `dependsOn`.
*
* oclif validates dependsOn whenever the flag has a value. A parser default
* always supplies one, so oclif rejects every invocation that omits the
* dependency. Apply defaults in the action layer instead, as channels status
* does for --timeout (#8883).
*
* The scan covers direct `Flags.<method>({...})` object literals. Options
* passed through `Flags.custom` factories, spread composition, or aliased
* imports are out of scope; no flag under `src` or `nemoclaw/src` combines
* them with dependsOn today.
*/

import fs from "node:fs";
import path from "node:path";
import { fileURLToPath } from "node:url";
import ts from "typescript";

const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../..");
const SCAN_ROOTS = ["src", "nemoclaw/src"];
const SKIP_DIRS = new Set([".git", "coverage", "dist", "node_modules"]);

export interface DefaultedDependentFlagViolation {
filePath: string;
line: number;
flagName: string;
}

function flagObjectPropertyNames(node: ts.ObjectLiteralExpression): string[] {
return node.properties.flatMap((property) =>
(ts.isPropertyAssignment(property) ||
ts.isShorthandPropertyAssignment(property) ||
ts.isMethodDeclaration(property)) &&
(ts.isIdentifier(property.name) || ts.isStringLiteral(property.name))
? [property.name.text]
: [],
);
}

function flagNameFor(callExpression: ts.CallExpression): string {
const parent = callExpression.parent;
return ts.isPropertyAssignment(parent) &&
(ts.isIdentifier(parent.name) || ts.isStringLiteral(parent.name))
? parent.name.text
: "(unnamed flag)";
}

export function findDefaultedDependentFlags(
sourceText: string,
filePath: string,
): DefaultedDependentFlagViolation[] {
if (!sourceText.includes("dependsOn")) return [];
const sourceFile = ts.createSourceFile(filePath, sourceText, ts.ScriptTarget.Latest, true);
const violations: DefaultedDependentFlagViolation[] = [];
const visit = (node: ts.Node): void => {
if (
ts.isCallExpression(node) &&
ts.isPropertyAccessExpression(node.expression) &&
ts.isIdentifier(node.expression.expression) &&
node.expression.expression.text === "Flags" &&
node.arguments.length > 0 &&
ts.isObjectLiteralExpression(node.arguments[0])
) {
const names = flagObjectPropertyNames(node.arguments[0]);
if (names.includes("dependsOn") && names.includes("default")) {
const { line } = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile));
violations.push({ filePath, line: line + 1, flagName: flagNameFor(node) });
}
}
ts.forEachChild(node, visit);
};
visit(sourceFile);
return violations;
}

export function checkFiles(filePaths: readonly string[]): DefaultedDependentFlagViolation[] {
return filePaths.flatMap((filePath) => {
const absolutePath = path.resolve(REPO_ROOT, filePath);
return findDefaultedDependentFlags(
fs.readFileSync(absolutePath, "utf-8"),
path.relative(REPO_ROOT, absolutePath).split(path.sep).join("/"),
);
});
}

export function formatViolations(
violations: readonly DefaultedDependentFlagViolation[],
): string {
return [
"oclif flags must not combine a parser default with dependsOn.",
"The default gives the flag a value on every parse, so oclif applies",
"dependsOn validation and rejects each invocation that omits the",
"dependency (#8883). Apply the default in the action layer instead.",
"",
...violations.map(
(violation) => `${violation.filePath}:${violation.line} ${violation.flagName}`,
),
].join("\n");
}

export function isScannedSourcePath(filePath: string): boolean {
return (
SCAN_ROOTS.some((root) => filePath.startsWith(`${root}/`)) &&
filePath.endsWith(".ts") &&
!filePath.endsWith(".test.ts") &&
!filePath.endsWith(".test-helpers.ts") &&
!filePath.endsWith(".d.ts")
);
}

function sourceFiles(): string[] {
return SCAN_ROOTS.flatMap((root) => [...walkSourceFiles(path.join(REPO_ROOT, root))])
.map((filePath) => path.relative(REPO_ROOT, filePath).split(path.sep).join("/"))
.filter(isScannedSourcePath);
}

function* walkSourceFiles(dir: string): Generator<string> {
if (!fs.existsSync(dir)) return;
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
const fullPath = path.join(dir, entry.name);
if (entry.isDirectory()) {
if (!SKIP_DIRS.has(entry.name)) yield* walkSourceFiles(fullPath);
continue;
}
if (entry.isFile() && entry.name.endsWith(".ts")) yield fullPath;
}
}

function normalizeCliPaths(args: readonly string[]): string[] {
return args
.filter((arg) => arg !== "--")
.map((arg) => path.relative(REPO_ROOT, path.resolve(arg)).split(path.sep).join("/"))
.filter(isScannedSourcePath);
}

function main(): void {
const cliPaths = normalizeCliPaths(process.argv.slice(2));
const filePaths = cliPaths.length > 0 ? cliPaths : sourceFiles();
const violations = checkFiles(filePaths);
if (violations.length > 0) {
console.error(formatViolations(violations));
process.exitCode = 1;
}
}

if (fileURLToPath(import.meta.url) === path.resolve(process.argv[1] ?? "")) {
main();
}
5 changes: 5 additions & 0 deletions scripts/checks/run.mts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ export const CHECKS: readonly CheckCommand[] = [
command: TSX,
args: ["scripts/checks/dependency-pins.mts"],
},
{
name: "no-defaulted-dependent-flags",
command: TSX,
args: ["scripts/checks/no-defaulted-dependent-flags.mts"],
},
{
name: "no-coverage-ignore",
command: TSX,
Expand Down
20 changes: 18 additions & 2 deletions src/commands/sandbox/channels/status.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,27 @@ describe("SandboxChannelsStatusCommand readiness flags", () => {
rootDir,
);

expect(showSandboxChannelStatusMock).toHaveBeenCalledWith("alpha", {
channel: "slack",
asJson: true,
quietJson: true,
wait: true,
timeoutSeconds: undefined,
});
expect(process.exitCode).toBe(1);
});

it.each([
[["alpha"], undefined],
[["alpha", "--channel", "slack"], "slack"],
] as const)("accepts the documented no-wait invocation %j (#8883)", async (argv, channel) => {
await SandboxChannelsStatusCommand.run([...argv], rootDir);

expect(showSandboxChannelStatusMock).toHaveBeenCalledTimes(1);
expect(showSandboxChannelStatusMock).toHaveBeenCalledWith(
"alpha",
expect.objectContaining({ timeoutSeconds: 180 }),
expect.objectContaining({ channel, wait: undefined, timeoutSeconds: undefined }),
);
expect(process.exitCode).toBe(1);
});

it.each([
Expand Down
7 changes: 5 additions & 2 deletions src/commands/sandbox/channels/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,11 @@ export default class SandboxChannelsStatusCommand extends NemoClawCommand {
}),
timeout: Flags.integer({
dependsOn: ["wait"],
description: "Readiness timeout in seconds",
default: 180,
// No parser default: oclif validates dependsOn whenever the flag has a
// value, so a default makes oclif reject every invocation that omits
// --wait (#8883). showSandboxChannelStatus applies the 180-second
// budget documented in docs/reference/commands.mdx.
description: "Readiness timeout in seconds (default: 180)",
min: 1,
}),
};
Expand Down
22 changes: 22 additions & 0 deletions src/lib/actions/sandbox/channel-status.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,28 @@ describe("showSandboxChannelStatus Slack readiness wait", () => {
expect(configRead.mock.calls.map(([timeoutMs]) => timeoutMs)).toEqual([1_150]);
expect(sleep).toHaveBeenCalledWith(500);
});

it("applies the documented 180-second budget when the caller omits timeoutSeconds (#8883)", async () => {
const { deps, gatewayPolicy } = slackWaitHarness([{ connected: false }]);

const result = await showSandboxChannelStatus("alpha", {
deps,
channel: "slack",
wait: true,
timeoutSeconds: undefined,
pollIntervalMs: 60_000,
asJson: true,
quietJson: true,
});

expect(result && "readiness" in result ? result.readiness : null).toMatchObject({
state: "timeout",
category: "timeout",
reason: "timeout",
elapsedMs: 180_000,
});
expect(gatewayPolicy.mock.calls[0]?.[1]).toBe(180_000);
});
});

describe("showSandboxChannelStatus unsupported readiness wait", () => {
Expand Down
8 changes: 8 additions & 0 deletions test/checks-runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ describe("checks runner", () => {
});
});

it("registers the defaulted dependent flag check (#8883)", () => {
expect(CHECKS).toContainEqual({
name: "no-defaulted-dependent-flags",
command: process.platform === "win32" ? "tsx.cmd" : "tsx",
args: ["scripts/checks/no-defaulted-dependent-flags.mts"],
});
});

it("runs Windows command shims through cmd.exe", () => {
expect(
buildCheckSpawnInvocation(sampleCheck, "win32", {
Expand Down
2 changes: 1 addition & 1 deletion test/managed-image-publication-workflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ describe("complete managed-image publication workflow", () => {
expect(qaBuilder.permissions).toEqual({ contents: "read" });
expect(qaBuilder.env).toMatchObject({
CANDIDATE_SHA: "${{ github.event.pull_request.head.sha }}",
STAGING_QA_SOURCE_SHA: "af2a73f0d6ce8f08a2975560f376470387c535d0",
STAGING_QA_SOURCE_SHA: "ce96811ddb418ad01c040521a1fe912b5bcb405e",
STAGING_QA_BASE_IMAGE: "nemoclaw-deepagents-code-base:staging-31396519688",
});
expect(qaBuilder.env).not.toHaveProperty("STAGING_PRODUCER_SHA");
Expand Down
64 changes: 64 additions & 0 deletions test/no-defaulted-dependent-flags.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import { describe, expect, it } from "vitest";
import {
findDefaultedDependentFlags,
isScannedSourcePath,
} from "../scripts/checks/no-defaulted-dependent-flags.mts";

describe("defaulted dependent flag guard", () => {
it("reports a Flags.integer definition that combines default with dependsOn (#8883)", () => {
const source = [
"const flags = {",
" timeout: Flags.integer({",
' dependsOn: ["wait"],',
" default: 180,",
" }),",
"};",
].join("\n");

expect(findDefaultedDependentFlags(source, "src/example.ts")).toEqual([
{ filePath: "src/example.ts", line: 2, flagName: "timeout" },
]);
});

it("flags a function-valued default, which oclif also resolves on every parse", () => {
const source = 'const f = Flags.string({ dependsOn: ["wait"], default: () => "x" });';

expect(findDefaultedDependentFlags(source, "src/example.ts")).toMatchObject([
{ line: 1, flagName: "(unnamed flag)" },
]);
});

it("allows dependsOn without a default and a default without dependsOn", () => {
const source = [
'const a = Flags.integer({ dependsOn: ["wait"], min: 1 });',
"const b = Flags.integer({ default: 180 });",
].join("\n");

expect(findDefaultedDependentFlags(source, "src/example.ts")).toEqual([]);
});

it("ignores non-Flags calls that combine the same option names", () => {
const source = 'options({ dependsOn: ["wait"], default: 180 });';

expect(findDefaultedDependentFlags(source, "src/example.ts")).toEqual([]);
});
});

describe("scanned source path selection", () => {
it("scans source TypeScript under src and nemoclaw/src", () => {
expect(isScannedSourcePath("src/commands/sandbox/channels/status.ts")).toBe(true);
expect(isScannedSourcePath("nemoclaw/src/commands/example.ts")).toBe(true);
});

it("excludes tests, declarations, and paths outside the scan roots", () => {
expect(isScannedSourcePath("src/commands/sandbox/channels/status.test.ts")).toBe(false);
expect(isScannedSourcePath("src/lib/actions/sandbox/channel-status.test-helpers.ts")).toBe(
false,
);
expect(isScannedSourcePath("src/lib/example.d.ts")).toBe(false);
expect(isScannedSourcePath("scripts/checks/run.mts")).toBe(false);
});
});
Loading