feat(review): add --path support for targeted folder/file reviews by JohnnyVicious · Pull Request #60 · JohnnyVicious/opencode-plugin-cc

JohnnyVicious · 2026-04-13T05:46:41Z

Summary

Extends /opencode:review and /opencode:adversarial-review to accept --path <path> flags for reviewing specific directories or files instead of only git diff.

--path src/auth reviews that specific directory
--path src/auth --path scripts/migrate.mjs reviews multiple paths
Mutually exclusive with --pr (paths take precedence)

Changes

fs.mjs: Add collectFolderContext() with per-file and total size caps, binary file detection, and broken symlink skipping
prompts.mjs: Extend buildReviewPrompt() to accept paths option, builds <files> section instead of <diff> when paths are set
opencode-companion.mjs: Add "path" to valueOptions, normalizePathOption() helper, thread paths to prompt builder
review.md / adversarial-review.md: Document --path flag in argument-hint and description
README.md: Document --path in the slash commands section

Testing

All 172 existing tests pass.

Fixes #21

Extends /opencode:review and /opencode:adversarial-review to accept --path <path> flags for reviewing specific directories or files instead of only git diff. Useful for reviewing specific subdirectories, fixed sets of files, or large untracked/imported code drops. Changes: - Add collectFolderContext() to fs.mjs with per-file and total size caps - Extend buildReviewPrompt() in prompts.mjs to accept paths option - Add normalizePathOption() helper and --path to valueOptions in companion - Update review.md and adversarial-review.md command documentation - Update README.md to document the new --path option Implements: #21

JohnnyVicious · 2026-04-13T08:06:46Z

@codex review this PR

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4fc5af207a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-13T08:09:52Z

plugins/opencode/scripts/lib/fs.mjs

+        const entries = fs.readdirSync(resolvedPath, { withFileTypes: true });
+        for (const entry of entries) {
+          if (result.files.length >= maxFiles) break;
+          const entryPath = path.join(resolvedPath, entry.name);
+          if (visited.has(entryPath)) continue;


Recursively process directory targets for --path reviews

When a --path points to a directory, this block only marks each entry as visited and never traverses or reads those entries, so directory-based reviews end up with empty <files> context (I verified collectFolderContext(..., ['plugins/opencode/scripts/lib']) returns zero files). This breaks the main documented use case (--path src) and yields near-useless reviews for folders.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-13T08:09:52Z

plugins/opencode/scripts/lib/fs.mjs

+    if (!resolvedPath.startsWith(path.resolve(cwd))) {
+      continue;


Harden workspace boundary check for --path inputs

The workspace guard uses a raw string prefix check, which allows sibling paths like ../opencode-plugin-cc-malicious to pass when the repo path is /workspace/opencode-plugin-cc because /workspace/opencode-plugin-cc-malicious.startsWith(/workspace/opencode-plugin-cc) is true. That lets --path read files outside the intended repository scope.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-13T08:09:52Z

plugins/opencode/scripts/lib/prompts.mjs

+  let folderContext = null;
+
+  // Step 1: When --path is specified, collect folder context instead of git diff
+  if (opts.paths && opts.paths.length > 0 && !opts.pr) {


Make --path take precedence when --pr is also set

Documentation says --path and --pr are mutually exclusive with paths taking precedence, but this condition disables path-mode whenever opts.pr exists. In practice, passing both flags falls through to PR diff logic (and can fail if gh/auth is unavailable), which is the opposite of the documented behavior.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-13T08:09:52Z

plugins/opencode/scripts/opencode-companion.mjs

+  if (Array.isArray(pathOption)) {
+    return pathOption.flatMap((p) => p.split(",")).filter(Boolean);
+  }


Preserve all repeated --path flags in option parsing

This normalization expects pathOption may be an array from repeated --path flags, but the current parser stores only the last value for value options, so the array branch is never reached and earlier --path entries are dropped. As a result, --path src --path lib reviews only lib instead of both paths.

Useful? React with 👍 / 👎.

Copilot

Pull request overview

Adds --path <path> support to /opencode:review and /opencode:adversarial-review so users can run targeted reviews over specific files/directories (instead of only git diffs / PR diffs).

Changes:

Thread a new paths option from the companion CLI into buildReviewPrompt().
Add folder/file content collection utilities (collectFolderContext) and render a <files>-based prompt context when --path is used.
Update command docs and README to document the new flag.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
README.md	Documents `--path` for the slash commands.
plugins/opencode/scripts/opencode-companion.mjs	Adds `--path` parsing/plumbing and logs selected paths.
plugins/opencode/scripts/lib/prompts.mjs	Adds path-based prompt construction (`<files>` context) instead of `<diff>`.
plugins/opencode/scripts/lib/fs.mjs	Introduces `collectFolderContext()` for reading file contents with caps and skipping binary/broken symlinks.
plugins/opencode/commands/review.md	Documents `--path` usage and semantics.
plugins/opencode/commands/adversarial-review.md	Documents `--path` usage and semantics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-13T08:10:01Z

plugins/opencode/scripts/lib/fs.mjs

+      if (stat.isDirectory()) {
+        const entries = fs.readdirSync(resolvedPath, { withFileTypes: true });
+        for (const entry of entries) {
+          if (result.files.length >= maxFiles) break;
+          const entryPath = path.join(resolvedPath, entry.name);
+          if (visited.has(entryPath)) continue;
+          visited.add(entryPath);
+        }


Directory targets are not actually traversed/collected: the stat.isDirectory() branch only reads entries and adds them to visited, but never recurses into them or enqueues files for processing. As a result, --path some/dir will typically produce an empty <files> block. Update this to walk the directory tree (or at least include direct child files) while respecting maxFiles/maxBytes/ignore rules.

Copilot · 2026-04-13T08:10:01Z

plugins/opencode/scripts/lib/fs.mjs

+    const resolvedPath = path.resolve(cwd, targetPath);
+
+    if (!resolvedPath.startsWith(path.resolve(cwd))) {
+      continue;
+    }


The path containment check can be bypassed via prefix matches (e.g., cwd=/repo and resolvedPath=/repo-evil/... passes startsWith('/repo')). Use a safer check like path.relative(cwd, resolvedPath) (reject if it starts with .. or is absolute) or ensure the prefix includes a path separator.

Copilot · 2026-04-13T08:10:02Z

plugins/opencode/scripts/opencode-companion.mjs

+/**
+ * Normalize path option to always return an array.
+ * Handles both `--path src --path lib` (array) and `--path src,lib` (comma-separated string).
+ * @param {string|string[]|undefined} pathOption
+ * @returns {string[]|null}
+ */
+function normalizePathOption(pathOption) {
+  if (!pathOption) return null;
+  if (Array.isArray(pathOption)) {
+    return pathOption.flatMap((p) => p.split(",")).filter(Boolean);
+  }
+  if (typeof pathOption === "string") {
+    return pathOption.split(",").filter(Boolean);
+  }


normalizePathOption expects options.path may be an array (multiple --path occurrences), but the current parseArgs implementation only keeps the last value for repeated --key value flags. This means --path src --path lib will silently drop all but the last path. Consider updating the arg parser to accumulate repeated value-options into arrays, or explicitly collect --path values in the handler.

Copilot · 2026-04-13T08:10:02Z

plugins/opencode/scripts/lib/prompts.mjs

+  // Step 1: When --path is specified, collect folder context instead of git diff
+  if (opts.paths && opts.paths.length > 0 && !opts.pr) {
+    folderContext = await collectFolderContext(cwd, opts.paths, {


Docs/PR description say --path is mutually exclusive with --pr and that paths take precedence, but this branch is gated by !opts.pr, so passing both will ignore paths and run PR mode instead. Either enforce exclusivity with a clear error, or make paths override pr here (and align the behavior across commands/docs).

Copilot · 2026-04-13T08:10:02Z

plugins/opencode/scripts/lib/fs.mjs

+    const result = fs.statSync(filePath);
+    if (!result.isFile()) return false;
+    const dir = path.dirname(filePath);
+    const gitignorePath = path.join(dir, ".gitignore");
+    if (!fs.existsSync(gitignorePath)) {
+      return isGitignored(dir, cwd);
+    }
+    const patterns = fs.readFileSync(gitignorePath, "utf8").split("\n");
+    const relativePath = path.relative(cwd, filePath);
+    for (const pattern of patterns) {
+      const trimmed = pattern.trim();
+      if (!trimmed || trimmed.startsWith("#")) continue;
+      const glob = trimmed.replace(/\/$/, "");
+      if (glob === relativePath || relativePath.startsWith(glob + "/")) {
+        return true;
+      }
+    }


isGitignored()'s parent-directory lookup is broken: when .gitignore is missing it recurses with dir, but the top of the function returns false for non-files, so parent .gitignore files are never consulted. This will cause ignored files under nested directories to be included unexpectedly. Refactor to walk up directories without requiring filePath to be a file for the traversal step.

Suggested change

const result = fs.statSync(filePath);

if (!result.isFile()) return false;

const dir = path.dirname(filePath);

const gitignorePath = path.join(dir, ".gitignore");

if (!fs.existsSync(gitignorePath)) {

return isGitignored(dir, cwd);

}

const patterns = fs.readFileSync(gitignorePath, "utf8").split("\n");

const relativePath = path.relative(cwd, filePath);

for (const pattern of patterns) {

const trimmed = pattern.trim();

if (!trimmed || trimmed.startsWith("#")) continue;

const glob = trimmed.replace(/\/$/, "");

if (glob === relativePath || relativePath.startsWith(glob + "/")) {

return true;

}

}

const absoluteFilePath = path.resolve(filePath);

const absoluteCwd = path.resolve(cwd);

const result = fs.statSync(absoluteFilePath);

if (!result.isFile()) return false;

const relativePath = path.relative(absoluteCwd, absoluteFilePath);

let dir = path.dirname(absoluteFilePath);

while (true) {

const gitignorePath = path.join(dir, ".gitignore");

if (fs.existsSync(gitignorePath)) {

const patterns = fs.readFileSync(gitignorePath, "utf8").split(/\r?\n/);

for (const pattern of patterns) {

const trimmed = pattern.trim();

if (!trimmed || trimmed.startsWith("#")) continue;

const glob = trimmed.replace(/\/$/, "");

if (glob === relativePath || relativePath.startsWith(glob + "/")) {

return true;

}

}

}

if (dir === absoluteCwd) break;

const parentDir = path.dirname(dir);

if (parentDir === dir) break;

if (path.relative(absoluteCwd, parentDir).startsWith("..")) break;

dir = parentDir;

}

Copilot · 2026-04-13T08:10:02Z

plugins/opencode/scripts/lib/fs.mjs

+    const bytesRead = fs.readSync(fd, buffer, 0, 8192, 0);
+    fs.closeSync(fd);
+    for (let i = 0; i < bytesRead; i++) {
+      if (buffer[i] === 0) return true;
+    }
+    return false;


isBinaryFile() opens an fd and closes it only on the happy path. If readSync throws after openSync succeeds, the fd will leak. Wrap the read in a try/finally to ensure closeSync is always called when fd is acquired.

Suggested change

const bytesRead = fs.readSync(fd, buffer, 0, 8192, 0);

fs.closeSync(fd);

for (let i = 0; i < bytesRead; i++) {

if (buffer[i] === 0) return true;

}

return false;

try {

const bytesRead = fs.readSync(fd, buffer, 0, 8192, 0);

for (let i = 0; i < bytesRead; i++) {

if (buffer[i] === 0) return true;

}

return false;

} finally {

fs.closeSync(fd);

}

Copilot · 2026-04-13T08:10:03Z

plugins/opencode/scripts/lib/fs.mjs

+        result.files.push(relativePath);
+
+        if (result.files.length >= maxFiles) {
+          break;
+        }


Hitting maxFiles stops collection, but collectFolderContext() does not mark that the file list/content is incomplete due to the file cap (only byte overflow sets overflowed). This prevents the prompt from emitting a <content_note> about incomplete evidence when file count is truncated. Track a separate overflowedFiles flag (or fold it into overflowed) when maxFiles is reached.

Copilot · 2026-04-13T08:10:03Z

plugins/opencode/scripts/lib/prompts.mjs

+    changedFiles = folderContext.files;
+    overByteLimit = folderContext.overflowed;
+    const diffBytes = folderContext.totalBytes;
+    const diffIsComplete = !folderContext.overflowed;
+    const collectionGuidance = buildCollectionGuidance(diffIsComplete);
+
+    const targetLabel = `Review of ${opts.paths.join(", ")}`;
+
+    const reviewContext = buildFolderContext(folderContext, {
+      diffIsComplete,
+      originalDiffBytes: diffBytes,
+      maxInlineDiffBytes: maxBytes,
+      overFileLimit: changedFiles.length > maxFiles,
+      overByteLimit: folderContext.overflowed,
+    });


overFileLimit is computed as changedFiles.length > maxFiles, but collectFolderContext() never lets files.length exceed maxFiles (it breaks early). That means overFileLimit will never be true in path mode, even when file collection was truncated. Have the collector report file-cap overflow explicitly and use that signal here.

Copilot · 2026-04-13T08:10:03Z

plugins/opencode/scripts/lib/prompts.mjs

+  // Step 1: When --path is specified, collect folder context instead of git diff
+  if (opts.paths && opts.paths.length > 0 && !opts.pr) {
+    folderContext = await collectFolderContext(cwd, opts.paths, {
+      maxBytes,
+      maxFiles,
+    });
+    changedFiles = folderContext.files;


There are existing tests for buildReviewPrompt, but the new --path branch (including directory traversal, byte/file caps, ignore/binary/symlink skipping, and --pr/--path interaction) is not covered. Add tests to prevent regressions and to validate the new behavior end-to-end.

JohnnyVicious requested a review from Copilot April 13, 2026 08:06

Copilot started reviewing on behalf of JohnnyVicious April 13, 2026 08:07 View session

chatgpt-codex-connector bot reviewed Apr 13, 2026

View reviewed changes

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Conversation

JohnnyVicious commented Apr 13, 2026

Summary

Changes

Testing

Uh oh!

JohnnyVicious commented Apr 13, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants