Skip to content
Merged
108 changes: 108 additions & 0 deletions .agents/skills/find-review-pr/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
---
name: find-review-pr
description: Finds open GitHub PRs with security and priority-high labels, links each to its issue, detects duplicates (multiple PRs fixing the same issue), and presents a table of review candidates. Use when looking for the next PR to review. Trigger keywords - find pr, find review, next pr, pr to review, duplicate pr, security pr.
user_invocable: true
---

# Find PR to Review

Search for open PRs labeled `security` + `priority: high`, associate each with its linked issue, detect duplicates (multiple PRs targeting the same issue), and present a clean summary so you can decide what to review or close.

## Prerequisites

- `gh` (GitHub CLI) must be installed and authenticated.
- You must be in a GitHub repository (or the user must specify `OWNER/REPO`).

## Step 1: Fetch candidate PRs

List all open PRs that carry **both** the `security` and `priority: high` labels:

```bash
gh pr list --label security --label "priority: high" --state open --limit 50 --json number,title,author,headRefName,labels,body,createdAt
```

If the result is empty, report that there are no matching PRs and stop.

## Step 2: Extract linked issues

For each PR, parse the body for linked issue references. Look for these patterns (case-insensitive):

- `Fixes #NNN`, `Closes #NNN`, `Resolves #NNN`
- `Related Issue` / `Linked Issue` section containing `#NNN`
- Issue number in the PR title, e.g. `(#NNN)` suffix
- Branch name containing an issue number, e.g. `fix/something-NNN`

Build a mapping: `PR# → [issue numbers]`.

If a PR has no detectable linked issue, mark it as `(no linked issue)`.

## Step 3: Detect duplicates

Group PRs by linked issue number. Any issue with **two or more** open PRs is a duplicate group.

For each duplicate group, fetch a brief summary of each competing PR to help the user decide which to keep:

```bash
gh pr view <number> --json number,title,author,createdAt,additions,deletions,reviewDecision,statusCheckRollup --jq '{number,title,author: .author.login,created: .createdAt,additions,deletions,review: .reviewDecision,checks: [.statusCheckRollup[]?.conclusion] | unique}'
```

## Step 4: Check for superseded PRs

Also flag PRs whose body contains phrases like:

- `follow-up to #NNN` / `supersedes #NNN` / `replaces #NNN` / `folds in #NNN`

where `#NNN` is another **open** PR number in the candidate list. These indicate one PR has absorbed another.

## Step 5: Present results

### Duplicates / Superseded

If duplicates or superseded PRs exist, present them first in a table:

```markdown
### Duplicate PRs (same issue)

| Issue | PR | Author | Title | +/- | Status |
|-------|-----|--------|-------|-----|--------|
| #804 | #1121 | user1 | ... | +50/-10 | Checks passing |
| #804 | #1300 | user2 | ... | +80/-20 | Checks failing |

**Recommendation:** #1121 is smaller and passing checks — consider closing #1300.
```

For superseded PRs:

```markdown
### Superseded PRs

- #1416 supersedes/folds in #1392 (shell-quote sandboxName)
→ Consider closing #1392 if #1416 covers its scope.
```

### Clean candidates

Present non-duplicate PRs in a table:

```markdown
### Review candidates (no duplicates)

| PR | Issue | Title | Author | Age |
|----|-------|-------|--------|-----|
| #1476 | #577 | disable remote uninstall fallback | user1 | 2d |
| #1121 | #804 | Landlock read-only /sandbox | user2 | 6d |
```

### Summary line

End with a one-line recommendation of which PR to review first, preferring:

1. Older PRs (waiting longest)
2. PRs with passing checks
3. PRs with smaller diff size (easier to review)

## Notes

- Do NOT automatically close any PRs. Only present findings and recommendations.
- If the user specifies additional filters (e.g., a specific scope label like `OpenShell`), apply them.
- If the user asks for a different priority label, adjust accordingly.
22 changes: 21 additions & 1 deletion .agents/skills/nemoclaw-reference/references/commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ The wizard creates an OpenShell gateway, registers inference providers, builds t
Use this command for new installs and for recreating a sandbox after changes to policy or configuration.

```console
$ nemoclaw onboard
$ nemoclaw onboard [--non-interactive] [--resume] [--from <Dockerfile>]
```

The wizard prompts for a provider first, then collects the provider credential if needed.
Expand Down Expand Up @@ -82,6 +82,26 @@ Uppercase letters are automatically lowercased.
Before creating the gateway, the wizard runs preflight checks.
It verifies that Docker is reachable, warns on unsupported runtimes such as Podman, and prints host remediation guidance when prerequisites are missing.

#### `--from <Dockerfile>`

Build the sandbox image from a custom Dockerfile instead of the stock NemoClaw image.
The entire parent directory of the specified file is used as the Docker build context, so any files your Dockerfile references (scripts, config, etc.) must live alongside it.

```console
$ nemoclaw onboard --from path/to/Dockerfile
```

The file can have any name; if it is not already named `Dockerfile`, onboard copies it to `Dockerfile` inside the staged build context automatically.
All NemoClaw build arguments (`NEMOCLAW_MODEL`, `NEMOCLAW_PROVIDER_KEY`, `NEMOCLAW_INFERENCE_BASE_URL`, etc.) are injected as `ARG` overrides at build time, so declare them in your Dockerfile if you need to reference them.

In non-interactive mode, the path can also be supplied via the `NEMOCLAW_FROM_DOCKERFILE` environment variable:

```console
$ NEMOCLAW_NON_INTERACTIVE=1 NEMOCLAW_FROM_DOCKERFILE=path/to/Dockerfile nemoclaw onboard
```

If a `--resume` is attempted with a different `--from` path than the original session, onboarding exits with a conflict error rather than silently building from the wrong image.

### `nemoclaw list`

List all registered sandboxes with their model, provider, and policy presets.
Expand Down
80 changes: 76 additions & 4 deletions bin/lib/onboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -1327,6 +1327,18 @@ function getResumeConfigConflicts(session, opts = {}) {
});
}

const requestedFrom = opts.fromDockerfile ? path.resolve(opts.fromDockerfile) : null;
const recordedFrom = session?.metadata?.fromDockerfile
? path.resolve(session.metadata.fromDockerfile)
: null;
if (requestedFrom !== recordedFrom) {
conflicts.push({
field: "fromDockerfile",
requested: requestedFrom,
recorded: recordedFrom,
});
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return conflicts;
}

Expand Down Expand Up @@ -1942,6 +1954,7 @@ async function createSandbox(
sandboxNameOverride = null,
webSearchConfig = null,
enabledChannels = null,
fromDockerfile = null,
) {
step(6, 8, "Creating sandbox");

Expand Down Expand Up @@ -2028,8 +2041,34 @@ async function createSandbox(
registry.removeSandbox(sandboxName);
}

// Stage only the files the Docker build actually consumes so uploads stay small.
const { buildCtx, stagedDockerfile } = stageOptimizedSandboxBuildContext(ROOT);
// Stage build context — use the custom Dockerfile path when provided,
// otherwise use the optimised default that only sends what the build needs.
let buildCtx, stagedDockerfile;
if (fromDockerfile) {
const fromResolved = path.resolve(fromDockerfile);
if (!fs.existsSync(fromResolved)) {
console.error(` Custom Dockerfile not found: ${fromResolved}`);
process.exit(1);
}
buildCtx = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-build-"));
stagedDockerfile = path.join(buildCtx, "Dockerfile");
// Copy the entire parent directory as build context.
fs.cpSync(path.dirname(fromResolved), buildCtx, {
recursive: true,
filter: (src) => {
const base = path.basename(src);
return !["node_modules", ".git", ".venv", "__pycache__"].includes(base);
},
});
// If the caller pointed at a file not named "Dockerfile", copy it to the
// location openshell expects (buildCtx/Dockerfile).
if (path.basename(fromResolved) !== "Dockerfile") {
fs.copyFileSync(fromResolved, stagedDockerfile);
}
Comment on lines +2047 to +2067

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Validate that --from points to a regular file.

The new check only guards existence. Passing a directory here falls through to fs.copyFileSync() and throws an uncaught EISDIR, which skips the friendly CLI error path and leaves the temp build context behind.

🛠 Suggested validation
     if (!fs.existsSync(fromResolved)) {
       console.error(`  Custom Dockerfile not found: ${fromResolved}`);
       process.exit(1);
     }
+    if (!fs.statSync(fromResolved).isFile()) {
+      console.error(`  Custom Dockerfile must be a file: ${fromResolved}`);
+      process.exit(1);
+    }
     // Copy the entire parent directory as build context. copyBuildContextDir
📝 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
if (fromDockerfile) {
const fromResolved = path.resolve(fromDockerfile);
if (!fs.existsSync(fromResolved)) {
console.error(` Custom Dockerfile not found: ${fromResolved}`);
process.exit(1);
}
// Copy the entire parent directory as build context. copyBuildContextDir
// already filters out node_modules, .git, .venv, __pycache__, etc.
copyBuildContextDir(path.dirname(fromResolved), buildCtx);
// If the caller pointed at a file not named "Dockerfile", copy it to the
// location openshell expects (buildCtx/Dockerfile).
if (path.basename(fromResolved) !== "Dockerfile") {
fs.copyFileSync(fromResolved, stagedDockerfile);
}
if (fromDockerfile) {
const fromResolved = path.resolve(fromDockerfile);
if (!fs.existsSync(fromResolved)) {
console.error(` Custom Dockerfile not found: ${fromResolved}`);
process.exit(1);
}
if (!fs.statSync(fromResolved).isFile()) {
console.error(` Custom Dockerfile must be a file: ${fromResolved}`);
process.exit(1);
}
// Copy the entire parent directory as build context. copyBuildContextDir
// already filters out node_modules, .git, .venv, __pycache__, etc.
copyBuildContextDir(path.dirname(fromResolved), buildCtx);
// If the caller pointed at a file not named "Dockerfile", copy it to the
// location openshell expects (buildCtx/Dockerfile).
if (path.basename(fromResolved) !== "Dockerfile") {
fs.copyFileSync(fromResolved, stagedDockerfile);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bin/lib/onboard.js` around lines 1776 - 1789, The current check only verifies
existence so passing a directory for --from leads to an uncaught EISDIR in
fs.copyFileSync; update the fromDockerfile handling (the fromResolved branch
used with copyBuildContextDir, stagedDockerfile and fs.copyFileSync) to verify
that fromResolved is a regular file (use fs.statSync or fs.lstatSync and
isFile()) before attempting to copy; if it is not a file, print a clear error
like "Custom Dockerfile is not a file: <path>" and exit(1) so the friendly CLI
error path runs and temp build context is cleaned up.

console.log(` Using custom Dockerfile: ${fromResolved}`);
} else {
({ buildCtx, stagedDockerfile } = stageOptimizedSandboxBuildContext(ROOT));
}

// Create sandbox (use -- echo to avoid dropping into interactive shell)
// Pass the base policy so sandbox starts in proxy mode (required for policy updates later)
Expand Down Expand Up @@ -3737,6 +3776,12 @@ async function onboard(opts = {}) {
NON_INTERACTIVE = opts.nonInteractive || process.env.NEMOCLAW_NON_INTERACTIVE === "1";
delete process.env.OPENSHELL_GATEWAY;
const resume = opts.resume === true;
// In non-interactive mode also accept the env var so CI pipelines can set it.
// This is the explicitly requested value; on resume it may be absent and the
// session-recorded path is used instead (see below).
const requestedFromDockerfile =
opts.fromDockerfile ||
(isNonInteractive() ? process.env.NEMOCLAW_FROM_DOCKERFILE || null : null);
const noticeAccepted = await ensureUsageNoticeConsent({
nonInteractive: isNonInteractive(),
acceptedByFlag: opts.acceptThirdPartySoftware === true,
Expand All @@ -3746,7 +3791,7 @@ async function onboard(opts = {}) {
process.exit(1);
}
const lockResult = onboardSession.acquireOnboardLock(
`nemoclaw onboard${resume ? " --resume" : ""}${isNonInteractive() ? " --non-interactive" : ""}`,
`nemoclaw onboard${resume ? " --resume" : ""}${isNonInteractive() ? " --non-interactive" : ""}${requestedFromDockerfile ? ` --from ${requestedFromDockerfile}` : ""}`,
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if (!lockResult.acquired) {
console.error(" Another NemoClaw onboarding run is already in progress.");
Expand All @@ -3771,22 +3816,47 @@ async function onboard(opts = {}) {

try {
let session;
// Merged, absolute fromDockerfile: explicit flag/env takes precedence; on
// resume falls back to what the original session recorded so the same image
// is used even when --from is omitted from the resume invocation.
let fromDockerfile;
if (resume) {
session = onboardSession.loadSession();
if (!session || session.resumable === false) {
console.error(" No resumable onboarding session was found.");
console.error(" Run: nemoclaw onboard");
process.exit(1);
}
const sessionFrom = session?.metadata?.fromDockerfile || null;
fromDockerfile = requestedFromDockerfile
? path.resolve(requestedFromDockerfile)
: sessionFrom
? path.resolve(sessionFrom)
: null;
const resumeConflicts = getResumeConfigConflicts(session, {
nonInteractive: isNonInteractive(),
fromDockerfile: requestedFromDockerfile,
});
if (resumeConflicts.length > 0) {
for (const conflict of resumeConflicts) {
if (conflict.field === "sandbox") {
console.error(
` Resumable state belongs to sandbox '${conflict.recorded}', not '${conflict.requested}'.`,
);
} else if (conflict.field === "fromDockerfile") {
if (!conflict.recorded) {
console.error(
` Session was started without --from; add --from '${conflict.requested}' to resume it.`,
);
} else if (!conflict.requested) {
console.error(
` Session was started with --from '${conflict.recorded}'; rerun with that path to resume it.`,
);
} else {
console.error(
` Session was started with --from '${conflict.recorded}', not '${conflict.requested}'.`,
);
}
} else {
console.error(
` Resumable state recorded ${conflict.field} '${conflict.recorded}', not '${conflict.requested}'.`,
Expand All @@ -3805,10 +3875,11 @@ async function onboard(opts = {}) {
});
session = onboardSession.loadSession();
} else {
fromDockerfile = requestedFromDockerfile ? path.resolve(requestedFromDockerfile) : null;
session = onboardSession.saveSession(
onboardSession.createSession({
mode: isNonInteractive() ? "non-interactive" : "interactive",
metadata: { gatewayName: "nemoclaw" },
metadata: { gatewayName: "nemoclaw", fromDockerfile: fromDockerfile || null },
}),
);
}
Expand Down Expand Up @@ -4007,6 +4078,7 @@ async function onboard(opts = {}) {
sandboxName,
webSearchConfig,
enabledChannels,
fromDockerfile,
);
onboardSession.markStepComplete("sandbox", { sandboxName, provider, model, nimContainer });
}
Expand Down
22 changes: 20 additions & 2 deletions bin/nemoclaw.js
Original file line number Diff line number Diff line change
Expand Up @@ -779,20 +779,37 @@ function exitWithSpawnResult(result) {

async function onboard(args) {
const { onboard: runOnboard } = require("./lib/onboard");

// Extract --from <path> before the unknown-arg validator: it takes a value
// so the set-based check would reject the value token as an unknown flag.
let fromDockerfile = null;
const fromIdx = args.indexOf("--from");
if (fromIdx !== -1) {
fromDockerfile = args[fromIdx + 1];
if (!fromDockerfile || fromDockerfile.startsWith("--")) {
console.error(" --from requires a path to a Dockerfile");
console.error(
` Usage: nemoclaw onboard [--non-interactive] [--resume] [--from <Dockerfile>] [${NOTICE_ACCEPT_FLAG}]`,
);
process.exit(1);
}
args = [...args.slice(0, fromIdx), ...args.slice(fromIdx + 2)];
}

const allowedArgs = new Set(["--non-interactive", "--resume", NOTICE_ACCEPT_FLAG]);
const unknownArgs = args.filter((arg) => !allowedArgs.has(arg));
if (unknownArgs.length > 0) {
console.error(` Unknown onboard option(s): ${unknownArgs.join(", ")}`);
console.error(
` Usage: nemoclaw onboard [--non-interactive] [--resume] [${NOTICE_ACCEPT_FLAG}]`,
` Usage: nemoclaw onboard [--non-interactive] [--resume] [--from <Dockerfile>] [${NOTICE_ACCEPT_FLAG}]`,
);
process.exit(1);
}
const nonInteractive = args.includes("--non-interactive");
const resume = args.includes("--resume");
const acceptThirdPartySoftware =
args.includes(NOTICE_ACCEPT_FLAG) || String(process.env[NOTICE_ACCEPT_ENV] || "") === "1";
await runOnboard({ nonInteractive, resume, acceptThirdPartySoftware });
await runOnboard({ nonInteractive, resume, fromDockerfile, acceptThirdPartySoftware });
}

async function setup(args = []) {
Expand Down Expand Up @@ -1262,6 +1279,7 @@ function help() {

${G}Getting Started:${R}
${B}nemoclaw onboard${R} Configure inference endpoint and credentials
nemoclaw onboard ${D}--from <Dockerfile>${R} Use a custom Dockerfile for the sandbox image
${D}(non-interactive: ${NOTICE_ACCEPT_FLAG} or ${NOTICE_ACCEPT_ENV}=1)${R}

${G}Sandbox Management:${R}
Expand Down
22 changes: 21 additions & 1 deletion docs/reference/commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ The wizard creates an OpenShell gateway, registers inference providers, builds t
Use this command for new installs and for recreating a sandbox after changes to policy or configuration.

```console
$ nemoclaw onboard
$ nemoclaw onboard [--non-interactive] [--resume] [--from <Dockerfile>]
```

The wizard prompts for a provider first, then collects the provider credential if needed.
Expand Down Expand Up @@ -104,6 +104,26 @@ Uppercase letters are automatically lowercased.
Before creating the gateway, the wizard runs preflight checks.
It verifies that Docker is reachable, warns on unsupported runtimes such as Podman, and prints host remediation guidance when prerequisites are missing.

#### `--from <Dockerfile>`

Build the sandbox image from a custom Dockerfile instead of the stock NemoClaw image.
The entire parent directory of the specified file is used as the Docker build context, so any files your Dockerfile references (scripts, config, etc.) must live alongside it.

```console
$ nemoclaw onboard --from path/to/Dockerfile
```

The file can have any name; if it is not already named `Dockerfile`, onboard copies it to `Dockerfile` inside the staged build context automatically.
All NemoClaw build arguments (`NEMOCLAW_MODEL`, `NEMOCLAW_PROVIDER_KEY`, `NEMOCLAW_INFERENCE_BASE_URL`, etc.) are injected as `ARG` overrides at build time, so declare them in your Dockerfile if you need to reference them.

In non-interactive mode, the path can also be supplied via the `NEMOCLAW_FROM_DOCKERFILE` environment variable:

```console
$ NEMOCLAW_NON_INTERACTIVE=1 NEMOCLAW_FROM_DOCKERFILE=path/to/Dockerfile nemoclaw onboard
```

If a `--resume` is attempted with a different `--from` path than the original session, onboarding exits with a conflict error rather than silently building from the wrong image.

### `nemoclaw list`

List all registered sandboxes with their model, provider, and policy presets.
Expand Down
Loading
Loading