feat: NEXUS — platform integration milestone#201
Merged
flyingrobots merged 21 commits intomainfrom Feb 14, 2026
Merged
Conversation
added 7 commits
February 12, 2026 13:45
Serialize the graph to YAML or JSON in v1 import-compatible format, enabling round-trip export → edit → import workflows. - exportGraph: reads nodes/edges, excludes decision:/commit: nodes - serializeExport: YAML or JSON output - exportToFile: write to disk with format inference - CLI: git mind export [file] --format yaml|json --prefix <prefix> - 13 new tests (round-trip, prefix filter, system node exclusion) Refs #195
Add --from-markdown flag to the import command, enabling nodes and edges to be created from YAML frontmatter in markdown files. - parseFrontmatter: extract YAML between --- delimiters - extractGraphData: build nodes/edges from frontmatter fields - findMarkdownFiles: simple recursive glob (no new deps) - importFromMarkdown: orchestrates the full pipeline - Refactored importData from importFile for pipeline reuse - 19 new tests (parsing, edge extraction, glob, round-trip) Refs #196
Add repo:owner/name:prefix:identifier syntax for referencing nodes in other repositories, enabling cross-repo knowledge graph edges. - parseCrossRepoId, buildCrossRepoId, isCrossRepoId, extractRepo, qualifyNodeId - validateNodeId now accepts cross-repo format - extractPrefix returns inner prefix for cross-repo IDs - 'repo' added to SYSTEM_PREFIXES - git mind link --remote <owner/name> qualifies IDs - 19 new tests (parsing, validation, graph integration) Refs #197
Merge another repository's graph into the local graph with cross-repo node qualification. Auto-detects repo identifier from origin remote. - detectRepoIdentifier: parses SSH and HTTPS remote URLs - mergeFromRepo: reads remote graph, qualifies nodes, atomic patch - Preserves confidence + rationale, copies node properties - CLI: git mind merge --from <path> --repo-name <owner/name> - 12 new tests (qualification, idempotency, dry-run, properties) Refs #198
Composite action that runs git mind suggest on PRs and posts formatted markdown suggestions as a comment. - action.yml: composite action with Node.js 22 setup - action/post-comment.js: formats suggestions as markdown table - .github/git-mind.yml.example: example config Refs #199
Add markdown formatting for PR suggestion display with interactive /gitmind slash commands for accept/reject/accept-all. - formatSuggestionsAsMarkdown: markdown table with command footer - parseReviewCommand: parses /gitmind slash commands from comments - gitmind-review.yml: workflow handles slash commands in PR comments - post-comment.js refactored to use shared formatSuggestionsAsMarkdown - 12 new tests (formatting, command parsing) Refs #200
Contributor
|
Caution Review failedThe pull request is closed. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds export/import/merge features, cross-repo ID handling, epoch/time-travel support, CLI command extensions, GitHub Action + workflow for suggestions and slash-command review handling, PR formatting/parsing, and extensive tests for these flows. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Platform
participant WF as Workflow Runner
participant Node as Node.js Action
participant CLI as git-mind CLI
participant GHCLI as gh CLI
GH->>WF: PR opened or comment containing /gitmind
WF->>Node: checkout + setup Node 22 + npm ci
Node->>CLI: run `git mind suggest --json` (base..head)
CLI-->>Node: returns JSON suggestions
Node->>Node: formatSuggestionsAsMarkdown(JSON)
Node->>GHCLI: POST /repos/:owner/:repo/issues/:pr/comments (body)
GHCLI->>GH: create PR comment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/cli/format.js (1)
312-328:⚠️ Potential issue | 🟡 MinorOrphaned JSDoc:
formatImportResult's doc comment is now separated from its function.The new
formatExportResultblock (lines 317–328) was inserted between the JSDoc forformatImportResult(lines 312–316) and theformatImportResultfunction itself (line 330). This leavesformatImportResultwithout its JSDoc and creates a confusing sequence of two consecutive doc comments.Proposed fix: move `formatExportResult` above the `formatImportResult` JSDoc
+/** + * Format an export result for terminal display. + * `@param` {{stats: {nodes: number, edges: number}, path?: string}} result + * `@param` {boolean} [toStdout=false] + * `@returns` {string} + */ +export function formatExportResult(result, toStdout = false) { + if (toStdout) { + return `${chalk.green(figures.tick)} Exported ${result.stats.nodes} node(s), ${result.stats.edges} edge(s)`; + } + return `${chalk.green(figures.tick)} Exported ${result.stats.nodes} node(s), ${result.stats.edges} edge(s) to ${chalk.cyan(result.path)}`; +} + /** * Format an import result for terminal display. * `@param` {import('../import.js').ImportResult} result * `@returns` {string} */ -/** - * Format an export result for terminal display. - * `@param` {{stats: {nodes: number, edges: number}, path?: string}} result - * `@param` {boolean} [toStdout=false] - * `@returns` {string} - */ -export function formatExportResult(result, toStdout = false) { - if (toStdout) { - return `${chalk.green(figures.tick)} Exported ${result.stats.nodes} node(s), ${result.stats.edges} edge(s)`; - } - return `${chalk.green(figures.tick)} Exported ${result.stats.nodes} node(s), ${result.stats.edges} edge(s) to ${chalk.cyan(result.path)}`; -} - export function formatImportResult(result) {bin/git-mind.js (1)
65-87:⚠️ Potential issue | 🔴 Critical
BOOLEAN_FLAGSis missing'dry-run'and'validate', causing flag-eating bugs.When
parseFlagsencounters--dry-run, it's not inBOOLEAN_FLAGS, so it consumes the next argument as its value. For example:git mind merge --from /path --dry-run --json
parseFlagssetsflags['dry-run'] = '--json'and skips over--json, soflags.jsonis never set. The--jsonoutput is silently lost.The same issue affects
--validatein the import path, and--dry-runin both the import and merge paths.Proposed fix
-const BOOLEAN_FLAGS = new Set(['json', 'fix']); +const BOOLEAN_FLAGS = new Set(['json', 'fix', 'dry-run', 'validate']);Then you can simplify the import/merge cases to use
parseFlagsconsistently instead of mixingparseFlagswithargs.includes():case 'import': { const importFlags = parseFlags(args.slice(1)); - const dryRun = args.includes('--dry-run') || args.includes('--validate'); - const jsonMode = args.includes('--json'); + const dryRun = importFlags['dry-run'] === true || importFlags['validate'] === true; + const jsonMode = importFlags.json === true;case 'merge': { const mergeFlags = parseFlags(args.slice(1)); await mergeCmd(cwd, { from: mergeFlags.from, repoName: mergeFlags['repo-name'], - dryRun: args.includes('--dry-run'), - json: mergeFlags.json ?? false, + dryRun: mergeFlags['dry-run'] === true, + json: mergeFlags.json === true, });
🤖 Fix all issues with AI agents
In @.github/workflows/gitmind-review.yml:
- Around line 38-65: The accept/reject branches currently only set a REPLY
message and do not perform any action; update the accept and reject cases so
they invoke the git-mind CLI to apply the change for the given INDEX (similar to
how accept-all runs node bin/git-mind.js review --batch accept --json), e.g.,
call node bin/git-mind.js review with the appropriate flags for single-item
accept/reject using the INDEX variable, validate that INDEX is non-empty and
numeric before invoking, capture the CLI exit status and set REPLY accordingly,
and ensure INDEX is passed safely (quoted) to avoid injection.
- Around line 19-22: The workflow dangerously checks out untrusted PR code using
actions/checkout@v4 with ref: ${{ github.event.issue.pull_request.head.ref }}
(and then runs npm ci / node bin/git-mind.js) on an issue_comment event;
github.event.issue.pull_request.head.ref is not available and this permits
remote code execution with elevated permissions. Fix by avoiding direct checkout
of PR branches in this privileged workflow: either convert to a low-privilege
trigger (e.g., workflow_run or a separate workflow without secrets), or before
any checkout call use the GitHub API (e.g., gh pr view) to fetch the PR head SHA
and pin actions/checkout@v4 to that SHA (pr_head_sha) instead of a branch ref,
and add a permissions/owner check to ensure the commenter is a trusted
collaborator; alternatively remove the npm ci / node bin/git-mind.js run from
this workflow entirely and perform it in a safe, reviewed context.
In `@action.yml`:
- Around line 4-11: The action currently declares the "agent" input optional
which lets runs proceed silently if neither the agent input nor the
GITMIND_AGENT env var is provided; make this explicit by either marking the
"agent" input as required in action.yml (set required: true for the "agent"
input) or add a runtime check before the suggest step that validates
process.env.GITMIND_AGENT || inputs.agent and fails with a clear error message
when missing, ensuring the check references the "agent" input name and the
GITMIND_AGENT env var so users see why the action stops.
- Around line 31-37: The current run step redirects stderr into stdout (the
`2>&1` on the `node ... git-mind.js suggest` invocation), which can corrupt the
JSON stored in RESULT; change the invocation so stdout and stderr are captured
separately (e.g., capture stdout into RESULT and stderr into a separate variable
like SUGGEST_ERR) instead of using `2>&1`, then write both variables to
GITHUB_OUTPUT under distinct keys so `post-comment.js` can parse RESULT reliably
while stderr is available for debugging; update references to the `git-mind.js
suggest` invocation and the RESULT variable accordingly.
In `@action/post-comment.js`:
- Line 14: The variables repo and prNumber are taken from process.argv and later
interpolated into a shell command unsafely; validate and sanitize them where
they’re declared (const [repo, prNumber] = process.argv.slice(2)) by enforcing
repo matches the owner/repo pattern (e.g., /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/)
and prNumber is a positive integer, and then avoid shell interpolation by
calling the command API that accepts args (e.g., child_process.spawn or
execFile) with repo and prNumber as separate arguments (or safely escape them)
instead of embedding them into a single shell string. Ensure you throw/exit with
a clear error when validation fails.
- Around line 35-39: The current execSync call in action/post-comment.js
interpolates the JSON.stringify(body) into a shell string (the execSync
invocation), which allows backticks, $, and other metacharacters in the markdown
to be interpreted by the shell; change the call that invokes execSync so it does
not embed body in the command string but instead pipes the body via stdin (use
the gh api option --input -) and pass JSON.stringify(body) via the execSync
options.input property; update the execSync invocation around the call site (the
execSync(...) line) to run "gh api repos/${repo}/issues/${prNumber}/comments
--input -" and supply the body through options.input to eliminate shell
interpretation of backticks and other metacharacters.
In `@src/format-pr.js`:
- Around line 22-27: The markdown table breaks when fields contain pipe
characters; update the loop that builds lines (iterating suggestions, using
variables s.rationale and s.type) to escape any '|' characters before composing
the table row—e.g., transform s.rationale and s.type by replacing '|' with an
escaped form (such as '\|' or '|') and then use those sanitized values when
pushing the row into lines; ensure you handle null/undefined rationale
(s.rationale ?? '') and keep the existing formatting for conf, source, and
target.
In `@src/frontmatter.js`:
- Around line 92-136: The current findMarkdownFiles function implements a
simplified glob that correctly handles patterns like '**/*.md', 'docs/**/*.md',
'*.md', and 'docs/*.md' but will silently return empty results for more complex
globs or exact file paths; update the implementation or documentation: either
(A) expand findMarkdownFiles to delegate to a lightweight glob library such as
fast-glob (replace the manual walk logic and startDir calculation with fast-glob
invocation) so complex patterns (e.g., 'docs/202?/*.md' and exact file paths)
are supported, or (B) explicitly document the supported patterns in the JSDoc
above findMarkdownFiles (mention supported patterns, that exact file paths are
not matched, and the behavior for non-existent start directories) so callers
know the limitations; reference the findMarkdownFiles function and its internal
walk logic when making the change.
- Around line 22-42: parseFrontmatter assumes Unix line endings by slicing with
fixed offsets (slice(4, endIdx)) which leaves a stray '\r' for Windows CRLF
files; update parseFrontmatter to detect the actual line ending after the
opening '---' (e.g., find the first newline sequence after the opening delimiter
or normalize line endings) and compute the yamlBlock start dynamically instead
of hardcoding 4, and similarly make the closing delimiter detection robust to
'\r\n' (or normalize content with content = content.replace(/\r\n/g, '\n')
before processing); adjust variables yamlBlock and endIdx logic and still use
yaml.load as before.
In `@src/import.js`:
- Around line 257-267: The local extraction const dryRun = opts.dryRun is
redundant in importFile because importData already reads dryRun from opts;
remove the local extraction and use opts.dryRun in the early-return error object
(or alternatively pass { ...opts, dryRun } explicitly into importData), updating
importFile and keeping the parseImportFile error-return shape unchanged so
importData still receives opts as before; refer to importFile, parseImportFile
and importData to locate the change.
In `@src/remote.js`:
- Around line 43-45: buildCrossRepoId currently concatenates repo and localId
without confirming localId uses the required "prefix:identifier" form, causing
produced IDs to fail CROSS_REPO_ID_REGEX/isCrossRepoId downstream; update
buildCrossRepoId to validate localId before concatenation (e.g., assert it
matches a /^[^:]+:[^:]+$/ pattern or use an existing helper) and throw a clear
Error mentioning buildCrossRepoId when the check fails (or optionally
normalize/transform localId if a safe rule exists), and include references to
CROSS_REPO_ID_REGEX and isCrossRepoId in the error message so consumers can
trace validation expectations.
In `@test/frontmatter.test.js`:
- Around line 27-33: The writeMd helper is using manual string splitting to
derive the directory from fullPath; replace that logic by importing and using
dirname from node:path and call dirname(fullPath) instead of
fullPath.substring(0, fullPath.lastIndexOf('/')); update the writeMd function
(and imports if needed) so it uses dirname to compute dir before mkdir to make
it platform-safe.
Comment on lines
+31
to
+37
| run: | | ||
| BASE_SHA="${{ github.event.pull_request.base.sha }}" | ||
| HEAD_SHA="${{ github.event.pull_request.head.sha }}" | ||
| RESULT=$(node ${{ github.action_path }}/bin/git-mind.js suggest --json --context "${BASE_SHA}..${HEAD_SHA}" 2>&1) || true | ||
| echo "result<<GITMIND_EOF" >> "$GITHUB_OUTPUT" | ||
| echo "$RESULT" >> "$GITHUB_OUTPUT" | ||
| echo "GITMIND_EOF" >> "$GITHUB_OUTPUT" |
Contributor
There was a problem hiding this comment.
2>&1 mixes stderr into the JSON result, corrupting it when warnings are emitted.
Redirecting stderr into stdout means any warnings or error messages from the suggest command get concatenated with the JSON output, making it unparseable. The post-comment.js fallback handles this gracefully (skips on invalid JSON), but it means suggestions are silently lost whenever stderr output is present.
Capture them separately instead:
Proposed fix: separate stdout from stderr
run: |
BASE_SHA="${{ github.event.pull_request.base.sha }}"
HEAD_SHA="${{ github.event.pull_request.head.sha }}"
- RESULT=$(node ${{ github.action_path }}/bin/git-mind.js suggest --json --context "${BASE_SHA}..${HEAD_SHA}" 2>&1) || true
+ RESULT=$(node ${{ github.action_path }}/bin/git-mind.js suggest --json --context "${BASE_SHA}..${HEAD_SHA}" 2>/dev/null) || true
echo "result<<GITMIND_EOF" >> "$GITHUB_OUTPUT"
echo "$RESULT" >> "$GITHUB_OUTPUT"
echo "GITMIND_EOF" >> "$GITHUB_OUTPUT"Or better yet, capture stderr separately for debugging:
run: |
BASE_SHA="${{ github.event.pull_request.base.sha }}"
HEAD_SHA="${{ github.event.pull_request.head.sha }}"
- RESULT=$(node ${{ github.action_path }}/bin/git-mind.js suggest --json --context "${BASE_SHA}..${HEAD_SHA}" 2>&1) || true
+ RESULT=$(node ${{ github.action_path }}/bin/git-mind.js suggest --json --context "${BASE_SHA}..${HEAD_SHA}" 2>suggest_stderr.log) || true
+ if [ -s suggest_stderr.log ]; then cat suggest_stderr.log >&2; fi
echo "result<<GITMIND_EOF" >> "$GITHUB_OUTPUT"
echo "$RESULT" >> "$GITHUB_OUTPUT"
echo "GITMIND_EOF" >> "$GITHUB_OUTPUT"📝 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
| run: | | |
| BASE_SHA="${{ github.event.pull_request.base.sha }}" | |
| HEAD_SHA="${{ github.event.pull_request.head.sha }}" | |
| RESULT=$(node ${{ github.action_path }}/bin/git-mind.js suggest --json --context "${BASE_SHA}..${HEAD_SHA}" 2>&1) || true | |
| echo "result<<GITMIND_EOF" >> "$GITHUB_OUTPUT" | |
| echo "$RESULT" >> "$GITHUB_OUTPUT" | |
| echo "GITMIND_EOF" >> "$GITHUB_OUTPUT" | |
| run: | | |
| BASE_SHA="${{ github.event.pull_request.base.sha }}" | |
| HEAD_SHA="${{ github.event.pull_request.head.sha }}" | |
| RESULT=$(node ${{ github.action_path }}/bin/git-mind.js suggest --json --context "${BASE_SHA}..${HEAD_SHA}" 2>/dev/null) || true | |
| echo "result<<GITMIND_EOF" >> "$GITHUB_OUTPUT" | |
| echo "$RESULT" >> "$GITHUB_OUTPUT" | |
| echo "GITMIND_EOF" >> "$GITHUB_OUTPUT" |
Suggested change
| run: | | |
| BASE_SHA="${{ github.event.pull_request.base.sha }}" | |
| HEAD_SHA="${{ github.event.pull_request.head.sha }}" | |
| RESULT=$(node ${{ github.action_path }}/bin/git-mind.js suggest --json --context "${BASE_SHA}..${HEAD_SHA}" 2>&1) || true | |
| echo "result<<GITMIND_EOF" >> "$GITHUB_OUTPUT" | |
| echo "$RESULT" >> "$GITHUB_OUTPUT" | |
| echo "GITMIND_EOF" >> "$GITHUB_OUTPUT" | |
| run: | | |
| BASE_SHA="${{ github.event.pull_request.base.sha }}" | |
| HEAD_SHA="${{ github.event.pull_request.head.sha }}" | |
| RESULT=$(node ${{ github.action_path }}/bin/git-mind.js suggest --json --context "${BASE_SHA}..${HEAD_SHA}" 2>suggest_stderr.log) || true | |
| if [ -s suggest_stderr.log ]; then cat suggest_stderr.log >&2; fi | |
| echo "result<<GITMIND_EOF" >> "$GITHUB_OUTPUT" | |
| echo "$RESULT" >> "$GITHUB_OUTPUT" | |
| echo "GITMIND_EOF" >> "$GITHUB_OUTPUT" |
🤖 Prompt for AI Agents
In `@action.yml` around lines 31 - 37, The current run step redirects stderr into
stdout (the `2>&1` on the `node ... git-mind.js suggest` invocation), which can
corrupt the JSON stored in RESULT; change the invocation so stdout and stderr
are captured separately (e.g., capture stdout into RESULT and stderr into a
separate variable like SUGGEST_ERR) instead of using `2>&1`, then write both
variables to GITHUB_OUTPUT under distinct keys so `post-comment.js` can parse
RESULT reliably while stderr is available for debugging; update references to
the `git-mind.js suggest` invocation and the RESULT variable accordingly.
| import { execSync } from 'node:child_process'; | ||
| import { formatSuggestionsAsMarkdown } from '../src/format-pr.js'; | ||
|
|
||
| const [repo, prNumber] = process.argv.slice(2); |
Contributor
There was a problem hiding this comment.
Unsanitized repo and prNumber are interpolated into a shell command.
While these values come from the GitHub Action context, it's good practice to validate them before shell interpolation.
Proposed validation
const [repo, prNumber] = process.argv.slice(2);
if (!repo || !prNumber) {
console.error('Usage: post-comment.js <owner/repo> <pr-number>');
process.exit(1);
}
+
+if (!/^[A-Za-z0-9._-]+\/[A-Za-z0-9._-]+$/.test(repo)) {
+ console.error('Invalid repo format. Expected owner/name.');
+ process.exit(1);
+}
+if (!/^\d+$/.test(prNumber)) {
+ console.error('Invalid PR number. Expected an integer.');
+ process.exit(1);
+}🤖 Prompt for AI Agents
In `@action/post-comment.js` at line 14, The variables repo and prNumber are taken
from process.argv and later interpolated into a shell command unsafely; validate
and sanitize them where they’re declared (const [repo, prNumber] =
process.argv.slice(2)) by enforcing repo matches the owner/repo pattern (e.g.,
/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/) and prNumber is a positive integer, and
then avoid shell interpolation by calling the command API that accepts args
(e.g., child_process.spawn or execFile) with repo and prNumber as separate
arguments (or safely escape them) instead of embedding them into a single shell
string. Ensure you throw/exit with a clear error when validation fails.
) Security: - Workflow checks out default branch, not PR head (untrusted code) - post-comment.js passes body via stdin, validates repo/prNumber - action.yml separates stderr from JSON output Bugs: - BOOLEAN_FLAGS now includes dry-run and validate (flag-eating fix) - Pipe chars escaped in markdown table cells - Frontmatter parser handles CRLF line endings - buildCrossRepoId validates localId format Cleanup: - Orphaned JSDoc reordered in format.js - Accept/reject stubs now respond with "not yet supported" - dirname() used instead of manual string splitting - Redundant dryRun extraction removed from importFile Refs #195, #196, #197, #198, #199, #200
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.github/workflows/gitmind-review.yml:
- Around line 50-54: The accept-all case currently runs the command node
bin/git-mind.js review --batch accept --json but unconditionally sets REPLY="All
pending suggestions accepted."; change this to run the command and check its
exit status (or use an if/then around the command) before assigning REPLY so
that on success you set REPLY="All pending suggestions accepted." and on failure
you set a different message (e.g., include the command exit code or an error
note); update the accept-all branch in the case block that references ACTION and
REPLY to conditionally set REPLY based on the command's outcome.
In `@bin/git-mind.js`:
- Around line 154-172: The import branch mixes flag parsing strategies: it uses
parseFlags to build importFlags but then reads --dry-run and --json directly
from args into dryRun and jsonMode, creating inconsistency with other commands;
change it to use importFlags for these flags instead (e.g., const dryRun =
importFlags['dry-run'] and const jsonMode = importFlags['json']) so both
importMarkdownCmd and importCmd receive flags consistently; update any variable
names if needed and keep parseFlags/importFlags as the single source of truth in
this case.
In `@src/format-pr.js`:
- Around line 6-9: The escapeCell function fails to escape backslashes so inputs
like "\|" become "\\|" and still create an unescaped pipe in Markdown; update
escapeCell to first escape backslashes (replace each "\" with "\\") and only
then escape pipe characters (replace "|" with "\|") so backslashes are preserved
and pipes are safely escaped; ensure you perform the backslash replacement
before the pipe replacement in the escapeCell function.
In `@src/frontmatter.js`:
- Around line 163-184: The variable warnings declared in the top of the
collection logic is never used; remove the dead const warnings = []; and any
references to it so the loop that builds nodes and edges (inside the for (const
filePath of files) {...}), the extractGraphData(relPath, frontmatter) call, and
the subsequent edges.push(edge) handling remain unchanged; ensure no callers
expect a warnings variable from this module and run tests to verify nothing else
references warnings.
In `@src/remote.js`:
- Around line 43-48: The buildCrossRepoId function currently allows localId
values with extra colons (e.g., "a:b:c") which produce repo:... IDs that won't
match isCrossRepoId/CROSS_REPO_ID_REGEX; update buildCrossRepoId to validate
more strictly by either checking localId against the local node ID pattern used
elsewhere or by composing the candidate id (`repo:${repo}:${localId}`) and
testing it against CROSS_REPO_ID_REGEX (or calling isCrossRepoId) and throwing
an Error if it fails; reference buildCrossRepoId and
CROSS_REPO_ID_REGEX/isCrossRepoId so the fix ensures returned IDs always match
the canonical cross-repo regex.
In `@test/format-pr.test.js`:
- Around line 50-64: Add a test that ensures backslashes in suggestion rationale
are escaped (or preserved safely) so escapeCell in format-pr.js handles
backslashes; update test/format-pr.test.js by adding a new it(...) that uses
formatSuggestionsAsMarkdown with a suggestion whose rationale contains
backslashes (e.g., "path\\to\\file") and asserts the output contains the
expected escaped/preserved tokens (reference formatSuggestionsAsMarkdown and
escapeCell to locate the escaping logic), and if failing, modify escapeCell to
first escape backslashes (replace "\" with "\\") before escaping pipe
characters.
…199, #200) - Use parseFlags consistently in import/merge (fix flag-eating bugs) - Add --index support to review command for individual accept/reject - Add collaborator permission check to review workflow - Implement accept/reject by index in workflow (invoke CLI) - Add agent validation step and separate stderr in action.yml - Normalize CRLF in parseFrontmatter - Document findMarkdownFiles supported patterns and limitations - Strengthen buildCrossRepoId validation with proper regex
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/cli/commands.js (1)
46-61: 🧹 Nitpick | 🔵 Trivial
--remotequalifies both source and target — is this the intended UX?When
--remoteis provided, bothsourceandtargetare qualified with the same repo. This meansgit mind link task:a spec:b --remote owner/repocreates an edge between two remote nodes. If the intent is to link a local node to a remote node, the user would need to manually use the fullrepo:owner/repo:prefix:idsyntax for one side and omit--remote. Consider documenting this behavior or adding--remote-source/--remote-targetfor finer control.bin/git-mind.js (1)
229-237:⚠️ Potential issue | 🟡 Minor
--indexwithout--batchsilently does nothing.If a user runs
git mind review --index 3(without--batch), theindexoption is parsed but the review function's interactive mode ignores it — all pending suggestions are shown. Consider validating that--indexrequires--batch.♻️ Proposed fix in bin/git-mind.js
case 'review': { const reviewFlags = parseFlags(args.slice(1)); + if (reviewFlags.index && !reviewFlags.batch) { + console.error('--index requires --batch accept|reject'); + process.exitCode = 1; + break; + } await review(cwd, { batch: reviewFlags.batch, index: reviewFlags.index ? parseInt(reviewFlags.index, 10) : undefined, json: reviewFlags.json ?? false, }); break; }
🤖 Fix all issues with AI agents
In @.github/workflows/gitmind-review.yml:
- Around line 7-9: The workflow's explicit permissions block is missing the
contents scope so actions/checkout and any write-back via "git mind review
--batch accept" will fail; update the permissions map by adding "contents: read"
to allow actions/checkout to fetch the repo and add "contents: write" (or change
to "contents: read, write" / "contents: write" if your job needs to update
refs/notes) so graph writes succeed; ensure the permissions block includes the
new "contents" key alongside the existing "pull-requests" and "issues" entries.
In `@action.yml`:
- Around line 26-34: The step "Validate agent configuration" currently forces
GITMIND_AGENT to inputs.agent (overriding workflow-level env), so change the env
assignment to fall back to the inherited environment variable (use the GitHub
Actions expression that prefers inputs.agent but uses env.GITMIND_AGENT when
inputs.agent is empty) and apply the same fix to the "suggest" step; update
references to GITMIND_AGENT and inputs.agent in those steps so the validation
uses the effective environment value rather than always emptying it.
In `@src/cli/commands.js`:
- Around line 316-349: The stdout branch of exportCmd currently prints the raw
graph data when opts.json is true, causing asymmetry with file mode which
returns metadata from exportToFile; change the stdout JSON output to match
file-mode metadata shape (e.g., { stats, path }) instead of the full graph. In
practice, after calling exportGraph(graph, { prefix: opts.prefix }) produce the
same metadata object that exportToFile returns (populate stats and set path to
null or "-" for stdout) and console.log(JSON.stringify(metadata, null, 2))
rather than logging the raw data; keep serializeExport/formatExportResult
behavior for non-JSON paths so exportCmd, exportGraph, exportToFile,
serializeExport and formatExportResult remain consistent.
In `@src/frontmatter.js`:
- Around line 108-152: The function findMarkdownFiles currently swallows all
errors in the try/catch around stat(startDir); change the catch to inspect the
caught error (e.g., err) and only return an empty results array for
filesystem-not-found errors (err.code === 'ENOENT' or err.code === 'ENOTDIR');
for any other error (permissions, EMFILE, etc.) rethrow the error so it
surfaces. Update the catch block that follows the stat/startDir logic
(references: findMarkdownFiles, startDir, stat, readdir, walk) to implement this
conditional check and rethrow behavior.
In `@src/remote.js`:
- Around line 83-86: qualifyNodeId currently forwards non-prefixed local IDs to
buildCrossRepoId which throws a generic "invalid localId"; update qualifyNodeId
to validate the input first (use isCrossRepoId(nodeId) and ensure nodeId is a
non-empty string and/or contains the expected separator) and throw a clearer,
contextual error if the precondition isn't met (include the offending nodeId and
repo in the message), or alternatively document the required format at the top
of qualifyNodeId; reference qualifyNodeId, isCrossRepoId, and buildCrossRepoId
when making this change.
Comment on lines
+316
to
+349
| /** | ||
| * Export the graph to a file or stdout. | ||
| * @param {string} cwd | ||
| * @param {{ file?: string, format?: string, prefix?: string, json?: boolean }} opts | ||
| */ | ||
| export async function exportCmd(cwd, opts = {}) { | ||
| try { | ||
| const graph = await loadGraph(cwd); | ||
| const format = opts.format ?? 'yaml'; | ||
|
|
||
| if (opts.file) { | ||
| const result = await exportToFile(graph, opts.file, { format, prefix: opts.prefix }); | ||
|
|
||
| if (opts.json) { | ||
| console.log(JSON.stringify(result, null, 2)); | ||
| } else { | ||
| console.log(formatExportResult(result)); | ||
| } | ||
| } else { | ||
| // stdout mode | ||
| const data = await exportGraph(graph, { prefix: opts.prefix }); | ||
|
|
||
| if (opts.json) { | ||
| console.log(JSON.stringify(data, null, 2)); | ||
| } else { | ||
| const output = serializeExport(data, format); | ||
| process.stdout.write(output); | ||
| } | ||
| } | ||
| } catch (err) { | ||
| console.error(error(err.message)); | ||
| process.exitCode = 1; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
exportCmd stdout mode: --json outputs raw graph data, not metadata.
In file mode, --json outputs { stats, path } metadata. In stdout mode, --json outputs the full graph data { version, nodes, edges }. This asymmetry could surprise users. A note in the usage help or docs would help clarify.
🤖 Prompt for AI Agents
In `@src/cli/commands.js` around lines 316 - 349, The stdout branch of exportCmd
currently prints the raw graph data when opts.json is true, causing asymmetry
with file mode which returns metadata from exportToFile; change the stdout JSON
output to match file-mode metadata shape (e.g., { stats, path }) instead of the
full graph. In practice, after calling exportGraph(graph, { prefix: opts.prefix
}) produce the same metadata object that exportToFile returns (populate stats
and set path to null or "-" for stdout) and console.log(JSON.stringify(metadata,
null, 2)) rather than logging the raw data; keep
serializeExport/formatExportResult behavior for non-JSON paths so exportCmd,
exportGraph, exportToFile, serializeExport and formatExportResult remain
consistent.
added 2 commits
February 13, 2026 02:29
Prevents `\|` in rationale from being interpreted as a raw pipe delimiter in markdown tables.
- Remove dead `warnings` variable in importFromMarkdown - Use isCrossRepoId for end-to-end validation in buildCrossRepoId - Add backslash escaping test for formatSuggestionsAsMarkdown
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/format-pr.js`:
- Around line 29-34: The loop in format-pr.js builds markdown table rows using
s.source and s.target wrapped in backticks but doesn't escape backticks, so
values like `file:\`auth\`.js` break the table; modify the template in the for
loop (where suggestions are iterated) to pass s.source and s.target through the
existing escapeCell function (or a dedicated backtick-escaping helper) before
surrounding them with backticks (i.e., use escapeCell(s.source) and
escapeCell(s.target) or escape backticks inside those strings) so table cells
remain valid markdown.
- Around line 59-75: The parser parseReviewCommand currently accepts index 0
(e.g., "/gitmind accept 0"); update parseReviewCommand so it treats 0 as invalid
for 1-indexed suggestions by validating the parsed index is >= 1 (or change the
regex to only allow positive integers like [1-9]\d*), and return null for
invalid/zero indices; locate the logic around match/command/index and add the
index >= 1 check (or adjust the regex) to ensure accept/reject always require a
1-based index.
In `@src/frontmatter.js`:
- Line 59: The code computes pathWithoutExt by calling
relativePath.replace(extname(relativePath), '') which can remove an extension
from a directory name if the same substring appears earlier; change the removal
to target only the filename's extension (e.g., use the filename or last
occurrence): obtain the filename or the last index of extname(relativePath) and
strip only that suffix so pathWithoutExt reflects the full path minus the final
extension; update the logic around relativePath, pathWithoutExt, and
extname(relativePath) accordingly.
In `@test/format-pr.test.js`:
- Around line 113-141: Add a unit test to the parseReviewCommand suite to cover
the edge case "/gitmind accept 0": inside the describe('parseReviewCommand')
block add an it test named like "returns null for accept with index 0" and
assert that parseReviewCommand('/gitmind accept 0') returns null, referencing
the parseReviewCommand function to document that zero is treated as invalid in
1-based indexing.
Add `git mind at <ref>` command that materializes the graph at a historical point in time using CRDT Lamport tick correlation. - New src/epoch.js module (getCurrentTick, recordEpoch, lookupEpoch, lookupNearestEpoch, getEpochForRef) - processCommit auto-records epoch after each commit - epoch: added to SYSTEM_PREFIXES, excluded from export and doctor - CLI `at` command with --json support - 16 new tests (304 total across 19 files) - Fix README/GUIDE false claims about git checkout time-travel
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@bin/git-mind.js`:
- Around line 167-185: The import command assumes the file path is at args[1]
(importPath) which breaks when flags precede the positional argument; change the
logic in the 'import' case to locate the first non-flag positional argument from
args.slice(1) (skip any token that starts with '-'/ '--' or is a flag value) and
use that as importPath, keeping the existing dryRun/json/importMarkdown handling
and calls to importCmd and importMarkdownCmd; replace the const importPath =
args[1] assignment with a scan that finds the first arg not starting with '-'
and validate it the same way before calling importCmd.
In `@src/cli/commands.js`:
- Around line 288-293: Remove the unnecessary direct write to the private
property graph._seekCeiling: delete the assignment to graph._seekCeiling =
epoch.tick and rely solely on the public API call graph.materialize({ ceiling:
epoch.tick }); ensure any subsequent logic (e.g., computeStatus(graph)) uses the
materialized state rather than the removed private property so there are no
remaining references to graph._seekCeiling.
In `@src/cli/format.js`:
- Around line 344-359: The byPrefix/byType rendering in formatAtStatus
duplicates logic in formatStatus; extract a small shared helper (e.g.,
renderCountTable or renderPrefixTypeTable) used by both formatStatus and
formatAtStatus to iterate Object.entries(...).sort(...) and push formatted lines
(yellow key padded to 14 and count padded to 3, plus any separating blank line),
and have formatStatus call the helper and then add its percentage calculation as
before while formatAtStatus uses the helper without percentages; update
references to the existing status.nodes.byPrefix, status.edges.byType, and the
functions formatStatus/formatAtStatus to call the new helper.
In `@src/doctor.js`:
- Around line 99-100: Replace the hardcoded prefix checks in the nodes filter
with a check against the existing SYSTEM_PREFIXES array from validators.js and
handle "decision" separately (or add it to a small dedicated exclusion list) so
system-generated nodes like "commit:" are excluded consistently; specifically,
update the filter expression around nodes.filter(...) to consult SYSTEM_PREFIXES
(importing it from validators.js) to skip any node whose prefix is in
SYSTEM_PREFIXES and also skip nodes starting with "decision:" (or include
"decision" in a new EXCLUDE_PREFIXES array) while preserving the existing
connected.has(n) check.
In `@src/epoch.js`:
- Around line 38-53: The epoch node ID uses an 8-char truncated SHA (sha8/nodeId
via commitSha.slice(0, 8)), creating a collision risk; update recordEpoch to use
a longer unique suffix (e.g., commitSha.slice(0, 12) or the full commitSha) when
constructing nodeId (change sha8 to something like sha12 or fullShaSuffix and
build nodeId = `epoch:${sha12}` or `epoch:${commitSha}`), keep storing the full
SHA in the 'fullSha' property, and ensure any related existence checks
(graph.hasNode(nodeId)) and other references use the new nodeId strategy.
- Around line 89-99: The execSync calls in lookupNearestEpoch (using
interpolated targetSha and maxWalk) and in getEpochForRef (using ref) are
vulnerable to shell injection; replace them with execFileSync/execFile and pass
the git command and its arguments as an array (e.g., ['rev-list','--max-count',
String(maxWalk), targetSha]) so values are treated as args not shell text,
validate or coerce maxWalk to an integer and ensure targetSha/ref are
expected-length hex strings before use, update imports to remove execSync, and
adjust error handling to work with execFileSync.
In `@src/export.js`:
- Around line 109-113: serializeExport currently silently falls back to YAML
when an unsupported format is passed; update serializeExport to validate the
format parameter (e.g., allowedFormats = ['json','yaml']) and if the value is
not allowed either throw a clear Error listing supported formats or explicitly
default while emitting a warning; modify the function body around the format
check in serializeExport to perform this validation and return/throw
appropriately so callers cannot unknowingly receive YAML when they requested an
unsupported format.
In `@src/hooks.js`:
- Around line 67-71: The empty catch around getCurrentTick(graph) /
recordEpoch(graph, commit.sha, tick) swallows failures; change it to catch the
error (e.g., catch (err)) and emit a debug-level log that includes the error and
contextual info (commit.sha and graph) so epoch recording failures are
observable; use the existing logger in the module (or console.debug if none) and
keep the operation non-fatal.
In `@test/epoch.test.js`:
- Around line 176-183: The test redundantly asserts that issues.find(...) is
undefined immediately after expect(issues).toHaveLength(0); remove the second
assertion so the case only checks that detectOrphanNodes(nodes, edges) returns
an empty array; locate the test using the it block that mentions "excludes epoch
nodes from orphan detection" and the issues variable and delete the
expect(issues.find(i => i.affected[0] === 'epoch:abc123de')).toBeUndefined()
line (or alternatively replace the toHaveLength(0) with only that specific
.find(...) assertion if you prefer a more targeted check).
added 7 commits
February 13, 2026 03:29
Replace execSync string interpolation with execFileSync array args in lookupNearestEpoch and getEpochForRef, preventing shell injection via crafted ref names or SHAs.
When permissions is explicitly set, unspecified scopes default to none. The workflow needs contents:read for actions/checkout to fetch the repo.
The validation step was overriding inherited GITMIND_AGENT with inputs.agent (empty when not set), making the env var path unusable. Now checks INPUT_AGENT first, falls back to inherited GITMIND_AGENT. The suggest step also falls back to env.GITMIND_AGENT.
- parseReviewCommand now returns null for index 0 (suggestions are 1-indexed, so 0 is invalid) - formatSuggestionsAsMarkdown strips backtick characters from source/target to prevent breaking inline code spans in the table - 3 new regression tests
- findMarkdownFiles now only catches ENOENT/ENOTDIR, re-throwing permission errors and other unexpected failures - extractGraphData uses slice instead of String.replace to strip the file extension, preventing directory names like notes.md/ from being incorrectly modified - 2 new regression tests
qualifyNodeId now throws a descriptive error when given a node ID without a prefix:identifier format (e.g. "readme"), instead of falling through to buildCrossRepoId's generic validation error.
Owner
Author
|
@coderabbitai review please |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/workflows/gitmind-review.yml:
- Around line 57-64: Replace the unsafe echo usage when reading COMMENT_BODY
with a predictable printf to avoid special-case interpretation (e.g., change
echo "$COMMENT_BODY" to printf '%s' "$COMMENT_BODY") in the shell block that
sets COMMAND; ensure any other uses of echo on untrusted input in that block are
converted to printf '%s' "<var>" so grep -oP can operate on the raw content
without shell-dependent mangling.
In `@src/format-pr.js`:
- Around line 29-35: The loop over suggestions can throw if s.source, s.target,
or s.confidence are undefined; update the block that builds
conf/rationale/source/target (the variables conf, rationale, source, target
inside the for-loop that uses escapeCell and lines.push) to defensively coerce
defaults: treat s.source and s.target as empty strings before calling
.replace/escapeCell, treat s.confidence as 0 when absent before computing
percentage, and treat s.rationale and s.type as empty strings if missing; ensure
you only call .replace on a string (e.g., String(s.source).replace(...)) and
then pass those safe values into escapeCell and lines.push so runtime errors are
avoided.
In `@src/frontmatter.js`:
- Around line 30-36: The current end-of-frontmatter search using
content.indexOf('\n---', firstNewline) can match '---' followed by other
characters; update the logic that computes endIdx (and any surrounding
parseFrontmatter function) to only accept a delimiter that is a standalone line
by searching for '\n---\n' or, if matching '\n---' at the end of the file,
ensuring the character after the match is either a newline or EOF; implement
this by either using a regex like /\n---(\r?\n|$)/g starting from firstNewline
or by iterating occurrences of '\n---' and checking content[matchPos + 4] is
undefined or '\n' (and handle CRLF), then use that validated index as endIdx and
return frontmatter/body accordingly.
In `@test/frontmatter.test.js`:
- Around line 154-168: The test redundantly uses a dynamic import for
mkdir/chmod; replace the dynamic import and local mkdirFs/chmodFs usage by the
existing statically-imported mkdir and a new static import of chmod, then update
the test to call chmod(restrictedDir, 0o000) and chmod(restrictedDir, 0o755) in
the try/finally block and use mkdir(tempDirPath) as before; ensure the test
still references findMarkdownFiles, restrictedDir and tempDir correctly.
src/format-pr.js
Outdated
Comment on lines
+29
to
+35
| for (let i = 0; i < suggestions.length; i++) { | ||
| const s = suggestions[i]; | ||
| const conf = `${(s.confidence * 100).toFixed(0)}%`; | ||
| const rationale = escapeCell(s.rationale ?? ''); | ||
| const source = escapeCell(s.source.replace(/`/g, '')); | ||
| const target = escapeCell(s.target.replace(/`/g, '')); | ||
| lines.push(`| ${i + 1} | \`${source}\` | \`${target}\` | ${escapeCell(s.type)} | ${conf} | ${rationale} |`); |
Contributor
There was a problem hiding this comment.
Potential crash if source, target, or confidence is undefined.
If a suggestion object is missing source, target, or confidence, the code will throw at runtime (e.g., Cannot read properties of undefined). The JSDoc types these as required, but since the data originates from an LLM agent response, a defensive guard would be prudent.
🛡️ Suggested defensive defaults
for (let i = 0; i < suggestions.length; i++) {
const s = suggestions[i];
- const conf = `${(s.confidence * 100).toFixed(0)}%`;
- const rationale = escapeCell(s.rationale ?? '');
- const source = escapeCell(s.source.replace(/`/g, ''));
- const target = escapeCell(s.target.replace(/`/g, ''));
+ const conf = s.confidence != null ? `${(s.confidence * 100).toFixed(0)}%` : '—';
+ const rationale = escapeCell(s.rationale ?? '');
+ const source = escapeCell((s.source ?? '').replace(/`/g, ''));
+ const target = escapeCell((s.target ?? '').replace(/`/g, ''));📝 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
| for (let i = 0; i < suggestions.length; i++) { | |
| const s = suggestions[i]; | |
| const conf = `${(s.confidence * 100).toFixed(0)}%`; | |
| const rationale = escapeCell(s.rationale ?? ''); | |
| const source = escapeCell(s.source.replace(/`/g, '')); | |
| const target = escapeCell(s.target.replace(/`/g, '')); | |
| lines.push(`| ${i + 1} | \`${source}\` | \`${target}\` | ${escapeCell(s.type)} | ${conf} | ${rationale} |`); | |
| for (let i = 0; i < suggestions.length; i++) { | |
| const s = suggestions[i]; | |
| const conf = s.confidence != null ? `${(s.confidence * 100).toFixed(0)}%` : '—'; | |
| const rationale = escapeCell(s.rationale ?? ''); | |
| const source = escapeCell((s.source ?? '').replace(/`/g, '')); | |
| const target = escapeCell((s.target ?? '').replace(/`/g, '')); | |
| lines.push(`| ${i + 1} | \`${source}\` | \`${target}\` | ${escapeCell(s.type)} | ${conf} | ${rationale} |`); |
🤖 Prompt for AI Agents
In `@src/format-pr.js` around lines 29 - 35, The loop over suggestions can throw
if s.source, s.target, or s.confidence are undefined; update the block that
builds conf/rationale/source/target (the variables conf, rationale, source,
target inside the for-loop that uses escapeCell and lines.push) to defensively
coerce defaults: treat s.source and s.target as empty strings before calling
.replace/escapeCell, treat s.confidence as 0 when absent before computing
percentage, and treat s.rationale and s.type as empty strings if missing; ensure
you only call .replace on a string (e.g., String(s.source).replace(...)) and
then pass those safe values into escapeCell and lines.push so runtime errors are
avoided.
- bin/git-mind.js: allow flags before import file path - src/cli/commands.js: remove redundant _seekCeiling assignment in at() - src/cli/format.js: extract renderCountTable helper to deduplicate - src/doctor.js: use SYSTEM_PREFIXES for orphan-node exclusion - src/epoch.js: widen SHA prefix from 8 to 12 chars - src/export.js: throw on unsupported format in serializeExport - src/hooks.js: log epoch errors when GITMIND_DEBUG is set - src/format-pr.js: null-coalescing guards for suggestion fields - src/frontmatter.js: fix closing --- to match standalone delimiter only - .github/workflows/gitmind-review.yml: use printf for untrusted input - test/epoch.test.js: update for 12-char SHA prefix, remove redundant assertion - test/frontmatter.test.js: use static import for chmod
Owner
Author
|
@coderabbitai review please |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bin/git-mind.js (1)
242-250:⚠️ Potential issue | 🟡 Minor
--indexwith non-numeric input passesNaNthrough, causing a runtime error.
parseInt('abc', 10)returnsNaN, which is truthy (NaN !== undefined), bypasses the bounds check inreview()(sinceNaN < 0andNaN >= nare bothfalse), and thenpending[NaN]isundefined, which will throw insideacceptSuggestion.Proposed fix
- index: reviewFlags.index ? parseInt(reviewFlags.index, 10) : undefined, + index: reviewFlags.index ? (Number.isNaN(parseInt(reviewFlags.index, 10)) ? undefined : parseInt(reviewFlags.index, 10)) : undefined,Or more readably, validate before dispatch:
+ const rawIndex = reviewFlags.index ? parseInt(reviewFlags.index, 10) : undefined; + if (rawIndex !== undefined && Number.isNaN(rawIndex)) { + console.error('--index must be a number'); + process.exitCode = 1; + break; + } await review(cwd, { batch: reviewFlags.batch, - index: reviewFlags.index ? parseInt(reviewFlags.index, 10) : undefined, + index: rawIndex, json: reviewFlags.json ?? false, });
🤖 Fix all issues with AI agents
In @.github/workflows/gitmind-review.yml:
- Around line 7-10: The workflow currently sets permissions: contents: read but
the CLI commands that run (accept, reject, accept-all) mutate the graph via git
notes and those changes are never pushed; change the workflow's permissions to
set contents: write and add a step to push commits (e.g., run git push origin
HEAD) after the CLI invocation that performs graph mutations so the notes/graph
updates persist; alternatively, if you only intend to reply with comments and
not persist mutations, remove the accept/reject/accept-all CLI invocations to
avoid unnecessary work.
In `@bin/git-mind.js`:
- Around line 187-197: The export subcommand currently reads args[1] directly so
a file placed after flags is ignored; update the logic that computes exportFile
(in the 'export' case handling, involving args, parseFlags, exportFlags, and
exportCmd) to locate the first positional argument that is not a flag or a flag
value (same approach used by the import command): iterate args.slice(1) skipping
tokens that start with '--' and also skip the immediate next token when a flag
expects a value, and use the first remaining token as exportFile so "git mind
export --format json graph.yaml" correctly treats graph.yaml as the file.
In `@src/cli/commands.js`:
- Around line 505-527: The error message when handling opts.index uses
pending.length directly, producing a nonsensical range when
getPendingSuggestions(graph) returns an empty array; update the index-handling
block to first check if pending.length === 0 and return a clear error (e.g., "No
pending suggestions") via the same error(...) + process.exitCode flow instead of
printing "Index X out of range (1-0)"; keep the rest of the logic (computing
idx, calling acceptSuggestion/rejectSuggestion, and formatting output with
formatDecisionSummary or JSON) unchanged so only the empty-list early-return is
added.
In `@src/epoch.js`:
- Around line 1-7: The module docstring incorrectly states epoch nodes use an
8-character SHA prefix; update the comment to match the actual implementation
(12-char prefix) used by recordEpoch and lookupEpoch by changing `epoch:<sha8>`
to `epoch:<sha12>` (or otherwise mentioning a 12-character commit prefix) so the
docs and the code are consistent.
- Around line 62-77: The lookupEpoch function currently builds nodeId with
commitSha.slice(0,12) which fails when callers pass abbreviated SHAs shorter
than 12 chars; update lookupEpoch (and mirror the change in recordEpoch) to
accept full or abbreviated SHAs by: compute shaPrefix = commitSha.slice(0,
Math.min(12, commitSha.length)); if commitSha.length < 12, instead of relying on
graph.hasNode(nodeId) do a prefix search for a node whose id startsWith
`epoch:${shaPrefix}` (e.g., iterate available node IDs via the graph API your
project exposes) and use that matching nodeId; otherwise keep the existing exact
lookup via graph.hasNode and graph.getNodeProps. Ensure you reference
lookupEpoch and recordEpoch and replace the existing commitSha.slice(0,12) usage
accordingly.
In `@src/format-pr.js`:
- Around line 29-36: The loop building table rows passes s.type directly to
escapeCell which can be undefined and cause a crash; update the call inside the
suggestions loop (where s is defined) to ensure a safe string is passed to
escapeCell (e.g., use a null-coalescing or string coercion for s.type before
calling escapeCell) so lines.push uses escapeCell(s.type ?? '') (or equivalent)
to guard against undefined.
In `@test/frontmatter.test.js`:
- Around line 154-167: The permission-based test using chmod(0o000) can silently
fail when run as root (e.g., in CI); modify the test case (the "throws on
permission errors (not ENOENT)" spec in test/frontmatter.test.js) to detect root
and skip the test: at the start of the test check process.getuid?.() === 0 and
call test.skip or return early so the assertions around
findMarkdownFiles(tempDir, 'restricted/**/*.md') are not executed under root;
keep the existing cleanup (chmod back to 0o755) but ensure it still runs when
skipping is used.
Comment on lines
+7
to
+10
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| issues: write |
Contributor
There was a problem hiding this comment.
contents: read may be insufficient if graph mutations need to persist.
The accept/reject/accept-all commands write to the graph (via git notes), but the workflow never pushes those changes back to the remote. This means the operations are effectively lost after the workflow run completes.
If graph mutations should persist, you need:
contents: writepermission- A
git pushstep after the CLI command
If the workflow is only meant to acknowledge the command via a comment reply (and the actual mutation happens elsewhere), this is fine as-is but the CLI invocations are unnecessary overhead.
🤖 Prompt for AI Agents
In @.github/workflows/gitmind-review.yml around lines 7 - 10, The workflow
currently sets permissions: contents: read but the CLI commands that run
(accept, reject, accept-all) mutate the graph via git notes and those changes
are never pushed; change the workflow's permissions to set contents: write and
add a step to push commits (e.g., run git push origin HEAD) after the CLI
invocation that performs graph mutations so the notes/graph updates persist;
alternatively, if you only intend to reply with comments and not persist
mutations, remove the accept/reject/accept-all CLI invocations to avoid
unnecessary work.
Comment on lines
+187
to
+197
| case 'export': { | ||
| const exportFlags = parseFlags(args.slice(1)); | ||
| const exportFile = args[1] && !args[1].startsWith('--') ? args[1] : undefined; | ||
| await exportCmd(cwd, { | ||
| file: exportFile, | ||
| format: exportFlags.format, | ||
| prefix: exportFlags.prefix, | ||
| json: exportFlags.json ?? false, | ||
| }); | ||
| break; | ||
| } |
Contributor
There was a problem hiding this comment.
export file argument must precede flags; otherwise it silently falls back to stdout.
args[1] is checked directly, so git mind export --format json graph.yaml treats the path as missing and writes to stdout instead of the file. This is consistent with the positional-first convention in this CLI, but the usage text export [file] doesn't make this ordering requirement obvious.
Option A: find positional arg like `import` does
- const exportFile = args[1] && !args[1].startsWith('--') ? args[1] : undefined;
+ const exportFile = args.slice(1).find(a => !a.startsWith('--'));This still has the fragility of matching flag values, but --format and --prefix values (yaml, json, prefix strings) are unlikely to be valid file paths and would fail at write time. A more robust approach would skip consumed flag values (same pattern as the import case).
🤖 Prompt for AI Agents
In `@bin/git-mind.js` around lines 187 - 197, The export subcommand currently
reads args[1] directly so a file placed after flags is ignored; update the logic
that computes exportFile (in the 'export' case handling, involving args,
parseFlags, exportFlags, and exportCmd) to locate the first positional argument
that is not a flag or a flag value (same approach used by the import command):
iterate args.slice(1) skipping tokens that start with '--' and also skip the
immediate next token when a flag expects a value, and use the first remaining
token as exportFile so "git mind export --format json graph.yaml" correctly
treats graph.yaml as the file.
Comment on lines
+62
to
+77
| export async function lookupEpoch(graph, commitSha) { | ||
| const shaPrefix = commitSha.slice(0, 12); | ||
| const nodeId = `epoch:${shaPrefix}`; | ||
|
|
||
| const exists = await graph.hasNode(nodeId); | ||
| if (!exists) return null; | ||
|
|
||
| const propsMap = await graph.getNodeProps(nodeId); | ||
| if (!propsMap) return null; | ||
|
|
||
| return { | ||
| tick: propsMap.get('tick'), | ||
| fullSha: propsMap.get('fullSha'), | ||
| recordedAt: propsMap.get('recordedAt'), | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
Abbreviated SHAs shorter than 12 characters will fail to match epoch nodes.
Both recordEpoch and lookupEpoch use commitSha.slice(0, 12) to build the node ID. If a caller passes an abbreviated SHA with fewer than 12 characters to lookupEpoch, it won't match the node created by recordEpoch with a full SHA — despite the JSDoc (line 59) documenting "Full or abbreviated commit SHA."
Current callers all pass full SHAs from git commands, so this isn't a runtime bug today, but the doc is misleading.
Suggested doc fix
- * `@param` {string} commitSha - Full or abbreviated commit SHA
+ * `@param` {string} commitSha - Full commit SHA (must be at least 12 characters)Apply the same to recordEpoch at line 34.
🤖 Prompt for AI Agents
In `@src/epoch.js` around lines 62 - 77, The lookupEpoch function currently builds
nodeId with commitSha.slice(0,12) which fails when callers pass abbreviated SHAs
shorter than 12 chars; update lookupEpoch (and mirror the change in recordEpoch)
to accept full or abbreviated SHAs by: compute shaPrefix = commitSha.slice(0,
Math.min(12, commitSha.length)); if commitSha.length < 12, instead of relying on
graph.hasNode(nodeId) do a prefix search for a node whose id startsWith
`epoch:${shaPrefix}` (e.g., iterate available node IDs via the graph API your
project exposes) and use that matching nodeId; otherwise keep the existing exact
lookup via graph.hasNode and graph.getNodeProps. Ensure you reference
lookupEpoch and recordEpoch and replace the existing commitSha.slice(0,12) usage
accordingly.
- epoch.js: update module doc sha8 → sha12, fix JSDoc for min SHA length - format-pr.js: null-coalescing guard on s.type - frontmatter.test.js: skip chmod test when running as root - commands.js: clear error for empty pending list in reviewCmd - bin/git-mind.js: export positional arg scan (same as import fix) - remote.js: reject multi-colon local IDs in qualifyNodeId - CHANGELOG: document all round-4 and round-5 fixes
Owner
Author
|
@coderabbitai review please |
Contributor
|
🧠 Learnings used✅ Actions performedReview triggered.
|
This was referenced Feb 14, 2026
This was referenced Feb 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NEXUS is the final milestone for git-mind v2, taking the graph from a local CLI tool to an integrated platform: GitHub Actions, markdown frontmatter import, multi-repo federation, and round-trip export.
git mind export— Serialize the graph to YAML/JSON in v1 import-compatible format (feat: git mind export command (NEX-006) #195)git mind import --from-markdown— Import nodes and edges from markdown frontmatter (feat: import from markdown frontmatter (NEX-003) #196)repo:owner/name:prefix:identifiersyntax for cross-repo edges (feat: cross-repo edge protocol (NEX-004) #197)git mind merge— Merge another repo's graph with cross-repo qualification (feat: multi-repo graph merge (NEX-005) #198)/gitmind accept|reject|accept-allslash commands in PR comments (feat: PR review commands and display (NEX-002) #200)Stats
Test plan
npx vitest run— 283 tests passinggit mind export --format yamlproduces valid importable YAMLgit mind import --from-markdown "docs/**/*.md"creates nodes from frontmattergit mind link file:a.js repo:other/repo:spec:auth --type implementscreates cross-repo edgegit mind merge --from /path/to/other/repoqualifies and merges nodesCloses #195, closes #196, closes #197, closes #198, closes #199, closes #200
Summary by CodeRabbit