Menu

#148 feat(ai,git): unify AI-ignore on git's ignore engine and honor `!` lines

closed
nobody
2026-08-07
2026-08-06
Anonymous
No

Originally created by: theBGuy
Originally owned by: theBGuy

AI-ignore lists are documented as gitignore-style, but the diff commands never used git's ignore engine — they ran a hand-written translation of each pattern into :(exclude,glob) pathspec terms, a second matcher that could drift from the real one on a privacy boundary (a pattern that fails to match is a file that reaches a third-party model). This deletes the translation layer so every AI-ignore verdict comes from git check-ignore, which in turn makes ! un-ignore lines expressible with git's own last-match-wins semantics and makes repo-then-global concatenation a security invariant rather than a preference.

Matching engine

  • Rewrites src-tauri/src/git/ai_ignore.rs around a single engine: the AiIgnorePathspecs struct, pathspecs_for/pathspecs_for_repo and the pattern-rewriting helpers are gone, and every verdict now goes through filter_ignored (check-ignore --no-index --stdin in a neutral repo, so paths need not exist in the index or working tree).
  • Adds filtered_diff, a two-pass flow for filtering a whole diff: name the changed files with --numstat -z, ask the engine which names are ignored, then re-run the content diff with one :(exclude,literal)<name> term per hidden name — exact, because it excludes a name git itself printed. Names that spelling cannot carry (holding a \, or not valid UTF-8) fall back to widened_glob_for_name, which over-hides rather than leaks.
  • Reworks actionable_lines to keep ! lines and report whether the list has any positive line, plus a has_actionable_lines short-circuit — a list of only negations can never hide anything, so git is not spawned for it. Order is now load-bearing and preserved.

Diff commands

  • git_staged_diff in src-tauri/src/git/diff.rs delegates to filtered_diff with recheck: true, since the index and working tree can change between the name pass and the content pass.
  • Adds DiffStatRow and parse_numstat_z_rows in src-tauri/src/git/diff.rs, retaining both sides of a rename; parse_numstat_z is now derived from it. A match on either the old or new name hides the pair, so excluding one side can no longer strand the other side's A/D row in the diff.
  • git_branch_diff in src-tauri/src/git/compare.rs delegates with recheck: false after the new pinned_range helper resolves both refs to SHAs in one rev-parse^{commit} peels annotated tags and turns multi-line rev expansions (HEAD^!, HEAD^@) into an error, and the results are hex-validated — so both passes read the same immutable trees.

Precedence invariant

  • Documents repo-first/global-last as a security invariant in ai_ignore_patterns (src-tauri/src/mcp_server/generate.rs) and src/lib/ai/ignore.ts: .gitdesktop/aiignore is committed content anyone with push access can write, so global patterns must be applied last or a committed ! could re-expose a file the user excluded globally.

Documentation and copy

  • Replaces the "! re-include lines aren't supported" claim everywhere it lived — README.md, the AI-ignore guide section in src/features/help/content.ts, and the Excluded files copy in src/features/settings/InstructionsSection.tsx — with the new behavior plus the global-last guarantee.
  • Adds changelog fragments changelog.d/added-ai-unignore.md (! support and precedence) and changelog.d/changed-ai-ignore-single-engine.md (diffs filtered through git's engine, renames hidden on either name).
  • Refreshes now-stale comments that referenced the pathspec route in src/lib/git/api.ts, src/lib/git/conflict.ts and src/lib/git/glob.ts — including dropping the Windows pathspec-backslash caveat, since check-ignore reads the trailing-space escape natively on every platform.

Related

Tickets: #151

Discussion

  • Anonymous

    Anonymous - 2026-08-06
     
  • Anonymous

    Anonymous - 2026-08-06

    Originally posted by: theBGuy

    Context for reviewers — deliberate calls with their evidence, so rounds don't re-derive them. Numbered for citation.

    1. What/why: the PR body covers mechanics. In one line: AI-ignore verdicts now come from git check-ignore (one engine, nothing to drift), diffs are filtered by excluding the concrete names it hides, and ! un-ignore lines work with git's own semantics.

    Deliberate calls

    1. The exclude-direction design is forced, not chosen: git diff does not support --pathspec-from-file (probed live: usage error, exit 129), so a positive allowed-list shape is impossible.
    2. :(exclude,literal) composes with the positive . and is exact-path (probed — brackets stay literal). The term names a path git itself printed; there is no escaping/anchoring translation left to drift.
    3. Names containing \: Windows git normalizes the backslash to a separator even under literal magic (probed — such a term excludes nothing there), so those names use a widened glob with ? in the backslash's position (probed to exclude on Windows; Unix glob ? matches a literal \). Over-hides at most a sibling name, never leaks.
    4. Non-UTF-8 names are hidden unconditionally when patterns are active: a verdict computed on a lossy U+FFFD string can't be trusted in the leak direction. They're counted in excludedFiles.
    5. Renames fail closed: the numstat pass keeps both sides, and either side matching hides the whole row (probed: excluding only one side strands an A/D row; excluding both removes it). An undetected rename (below similarity threshold) is an add+delete pair and only the matching side hides — identical to gitignore semantics.
    6. TERM_BUDGET (16,000 argv bytes): Windows caps a command line at 32,767 UTF-16 units, so per-name terms need a ceiling (~550 terms kills the spawn). Over budget the diff is withheld whole — over-hiding, honestly counted. A positive-pathspec inversion for that case is deferred on record.
    7. The staged-path recheck (re-read names after the content pass, retry ≤3) runs whenever patterns are actionable including when nothing was hidden — a file appearing mid-flow that matches a pattern is exactly the race it closes. The all-hidden and over-budget branches skip it legitimately: no content diff ran, and empty output can only over-hide.
    8. The branch path pins refs via one rev-parse <ref>^{commit}: peels annotated tags; multi-line rev expansions (HEAD^!, HEAD^@) die as a git error (probed: exit 128 fatal) before the two-SHA parse. validate_ref still screens the raw inputs first.
    9. The gitignore parent-directory rule is inherited deliberately and pinned by a PARITY row (build/ + !build/x.txt hides identically to bare build/) — it is git's own documented limitation, and "fixing" it would recreate the engine divergence this PR deletes. Every user-facing ! mention now carries the vendor/*-not-vendor/ guidance for that case.
    10. Precedence is a directional security invariant, pinned at both assembly sites: repo lines first, global lines last, so a committed .gitdesktop/aiignore line can never re-expose a file the user's global patterns exclude. It is deliberately not stated as "global always wins" — a global ! under a repo-excluded directory is still subject to item 10.
    11. Negation-only lists take the unfiltered fast path on purpose (a ! line can never cause hiding) — no check-ignore spawn, no ref pin.
    12. ! classification is escape-aware and pinned: \!name is a positive pattern (PARITY covers both spellings). Load-bearing because this predicate decides whether filtering runs at all.
    13. Exclude from AI's dedup boundary is now "already effective", not "already present": a pattern sitting before a later hand-written ! is re-appended at the end, re-asserting it under last-match-wins — which is the user's intent when clicking Exclude. The old set-membership dedup could report a file protected while it still reached the model.
    14. PARITY is 34 measured rows (git 2.51.1) driving the real git_staged_diff, asserting both the returned file list and the +++ b/ set parsed from the diff text — the text is the privacy boundary; the file list alone is Rust-derived and would stay green under a broken term constructor.
    15. Perf shape: with patterns active the flow is sequential (name pass → check-ignore → content → recheck) where the old code ran two spawns in parallel — inherent to two-pass verdicts. The no-pattern path is byte-identical to before, parallel spawns included.
    16. Deliberately untouched: site/ (it never claimed negation semantics — its "gitignore syntax" wording now simply covers more) and capabilities.ts (the existing AI-ignore capability line covers the enhancement). Shipped CHANGELOG entries saying ! was unsupported remain as history.
    17. Live-verified before opening: a dev build drove commit-message generation through three states — selective hide (one of two staged files filtered), all-hidden (disclosure toast, no model call), and ! re-include (the generated message described exactly the re-included file).

    Disclosures

    1. The recheck retry's 3-attempt exhaustion error path is implemented but not test-exercised (a deterministic mid-flow index mutation needs a fixture the suite doesn't have). Recorded follow-up.
    2. Two over-hide-only edges are recorded and declined for this PR: copy detection (diff.renames=copies) can make the file list and text disagree by one row, and unmerged paths yield no numstat row, so a conflicted-only tree filters to empty.
    3. The shared "[N hidden by the user's AI ignore rules]" prompt note merges pattern hits with unreadable-name drops — pre-existing, recorded backlog item; this PR makes fixing it easier since hidden names are now known individually.

    Posted by GitDesktop — automated agent comment, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-08-06

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated


    Replaces the pathspec translation of AI-ignore patterns with a two-pass git check-ignore flow (name pass → verdicts → content pass with :(exclude,literal) terms), honors ! un-ignore lines, and pins repo-then-global ordering as the precedence invariant. The design is sound and the 34-row measured PARITY table now drives the real commands rather than a translation, which is a genuine improvement in the privacy boundary's coverage. Nothing here blocks; two should-fixes below.

    Recorded decisions I'm not re-litigating: the exclude-direction forcing (#2), the widened \/lossy terms (#4, [#5]), rename fail-closed (#6), the parent-directory rule as a PARITY row (#10), negation-only fast path (#12), the sequential perf shape (#16), site/ + capabilities.ts left untouched (#17), and the untested recheck-exhaustion path (#19).

    Correctness

    • should-fixsrc-tauri/src/git/ai_ignore.rs:422-428 (filtered_diff, over-TERM_BUDGET branch): the withhold-whole decision is on record (#7), but the signal it emits is indistinguishable from "everything matched your patterns", and that drives a wrong user-facing message on every generation surface. Trace: over budget → FilteredDiff { text: "", files: vec![], excluded_files: total_rows }git_staged_diff/git_branch_difffiles.is_empty() && excluded_files > 0src/features/commit/useGenerateCommitMessage.ts:42-44 ("All staged changes match your AI ignore patterns — nothing to describe."), src-tauri/src/mcp_server/generate.rs:1167-1173 (same, plus "Stage changes outside those patterns first."), src/features/pulls/useGeneratePrDescription.ts:104-105, src/features/history/RewriteDialogs.tsx:59-60, src/lib/stores/reviews.ts:537-538, src/features/repository/useGenerateBranchName.ts:145-159. The concrete case is the one the new test builds: 400 ignored vendor/ files plus a.txt and keep.txt — the user does have changes outside the patterns, and the advice to stage some is actively wrong. Minimal fix: return Err(AppError::Command("too many AI-ignored files to filter this diff safely — narrow the range or the ignore patterns".into())) from that branch instead of an empty FilteredDiff; knock-ons to apply in the same change — reword the TERM_BUDGET doc (ai_ignore.rs:260-265, which currently says "the diff is withheld whole") and the filtered_diff doc, and rewrite a_term_list_over_budget_withholds_the_whole_diff (ai_ignore.rs:1486-1521) to assert the error rather than excluded_files == 402. If you'd rather keep degrading instead of erroring, the alternative is a withheld: bool on FilteredDiff and StagedDiff (mirrored in src/lib/git/types.ts:130), branched at all six message sites above — bigger, and it needs all six touched or the gap just moves.

    • should-fixsrc/lib/git/glob.ts:14-16 and :27-28 (globLiteralPath doc): the reworded claim "a literal backslash is inexpressible as a gitignore pattern" is false. \ is git's escape in a gitignore pattern — the PARITY row (&["src\\foo.ts"], &["srcfoo.ts"]) (ai_ignore.rs:679) relies on exactly that — so \\ names a literal backslash. Because the function leaves \ alone, on Unix a file named a\b.txt yields the pattern /a\b.txt, which git reads as /ab.txt: Exclude from AI (src/features/repository/ChangesPanel.tsx:528, ChangesContextMenu.tsx) reports the file excluded via the append count while the file still reaches the model — the fail-open direction this module treats as a blocker everywhere else, and the mirror of the name-side case this PR just fixed with widened_glob_for_name. Fix: make .replace(/\\/g, "\\\\") the FIRST replacement in globLiteralPath (before the [*? class wrap and, critically, before the trailing-space escape, or the escape's own backslash gets doubled). Knock-ons: delete both doc paragraphs' "inexpressible" claims, and delete the "The escape is defeated when the name ALREADY ends in a backslash (notes\)" paragraph entirely — doubling resolves it (notes\notes\\notes\\\, an odd run that both trimIgnorePattern and Rust's trim_ignore_pattern already keep).

    Readability

    • nitsrc-tauri/src/git/ai_ignore.rs:279 has_actionable_lines returns has_positive, so it disagrees with its own namesake: actionable_lines(&["!a"]).0 is non-empty while has_actionable_lines(&["!a"]) is false. Rename to has_positive_pattern (or can_hide_anything) and update the two call sites, compare.rs:148 and ai_ignore.rs:355, plus the three test assertions at ai_ignore.rs:489/499/506 and the doc sentence at :276-278.

    • nit — the "already present" dedup wording is now stale in three places the diff didn't reach: src/lib/git/api.ts:3052-3053 ("already-present ones are skipped"), src/features/repository/ChangesPanel.tsx:510 and :525-526 ("The Rust side de-dupes and skips lines already present"). Reword all three to "already effective" to match instructions.rs:80-97.

    • nitsrc-tauri/src/git/ai_ignore.rs:450: sorted_names spells crate::git::diff::DiffStatRow inline while line 36 already imports from that module; add DiffStatRow to that use.

    Tests

    • nita_term_list_over_budget_withholds_the_whole_diff (ai_ignore.rs:1486) only exercises ~400 terms of ~94 bytes, far past the 16,000 ceiling; nothing pins the just-under case still returning a filtered diff, so a later change to TERM_BUDGET or term spelling can move the cliff silently. Add a sibling row sized just under the budget asserting the survivors still come back.

    Posted by GitDesktop — AI output, verify before acting on it.

     

    Related

    Tickets: #5

  • Anonymous

    Anonymous - 2026-08-06

    Originally posted by: theBGuy

    🤖 GitDesktop AI security audit · opus · automated


    Findings

    Severity: Medium — Confidence: 8/10

    ai-ignore-bypass / sensitive-data-exposure — src-tauri/src/git/ai_ignore.rs::filtered_diff (the args.extend(["--", "."]) + :(exclude,literal){name} term construction), reached via git_staged_diff / git_branch_diff from src-tauri/src/mcp_server/generate.rs.

    Exploit scenario: The new two-pass design makes the exclusion terms root-relative concrete names (taken from git diff --numstat -z, which prints root-relative paths) while the positive pathspec is . and git resolves every pathspec element against the process cwd (run_git does cmd.current_dir(repo_path)). The function's own doc comment states the resulting precondition: "repo_path must be the repository ROOT … a subdirectory would void every term."

    That precondition is not enforced anywhere, and the MCP entry point can violate it: McpArgs::parse (src-tauri/src/mcp_server/mod.rs) takes --repo verbatim, falling back to std::env::current_dir(), and with_options stores it unnormalized — there is no rev-parse --show-toplevel on that path (unlike the GUI, where validate_repo normalizes to root). The MCP config the app itself writes uses --repo "${CLAUDE_PROJECT_DIR:-.}" (Claude) and --repo "." (Copilot) — i.e. the agent's project/working directory. Start the agent inside a package subdirectory of a repo (cd services/api && claude) and self.repo is …/services/api. Then, for generate_commit_message:

    • pass 1 (git diff --cached --numstat -z, no pathspec) names the whole repo's changed files root-relative, so services/api/.env gets a correct "ignored" verdict from check-ignore and is counted in excluded_files;
    • pass 2 runs git diff --cached --no-color -- . :(exclude,literal)services/api/.env with cwd services/api, where the term resolves as services/api/services/api/.env and matches nothing, while . still covers the whole subdirectory.

    Net: the excluded file's full diff is in filtered.text and goes into the prompt handed to the third-party model, while files/excluded_files report it hidden — a silent fail-open on what the module header itself calls the privacy boundary. The old pattern-derived :(exclude,glob)**/… terms were relative patterns and did not depend on cwd this way, so this is a regression for that binding, not a pre-existing gap. What I verified: the term construction, the cwd wiring, the unnormalized --repo, and the config template the app emits. What I inferred rather than measured: that users bind the server below the toplevel (a usage assumption) and git's prefixing of pathspec elements — though the author's own comment asserts that same mechanism.

    Remediation: Resolve the working-tree root once (git rev-parse --show-toplevel) and run both passes there — either inside filtered_diff before the passes, or by normalizing --repo at MCP startup so every tool shares the root. Failing closed (error out) when repo_path is not the toplevel would also be acceptable; what must not happen is silently emitting terms that cannot match while still reporting the files as excluded.


    Copilot's src/lib/git/glob.ts:16 point (literal \ not escaped in globLiteralPath) is not actionable here: this PR changes only the doc comment above that function, not the replace line, and the version of globLiteralPath currently in the tree already maps \? and aiExcludePatternLinesForPath emits a second /-separated line for such names.


    Posted by GitDesktop — AI output, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-08-06

    Originally posted by: theBGuy

    Round 1 dispositions — all four findings accepted; everything below lands in the next push. (Copilot's inline thread carries its own reply and is resolved.)

    AI review, should-fix 1 (over-budget aliases the all-hidden signal): fixed with your minimal arm. The branch now returns a distinct error — too many AI-ignored changed files to filter this diff safely — narrow the diff or the AI ignore patterns — instead of the all-hidden shape; the TERM_BUDGET and filtered_diff docs are reworded off "withheld whole"; the budget test asserts the error and gained a just-under-budget sibling so the cliff is pinned from both sides. Your six-surface trace was verified caller-by-caller — every surface toasts the error; none folds it into an empty-diff message.

    AI review, should-fix 2 + Copilot's inline (globLiteralPath backslash): fixed with both halves — your PARITY-row evidence settled the escape semantics, and a measured Windows arm (a name's \ normalizes to a separator during matching) adds a /-separated twin line for backslash paths. Details and probe outputs in the inline thread.

    AI review, nits: has_actionable_lineshas_positive_pattern; DiffStatRow joined the existing import; the "already present" → "already effective" rewording applied to the AI-ignore surfaces — deliberately not to the gitignore-path comments (ChangesPanel's ignore path, appendToGitignore), which describe append_to_gitignore's unchanged dedup, where "effective" would be false.

    Security audit (subdir binding voids the exclude terms): confirmed by an independent probe before fixing — including a second effect the finding didn't claim: the positive . also re-scopes the content diff to the subdirectory, silently dropping root-level changes from the text while the file list keeps them. Your vector then generalized into a class. The sweep found two more halves: git ls-files --others is cwd-scoped and prints cwd-relative names (untracked-name disclosure past anchored patterns, plus silent omission of outside-subdir untracked work), and read_repo_ai_ignore joins .gitdesktop/aiignore under the given path (a subdirectory binding silently drops the team's entire committed rule set while global patterns keep working — the quietest failure of the family). All three now resolve the working-tree toplevel through one shared worktree_toplevel helper, each with a regression test that reproduces its leak when the fix is reverted; read_repo_instructions had the same shape (quality, not privacy) and rides the same helper. Remediation follows your first option — resolved at the boundary rather than normalizing --repo wholesale, so agent-relative path arguments to other MCP tools keep their frame.


    Posted by GitDesktop — automated agent comment, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-08-06

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated


    Re-reviewed against the current diff. All four previous findings are fixed in code I can see, and the new toplevel-binding work (worktree_toplevel + its three call sites) is sound: I traced validate_repo (src-tauri/src/git/repo.rs:154-182) and it normalizes the GUI's repo path to the toplevel already, so the gap really was MCP-only, and the three .gitdesktop/name-reading seams in mcp_server/generate.rs are the complete set (grep finds no other read_repo_* or ls-files call bound to self.repo). Nothing blocking.

    Resolved since last review

    • Over-budget aliasing the all-hidden signalai_ignore.rs:436-446 now returns AppError::Command("too many AI-ignored changed files…"); the TERM_BUDGET doc (:260-267) and the filtered_diff doc (:353-355) are reworded off "withheld whole", and a_term_list_over_budget_errors_instead_of_leaking_or_lying asserts the error. The Rust line-continuation in that message keeps its space (the \ sits after safely), so the em-dash isn't glued to the previous word.
    • globLiteralPath and the literal backslash (also Copilot's inline finding) — src/lib/git/glob.ts:41 doubles \ first, before the class wrap and the trailing-space escape. I verified the new doc's odd-run claim against both mirrors: trimIgnorePattern (glob.ts:83-93) and fsops::trim_ignore_pattern (src-tauri/src/fsops.rs:100-114) both keep the space for notes\\\. The "inexpressible" and "escape is defeated" paragraphs are gone, and the new aiExcludePatternLinesForPath twin is wired at every AI-exclude call site (ChangesContextMenu.tsx:296/314/331, ChangesPanel.tsx:542) — ChangesMenuActions.aiExclude has exactly one implementer, so no stale single-string caller remains.
    • has_actionable_lineshas_positive_pattern — renamed with both call sites (compare.rs:148, ai_ignore.rs:368), the three test assertions and the doc sentence.
    • "already present" → "already effective" — applied on the AI-ignore surfaces (api.ts:3052-3054, ChangesPanel.tsx:75-82 and :533-534) and correctly not applied to ignoreSelected (ChangesPanel.tsx:517-518), where append_to_gitignore still dedups on exact presence (fsops.rs:117-120) — "effective" would have been false there.
    • DiffStatRow import (ai_ignore.rs:36) and the just-under-budget test row (150 terms ≈ 14.1 KB, asserting survivors + excluded_files == 150).

    Fix-collateral check: the globLiteralPath doubling also reaches the .gitignore writers (ChangesContextMenu.tsx:208/223/246/265, ChangesPanel.tsx:521/564, RepositoryFilesDialog.tsx:39), and that is correct gitignore syntax there; rule removal reads lines back from check-ignore -v, not from this helper, so nothing compares the escaped form against a raw path.

    Nits

    • nitworktree_toplevel is re-resolved per seam: ai_ignore.rs:384, generate.rs:1148, :1166, :1833. A branch recipe therefore spawns four extra git rev-parse --show-toplevel calls (commit/PR: two–three). Since GitDesktopMcp.repo is fixed for the process, a OnceCell<String> on the struct resolved once — or one resolution threaded through build_branch_recipe — would collapse them; entirely optional given the model call that follows.

    Recorded decisions I'm not re-litigating: over-budget reachability stays a recorded decision (#7's deferred positive-pathspec inversion) — worth restating only that, at the test's own ~94 bytes/term, the 16,000-byte ceiling now trips at roughly 170 hidden files, where the pre-PR pathspec translation was file-count-independent; also still on record: exclude-direction forcing (#2), widened \/lossy terms (#4, [#5]), rename fail-closed (#6), parent-directory PARITY row (#10), negation-only fast path (#12), sequential perf shape (#16), untested recheck exhaustion (#19), and the copy-detection/unmerged edges (#20). I checked the site copy the decision (#17) covers — site/src/pages/ai.astro:277-286 and site/src/data/capabilities.ts:475 make no negation claim, so leaving them untouched is consistent with what's there; README, help/content.ts and the two changelog.d/ fragments carry this change, and CHANGELOG.md's ## [Unreleased] is correctly untouched.

    Nothing further to raise.

    Verdict: no blocking issues — remaining items are non-blocking; merge when ready


    Posted by GitDesktop — AI output, verify before acting on it.

     

    Related

    Tickets: #5

  • Anonymous

    Anonymous - 2026-08-06

    Originally posted by: theBGuy

    Round 2 disposition: the worktree_toplevel re-resolution nit is accepted and held — a OnceCell on the MCP struct now caches the resolution for the three generate.rs seams (filtered_diff keeps its own internal resolve deliberately: it serves non-MCP callers and fires only when patterns are actionable). Held for the next push rather than spending one on it alone.

    Housekeeping: the red gate/build checks on this head were GitHub Actions infrastructure ("Failed to resolve action download info: Service Unavailable" during job setup — no code ran); both runs have been re-run.


    Posted by GitDesktop — automated agent comment, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-08-06

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated


    Delta since the last review is the OnceCell toplevel cache on GitDesktopMcp plus moving untracked_files onto the struct; both are sound — get_or_try_init stores only successes, the borrow of &self across the run_git await is fine, and all three generate.rs seams (ai_ignore_patterns:1148, repo_instructions:1164, untracked_files:1182) now share one resolution. Nothing blocking; one doc nit in the moved hunk.

    Resolved since last review

    • worktree_toplevel re-resolved per seammcp_server/mod.rs:104 adds toplevel: Arc<tokio::sync::OnceCell<String>>, initialized at :234 and read through Self::toplevel (:259-264); grep confirms the only remaining independent resolution is ai_ignore.rs:384 inside filtered_diff, which the field doc calls out as deliberate (non-MCP callers, pattern-gated) and which I verified matches the code. The three generate.rs call sites and the two relocated tests all go through it; pub(super) matches the file's existing idiom for shared items (ShaArg, CiId::as_string).

    Readability

    • nitsrc-tauri/src/mcp_server/generate.rs:1173, untracked_files doc: the C-quoting example lost both its bytes and its point in the move. It now reads (`"café.txt"`) — mojibake (the octal escapes \303\251 re-read as Latin-1); it is the only such sequence in src-tauri/src, and the same file spells the example correctly at :255 and :2806 as "caf\303\251.txt". It also contradicts its own sentence: the claim is that git C-quotes the name, so the illustration has to be the quoted-octal form, not the rendered name. Restore (`"caf\303\251.txt"`) on that line, matching the two sibling docs; nothing else in the moved block is affected (the em-dashes on :1171/:1179 are intact).

    Nothing else new in this round's hunks. Documentation coverage is unchanged from last round: README, src/features/help/content.ts and the two changelog.d/ fragments carry the change, CHANGELOG.md's ## [Unreleased] is correctly untouched, and the site/capabilities.ts omission remains the recorded decision (#17) I checked against site/src/pages/ai.astro:277-286 and capabilities.ts:475.

    Verdict: no blocking issues — remaining items are non-blocking; merge when ready


    Posted by GitDesktop — AI output, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-08-06

    Originally posted by: theBGuy

    Round 3 disposition: the mojibake'd C-quoting example in untracked_files' doc is fixed — restored to "caf\303\251.txt" matching the two sibling docs, exactly as suggested. One-line comment change, applied directly and verified compiling; it rides the next push.

    With two consecutive clean rounds ("merge when ready" twice), all review threads resolved, and every remaining item either fixed or deferred on the record, this PR converges once CI is green on the final head.


    Posted by GitDesktop — automated agent comment, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-08-06

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated


    Resolved since last review

    • Mojibake'd C-quoting example in untracked_files' doc (src-tauri/src/mcp_server/generate.rs:1173) — restored to (`"caf\303\251.txt"`), now byte-identical to the sibling docs at :255 and :2806; a grep for caf in that file shows no remaining rendered-Latin-1 form. The fix is a single doc line still attached to async fn untracked_files, with no import, call-site, or symbol-placement collateral, and it introduces no doc_lazy_continuation surface (the line opens with ( inside backticks).

    I also re-checked that the removed pathspec layer left no stale references anywhere in the tree (pathspecs_for, AiIgnorePathspecs, skipped_negations, rewrite_for_pathspec, bracket_end, posix_class_end — zero hits), and that every aiExclude call site now passes the array form (ChangesContextMenu.tsx:296, :314, :331; ChangesPanel.tsx:436, :991), with the .gitignore-path call sites deliberately left on globLiteralPath per the recorded decision. Documentation coverage is unchanged and complete for this change: README, src/features/help/content.ts, and the two changelog.d/ fragments carry it, CHANGELOG.md's ## [Unreleased] is correctly untouched, and the site/capabilities.ts omission remains recorded decision [#17].

    Nothing further to raise — the only change since the last review is the one-line doc fix, and it lands exactly as suggested.

    Verdict: no blocking issues — remaining items are non-blocking; merge when ready


    Posted by GitDesktop — AI output, verify before acting on it.

     

    Related

    Tickets: #17

  • Anonymous

    Anonymous - 2026-08-07

    Ticket changed by: theBGuy

    • status: open --> closed
     

Log in to post a comment.