GitDesktop / Tickets / #135 feat(releases,ai,git,mcp): sync updater notes when editing a release

Anonymous - 2026-07-31

assigned_to: theBGuy
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-31

Originally posted by: cloudflare-workers-and-pages[bot]

Deploying gitdesktop with Cloudflare Pages

Latest commit: 674ae71

Status: ✅ Deploy successful!

Preview URL: https://91a6c0d2.gitdesktop.pages.dev

Branch Preview URL: https://feat-release-updater-sync-ai.gitdesktop.pages.dev

View logs

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-31

Originally posted by: theBGuy

Context for reviewers — deliberate calls and disclosures, each with its evidence. Items are numbered for later reference.

Scope. Two commits: the feature batch (590b38a) and a pre-open hardening pass (2b54118) that folded the findings of an internal adversarial review before this PR left draft. The updater-sync success path was exercised live against a real GitHub release (notes replaced in both the release body and the latest.json asset; version, pub_date, platform url/signature fields byte-preserved; asset name survived the re-upload), and the fill layout was exercised live at a 620px window (fields scroll; no overlap).

gh release upload --clobber is delete-then-upload, not atomic — gh 2.94.0's own help: "existing assets are deleted before new assets are uploaded. If the upload fails, the original assets will be lost." The code comments and the failure toast state this real contract. On an upload failure the already-patched manifest is saved to a recovery path (basename kept latest.json — the asset takes its name from the file) and that path rides the error into the toast's Details, so the guidance (re-upload via Assets → Upload) is achievable. A truly gapless swap would need an upload-under-temp-name + rename dance via gh api; deliberately not built for a rare transient-failure window — if reviewers want it, that is the correct execution.

Backslash in an AI-ignore pattern is always a gitignore escape, never a Windows path separator. src\foo.ts matches the literal srcfoo.ts — uniformly, on both matching engines, on every platform (the docs promise ".gitignore syntax"). Before this PR, that shape accidentally hid src/foo.ts on Windows on one engine only (pathspec argv normalization) while every other surface leaked it. The pin is tested: 22-row cross-engine parity table, emitted-term assertions, and a widened == 0 guard so exact re-encodes can't drift into the widened count. Measured on git 2.51.1.windows.1.

The five class-special escapes (\] \^ \! \- \\) and a dangling backslash widen to ? — fail-closed by design. Over-exclusion is the only permitted failure direction on this privacy boundary; a superset-direction test pins pathspec-hidden ⊇ gitignore-hidden for these shapes. A trailing \/ was measured to diverge between the engines (gitignore's dangling-escape residue matches nothing); the divergence is fail-closed, documented at the site, and deliberately not counted in widened, whose meaning stays exactly "the ? fallbacks".

Manifest sync is GitHub-only — not because GitLab lacks release assets (it has them; our upload/delete arms use them), but because the updater feed this app ships is a latest.json on a GitHub release. The checkbox is additionally gated on the release actually carrying that asset. README, the in-app guide, and the changelog all carry the GitHub qualifier.

Re-uploading the manifest re-serializes it with alphabetized keys (serde_json without preserve_order). Values are byte-preserved; the manifest is not signed over its own bytes (platform signatures cover the bundles), so this is cosmetic and disclosed in-code — the re-uploaded asset is not byte-diffable against CI's original.

The fill layout deliberately omits min-h-0 on the fill wrappers (comment at the prop): min-height: auto floors each box at its content minimum, which is what makes short windows scroll instead of letting the textarea paint over the toggles below — verified live at 620px. resize-y is deliberately removed in fill mode (a drag handle fights flex sizing); the changelog fragment discloses the handle's removal.

git_stash_paths now returns whether a stash was created; the GUI deliberately does not consume it yet. GUI selections come from git status, so a zero-match stash is near-impossible there; the MCP surface (where an agent can pass arbitrary pathspecs) consumes it and answers "nothing was stashed". The TS binding is typed boolean for honesty; wiring a GUI toast to it is a recorded follow-up.

Tests are hermetic against the developer's real settings store via a GD_SETTINGS_DIR/cfg!(test) seam in app_store.rs (mirrors the existing GD_OPLOG_DIR seam; the test arm returns no store at all because this module only reads). The env override ships in release binaries exactly as its oplog sibling does; #[cfg(test)]-gating it is a two-line change if preferred. The adversarial-store negative control and a panic-restore RAII guard are lock-serialized; the full suite ran 4× consecutively at 908/0 to clear an observed 1-in-4 interleaving flake during development.

Non-UTF-8 untracked filenames fail closed. ls-files -z gives raw bytes, but the lossy UTF-8 decode can still mint U+FFFD; any name carrying it is counted as hidden rather than sent mangled-and-unmatchable. A byte-level OsString rework across the ignore engine would be the complete fix and is a named possible follow-up.

Disclosure: MCP update_release still edits the release body only — an agent editing notes over MCP reproduces the manifest divergence this PR fixes in the GUI. Deliberately deferred (same fix pattern, separate change); tracked in the project backlog.

Disclosure: the failure path is code-reviewed, not live-forced. The recovery-file write and its toast were verified by reading and unit tests; nobody deliberately severed the network mid-upload. The success path is the live-verified one (item 1).

Disclosure: one pre-existing comment in glob.ts states a platform-neutral claim measured only on Windows (the [[]-over-\[ rationale). Qualifying it needs a Unix probe; the conclusion it justifies is measurement-independent (parity-proven on both engines), so it was left rather than guessed at.

Posted by GitDesktop — automated agent comment, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-31

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

Adds GitHub updater-manifest (latest.json) note-syncing to the release editor, re-encodes gitignore backslash escapes into pathspec-safe classes, applies AI-ignore to untracked names in branch naming (app + MCP), reports whether stash_push actually stashed, and fixes exact-match unignore. The shape is sound and the docs (README, capabilities, help guide, six changelog fragments) are all carried in the same change; nothing here is merge-blocking, but two fail-open holes on the AI-ignore privacy boundary and one process-env test race are worth fixing before merge.

Correctness

should-fix — src-tauri/src/git/ai_ignore.rs, escapes_to_classes: the rewriter walks characters with no bracket awareness and no non-ASCII arm, so two shapes come out matching less than gitignore does — the fail-OPEN direction the module forbids.

Escape inside a bracket expression: pattern a[b\-c]d → - hits the class-special arm and the output is a[b?c]d, a class of b, ?, c. gitignore hides a-d; the pathspec term no longer does, so a-d rides into the staged/branch diff sent to the model. On Unix this is a regression — wildmatch honors \ inside a bracket, so the raw term matched before this change. weird[\]].txt → weird[?].txt is the same bug.

Escaped non-ASCII: docs/\日本語.md → docs/[日]本語.md. wildmatch is byte-based and a bracket consumes exactly one byte, so the class matches a lone 0xE6 and the term matches nothing, while gitignore matches docs/日本語.md (your own FIXTURE row).
Fix inside the loop: (a) add a Some(e) if !e.is_ascii() => out.push(e) arm before the class arm — a non-ASCII char is never a glob metacharacter, so the bare char is exact; (b) handle an unescaped [ by scanning to its closing ] (allowing a leading !/^ and a first-member ]) and copying the expression verbatim when its body holds no backslash, else emitting a single ? and widened += 1 — ? is a strict superset of any bracket under ,glob (a bracket never matches / with WM_PATHNAME), so that stays fail-closed. Knock-ons: the escapes_to_classes doc block currently states a trailing \/ is the only divergence — reword it to cover the widened bracket; AiIgnorePathspecs::widened's doc ("the ? fallbacks") still reads true. Add a PARITY row for a[b\-c]d (fixture a-d/abd) and a superset row in widened_escapes_over_exclude_and_never_under_exclude.

should-fix — src/lib/ai/ignore.ts filterPathsByAiIgnore / src/features/repository/useGenerateBranchName.ts: the GUI half of the untracked-names fix doesn't fail closed on names that lost bytes, so the two surfaces the changelog fragment claims ("in the app and over MCP") disagree. git_status decodes with parse_status_v2(&out.stdout_lossy()) (src-tauri/src/git/status.rs:36), so on Linux/macOS a filename that isn't valid UTF-8 arrives as FileEntry.path with U+FFFD substituted, flows through opts.entries.filter(e => e.unstaged === "untracked") into untrackedPaths, and reaches the prompt as <name> (new file) — and no rule the user could write can match it, which is exactly why filter_untracked_by_ai_ignore (mcp_server/generate.rs) drops such names unconditionally. Fix in filterPathsByAiIgnore, ahead of the early return so it also applies with no patterns configured (the Rust twin's untracked_names_that_lost_bytes_are_hidden passes an empty exclude):
ts const safe = paths.filter((p) => !p.includes("\uFFFD")); const lost = paths.length - safe.length; if (safe.length === 0 || exclude.length === 0) return { paths: safe, excluded: lost }; const hidden = new Set(await gitFilterAiIgnored(repoPath, safe, exclude)); const kept = safe.filter((p) => !hidden.has(p)); return { paths: kept, excluded: paths.length - kept.length };
and reword the JSDoc's "An empty paths or exclude returns the input untouched, before any IPC" — it no longer returns the input untouched.

UI

should-fix — src/features/tags/TagDetailView.tsx, saveLatched: the latch doesn't mirror the decision it's meant to guard. syncManifest (in onSubmit) is canSyncUpdater && editSyncUpdater && !!editNotes.trim(), but saveLatched omits the notes test, so with the box checked and the notes cleared a plain title-only save is un-dismissible with Cancel disabled — contradicting the comment "A plain edit (box off) stays dismissible exactly as it always was". Conversely the latch reads live query data: canSyncUpdater derives from rel.assets, and useRepoMutation's onSettled invalidates the whole repo subtree when phase 1 lands, so a gh release view refetch that resolves after --clobber's delete step flips canSyncUpdater false and drops the latch mid-upload. Capture the decision instead: add const [syncArmed, setSyncArmed] = useState(false), set it from the same expression in onSubmit (before editRelease.mutate), clear it in both the success and error branches of syncUpdaterNotes.mutate and in the !syncManifest early return, and use syncArmed && savePending for both saveLatched and the Cancel button's disabled; also reset it alongside setEditSyncUpdater(true) when the Edit button opens the dialog.

Nits

src/features/tags/TagDetailView.tsx:502 — the sync <label> keeps cursor-pointer and full opacity while savePending disables its Checkbox; the "Latest" label 25 lines above conditionally applies cursor-not-allowed opacity-60. Match that idiom.

src-tauri/src/github/release.rs, save_updater_recovery_copy — the recovery dir name is derived from a wall-clock nanosecond in the shared std::env::temp_dir(), and create_dir_all + fs::copy happily follow a pre-created dir/symlink; ai_ignore.rs's NEUTRAL_REPO doc argues this exact point and uses tempfile for exclusive creation with owner-only permissions. Use tempfile::Builder::new().prefix("gd-updater-recovery-").tempdir() and persist it (into_path()/keep()), then join UPDATER_MANIFEST.

src/components/markdown-editor.tsx:482 — in fill mode the Preview box floors at min-h-24 and ignores textareaClassName, so CreateReleaseDialog's min-h-32 textarea and its Preview have different minimums; either apply the same override to both or note it in the fill doc comment (which currently only documents the textarea's floor).

Tests

should-fix — src-tauri/src/mcp_server/generate.rs, SettingsDirOverride: it mutates process-global env inside a parallel test binary, and SETTINGS_STORE_LOCK only serializes the two tests that opt in — every other test in the binary is concurrently reading the environment (Command spawns for git, dirs::data_dir()), which is precisely the race the sibling seam refuses: oplog.rs:509 says "Branch on the var rather than mutating process env (which would race parallel tests)". The disclosed 1-in-4 flake is the symptom. Fix without losing the end-to-end assertion: add a #[cfg(test)] static TEST_STORE_DIR: Mutex<Option<PathBuf>> in app_store.rs consulted by store_path() ahead of GD_SETTINGS_DIR, have the RAII guard set/restore that (Drop unchanged, panic-restore test still valid), and keep SETTINGS_STORE_LOCK serializing the guard's users; then SettingsDirOverride::set takes no env at all and the "must set the override through the guard" comment can drop its env caveat.

No test covers created_entry's message match against a localized git — worth one line in the comment that LC_ALL=C is what makes the string stable (already stated) — no action needed; the three new stash tests cover fast path, slow path and the positive case.

Recorded decisions I'm not re-raising: the --clobber delete-then-upload window with its on-disk recovery copy (note 2), the fail-closed ? widening and the \/ divergence (notes 3–4), the alphabetized re-serialization (note 6), the omitted min-h-0 on fill wrappers (note 7), the unconsumed git_stash_paths boolean in the GUI (note 8), the GD_SETTINGS_DIR override shipping in release binaries (note 9), MCP update_release still body-only (note 11), and the pre-existing Windows-only measurement in glob.ts (note 13).

Posted by GitDesktop — AI output, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-31

Originally posted by: theBGuy

🤖 GitDesktop AI security audit · opus · automated

No genuinely exploitable vulnerabilities are introduced by these changes — the new gh invocations pass argv arrays (no shell) with validate_tag blocking a leading - on the only positional user value, the untracked-name filter and the escape re-encoding both fail closed on error/undecodable input, the release-notes preview still renders through the DOMPurify-sanitizing Markdown component, and the GD_SETTINGS_DIR seam mirrors the existing GD_OPLOG_DIR pattern and is fed only by the invoking user's own environment.

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

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-31

Originally posted by: theBGuy

Round 1 dispositions (context items referenced by number; all fixes land in the next push):

AI review — all four should-fixes accepted and fixed, with two upgrades found while fixing:

escapes_to_classes bracket-blindness + non-ASCII (fail-open ×2): fixed and measured. Escaped non-ASCII now passes through bare; unescaped bracket expressions are scanned whole — copied verbatim when clean (measured identical on both engines, including [!…], [^…], []…], and unterminated [, which matches nothing on either engine), widened to a single ? when a backslash rides inside (measured strict superset). The fn was renamed rewrite_for_pathspec — it no longer only rewrites escapes, so the old name had become misleading. Two upgrades beyond the finding, both measured: the bracket scan steps over \] so a[b\]c]d can't split the class into a non-superset, and POSIX character classes are consumed (x[[:digit:]\-]y previously leaked x-y — reproduced live, now superset-widened). One deliberate non-extension: wildmatch has no [=…=]/[.….] collating forms (measured — they match nothing), so the scanner treats them as ordinary members rather than "fixing" a divergence that doesn't exist. Parity table now 27 rows.

GUI-side U+FFFD guard: fixed, mirroring the MCP side, ahead of the IPC short-circuit, with the JSDoc reworded. The character is referenced via a named constant rather than a literal (an invisible literal in source is a silent fail-open risk if any tool re-encodes it). And an honesty upgrade the fix surfaced: unreadable-name drops are now counted separately from pattern hits on both surfaces, so a user with zero AI-ignore patterns is no longer told their patterns hid everything — the message names the real cause.

saveLatched capture: fixed as suggested — the decision is captured in state at submit (including the notes-emptiness term), cleared on every terminal path including a phase-1 failure, so neither a mid-clobber refetch nor a checkbox toggle can drop or fake the latch.

Env-mutation test seam: fixed as suggested — a #[cfg(test)] in-process static consulted ahead of the env arm; the RAII guard swaps the static and process env is never mutated in tests (the oplog seam's own rule). The GD_SETTINGS_DIR arm remains production code, covered by the pure-parameter resolution test. Full suite ran clean repeatedly after the change.

Nits: label idiom matched; recovery dir now uses tempfile's exclusive creation + keep() (verified against the installed 3.27.0 — into_path is deprecated there and would fail -D warnings); the fill Preview floor is documented as fixed with only the textarea floor caller-tunable.

Copilot: the trim-divergence finding is declined with evidence in its thread (both paths receive byte-identical text — the edit trims server-side); the toast-wording finding is confirmed and fixed (arm-neutral summary, arm-specific detail in Details). Both threads resolved.

Also in this push, from our own post-round audit: the unterminated-bracket contract is now stated exactly (a later emitted class can re-close a copied [ — fail-closed, and now counted in widened), and the unterminated shapes are pinned by live parity rows rather than string assertions alone.

Posted by GitDesktop — automated agent comment, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-31

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

Re-review of the delta. All four should-fixes and all three nits from round 1 are addressed in the current diff, and the fixes themselves are well-formed (the syncArmed capture is cleared on every terminal path, the in-process store override removes the setenv race without weakening the panic-restore test, tempfile exclusive creation replaces the guessable recovery dir). One residual fail-open survives in the new bracket scanner; nothing here blocks merge.

Correctness

should-fix — src-tauri/src/git/ai_ignore.rs, posix_class_end (and its caller bracket_end): the helper searches for the first :] anywhere, while wildmatch takes the first ] after [: and only accepts it as a POSIX class when p[-1] == ':' (dir.c/wildmatch.c: for (s = p += 2; (p_ch = *p) && p_ch != ']'; p++) {} … if (i < 0 || p[-1] != ':') { p = s - 2; p_ch = '['; … }). So when the first ] is not preceded by : but a :] appears later, bracket_end over-scans past the terminator wildmatch actually uses. That is harmless while the expression is copied verbatim (output == input, so both engines re-parse the same string), but it fails OPEN on the ? route. Concrete case: AI-ignore pattern a[[:x]\-b:]c]d. wildmatch (gitignore side) ends the class at the ] at index 5, so the pattern hides basenames a[-b:]c]d / a:-b:]c]d / ax-b:]c]d; posix_class_end instead returns the :] at index 9→10, bracket_end returns the ] at 12, the whole 12-char stretch contains \ and collapses to one ?, emitting :(exclude,glob)**/a?d — which matches none of those 9-character names, so a file the user's rule hides from check-ignore rides into the staged/branch diff sent to the model. Contrived, but it is the one direction this module forbids. Fix, mirroring wildmatch:
rust fn posix_class_end(chars: &[char], open: usize) -> Option<usize> { let close = open + 2 + chars[open + 2..].iter().position(|&c| c == ']')?; // Non-empty name AND a `:` right before the `]`, or wildmatch re-reads the // inner `[` as an ordinary member and that `]` closes the enclosing class. (close >= open + 4 && chars[close - 1] == ':').then_some(close) }
(chars[open + 2..] is safe: bracket_end only calls this when chars[open + 1] == ':', so open + 2 <= len.) Knock-ons to apply in the same edit: reword posix_class_end's doc line "or None when no :] follows" to state the first-]-preceded-by-: rule; extend bracket_end's doc bullet "a POSIX class [:name:] is consumed whole" to say it is recognised the way wildmatch recognises it (first ] after [:, :-preceded, non-empty name), since the "four details" list is what a later reader will trust; and add a row to user_bracket_expressions_pass_through_or_widen_whole pinning the shape — with the fix, specs("a[[:x]\\-b:]c]d")[0] is ":(exclude,glob)**/a[[:x]?b:]c]d" (widened 1), a strict superset of what gitignore hides. All existing assertions (a[[:digit:]]d, a[[:alpha:][:digit:]]d, a[![:digit:]]d, a[[:x]d, a[[:digit:]\-]d) are unchanged by the new rule.

Nits

src-tauri/src/app_store.rs, resolve_store_dir: the env arm still precedes the cfg!(test) arm, so a developer or CI runner with GD_SETTINGS_DIR exported makes store_path() read that store in test builds — which contradicts arm 2's doc ("an in-crate test can never read the developer's real settings") and the exact-count comment on branch_recipe_counts_ignored_untracked_names. Reorder to _ if is_test => None first (the in-process override already covers tests that want a store), and update the doc's precedence list plus the resolve_store_dir(Some("C:/tmp/gd-store"), true) assertion in store_dir_resolution_arms, whose "an explicit override wins, in tests and out" would no longer hold.

src/features/repository/useGenerateBranchName.ts, the new else if (untracked.unreadable > 0) arm: unlike every sibling arm (and unlike the Rust twin's (false, false) if filtered.unreadable > 0 message) it drops the vs ${fallback.base} clause, so with a fallback base the toast is less informative than the branch either side of it — mirror the sibling shape (fallback ? \Nothing to name a branch after — the only new files have names that aren't readable text, and there are no net changes vs ${fallback.base}.` : …`).

Copilot's remaining open finding (notes trimmed for the sync but not for the edit, TagDetailView.tsx:418) is not an issue: gh_release_edit does let notes = notes.trim(); at src-tauri/src/github/release.rs:261 before building --notes, so the release body and the manifest receive the same string.

Resolved since last review

escapes_to_classes bracket-blindness and the escaped non-ASCII fail-open — rewrite_for_pathspec now copies user brackets whole (or widens the whole expression to ?), passes escaped non-ASCII bare, and counts the re-closed dangling [; parity + superset rows added.

filterPathsByAiIgnore U+FFFD guard — now applied ahead of the exclude.length === 0 short-circuit, with unreadable broken out on both surfaces so no message blames an empty pattern list; JSDoc reworded.

saveLatched — captured in syncArmed at submit (including the notes-emptiness term) and cleared on the !syncManifest, phase-2 success, phase-2 error and phase-1 error paths plus on dialog open.

SettingsDirOverride process-env mutation — replaced by the #[cfg(test)] TEST_STORE_DIR slot consulted by store_path ahead of the env arm; the panic-restore test now probes that slot.

Nits: sync <label> matches the cursor-not-allowed opacity-60 idiom; recovery dir uses tempfile::Builder…tempdir() + keep(); the fill doc comment now states Preview's floor is fixed.

Copilot's toast-wording finding — the summary line is arm-neutral and the recovery path stays in Details.

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

Posted by GitDesktop — AI output, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-07-31

Originally posted by: theBGuy

Round 2 dispositions (all in the next push):

posix_class_end over-scan: confirmed by measurement and fixed. Your concrete shape reproduced exactly — the old emission **/a?d hid neither 9-character name the gitignore engine hides. The fix mirrors the recognition rule you quoted (first ] after [:, :-preceded), with one deliberate divergence found while verifying it: wildmatch actually accepts an empty-named class ([::] abandons the pattern — measured), so our non-empty-name guard is stricter than the engine. That divergence only fires on [::]-degenerates and only fail-closed (gitignore hides nothing there, so any term we emit is a superset of nothing); the docs now state it as a divergence rather than claiming an exact mirror, and the one shape where it changes behavior (a[\x[::]d → **/a?d, over-exclusion) is pinned with a control row isolating the empty name as the trigger.

The invariant itself is now harness-locked cross-engine, not just the emitted string: a tree-route test (object-database fixtures, so :-bearing names exist on Windows) asserts gitignore-hidden ⊆ pathspec-hidden for the backslash-carrying POSIX shape, with a fixture precondition, a strictness witness, and two controls — and it was proven to fail against the pre-fix body before being accepted (the red run states the leak in one line).

Store-resolution arm order: fixed as suggested. cfg!(test) now precedes the env arm, so an exported GD_SETTINGS_DIR can't reach a test build. Proven both directions with the same hostile-environment command: before the reorder it flipped the hermetic test (the hole was real); after, the test is green under the same adversarial store while the in-process override still works. The override-wins-in-tests assertion is replaced by env-loses-under-test, and a new case pins that the production knob still wins outside tests.

Unreadable-arm fallback clause: fixed, mirroring the sibling arms' shape and the Rust twin's structure.

Also in this push from our own closing audit: four comment-accuracy corrections in the bracket scanner's docs (the [::] truth above, an off-by-one in the name-span comment, and two superseded phrasings retired).

Posted by GitDesktop — automated agent comment, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-08-01

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

Re-review of the delta. All three round-2 items are fixed, and the fixes themselves check out: posix_class_end now mirrors wildmatch's recognition rule (first ] after [:, :-preceded), the deliberate [::] divergence is fail-closed in the one direction it can fire, and the new object-database parity test asserts the direction rather than the emitted term. One should-fix remains, on the manifest patcher — non-blocking.

Correctness

should-fix — src-tauri/src/github/release.rs, patch_updater_notes (lines 419–428): the only content gate is "parses as a JSON object", so any release asset that happens to be named latest.json is rewritten as if it were a Tauri updater manifest. Concrete case: a repo publishes its own latest.json version-pointer (not a Tauri feed). canSyncUpdater (src/features/tags/TagDetailView.tsx:153-154) gates only on the asset name, and editSyncUpdater is reset to true on every Edit click (TagDetailView.tsx:206), so a "fix a typo in the notes" Save silently inserts a notes string into that file, re-serializes it alphabetized + pretty-printed, and re-uploads with --clobber — delete-then-upload, so a transient upload failure leaves the release with no copy of that asset at all, only the parked temp file. Fix in patch_updater_notes, which runs before the upload, so the asset is never deleted on this path:
rust let looks_like_manifest = obj.get("version").is_some_and(serde_json::Value::is_string) && obj.get("platforms").is_some_and(serde_json::Value::is_object); if !looks_like_manifest { return Err(AppError::Gh(format!( "{UPDATER_MANIFEST} isn't a Tauri updater manifest (no `version` + `platforms`) — it was left unchanged." ))); }
Knock-ons to apply in the same edit: the doc comment on patch_updater_notes currently promises only "Replaces the manifest's notes and nothing else" — add the shape gate to it, since that is what a later reader will trust; the existing patch_updater_notes_adds_absent_notes fixture {"version":"1.0.0"} fails the new gate, so give it a "platforms": {…} object (its point is the absent-notes insert, not the missing platforms); add a row beside patch_updater_notes_rejects_a_non_object pinning a plain object ({"foo":1}) as rejected. No UI change is needed — the error arrives on the phase-2 arm the existing arm-neutral toast already covers, and with nothing deleted the "may not have been" wording stays accurate — and no documentation surface needs touching: README and src/features/help/content.ts already qualify the feature with "carries a latest.json updater manifest".

Resolved since last review

posix_class_end over-scan — now takes the first ] after [: and requires it be :-preceded; bracket_end's "four details" bullet and posix_class_end's doc both state the rule and the [::] divergence, and the pinning rows (a[[:x]\-b:]c]d → **/a[[:x]?b:]c]d, a[[:]]d, a[[::]]d, a[d[::]b, a[\x[::]d → **/a?d) plus the new widened_bracket_never_under_excludes_the_gitignore_set subset test land with it. The close >= open + 4 name-span comment and the (open + 2..chars.len()) range are both correct (empty range rather than a panic when open + 2 > len).

resolve_store_dir arm order — _ if is_test => None now precedes the env arm; the doc's precedence list, the oplog cross-reference ("it outranks the env var where the oplog's does not" — matches oplog::resolve_store_base, which does the opposite), the test's "Arm 1 is why…" prose and the two replacement assertions are all consistent.

useGenerateBranchName.ts unreadable arm — now carries the vs ${fallback.base} clause, matching its sibling arms and the Rust twin's (false, false) if filtered.unreadable > 0 shape.

Leftover polish (non-blocking)

src/features/tags/TagDetailView.tsx:154 hardcodes "latest.json", duplicating Rust's UPDATER_MANIFEST; if the asset name ever moves, the UI gate silently stops matching — a shared TS constant beside forgeReleaseSyncUpdaterNotes in src/lib/git/api.ts would keep the two ends nameable together.

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

Posted by GitDesktop — AI output, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-08-01

Originally posted by: theBGuy

Round 3 dispositions (in the next push):

Foreign latest.json shape gate: confirmed and fixed. patch_updater_notes now requires version to be a string and platforms an object before touching anything; the gate sits ahead of the upload, so a rejected asset is never deleted — the sync refuses with "…isn't a Tauri updater manifest (no version + platforms) — it was left unchanged." One deliberate divergence from the suggested snippet: the tests also pin right-keys-wrong-types shapes (version as a number, platforms as an array), the half a key-presence check would miss. On the error variant: Gh was kept after weighing it against this file's own local-failure-maps-to-Io rule — the content being judged is a remote artifact GitHub served, so Gh is the honest attribution and matches the two sibling rejection arms; InvalidArgument renders as "invalid argument: …", which would misdescribe remote content in the toast's first line.

Shared asset-name constant: done — UPDATER_MANIFEST_NAME exported beside the api binding with a KEEP-IN-SYNC pointer at Rust's UPDATER_MANIFEST, and the dialog gate now uses it. The two remaining latest.json literals are user-facing prose and a comment, left readable on purpose.

Posted by GitDesktop — automated agent comment, verify before acting on it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-08-01

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

The round-4 delta adds the manifest shape gate to patch_updater_notes (string version + object platforms, ahead of the clobbering upload) and replaces the hardcoded UI asset name with the shared UPDATER_MANIFEST_NAME. Both fixes are correct and integrate cleanly — the gate sits after the object cast and before the notes insert, so a rejected asset is never deleted; the TempDir still cleans up the downloaded copy on that path; the new tests cover the wrong-types shapes the presence-only check would miss; and the TS constant is a single import with a KEEP-IN-SYNC pointer at the Rust const. Nothing blocking.

Readability

nit — src-tauri/src/github/release.rs:434-435: the message says "no version + platforms" even when both keys are present but mistyped ({"version":1,"platforms":{}}, the exact shape the new test pins). "doesn't carry a string version + object platforms" matches what the gate actually checks and what the doc comment above already says.

Resolved since last review

Foreign same-named asset rewrite (patch_updater_notes, release.rs:427-437): the shape gate is in place ahead of the upload, the doc comment now states it, patch_updater_notes_adds_absent_notes carries a platforms object (and asserts the platform signature survives), and patch_updater_notes_rejects_a_foreign_same_named_asset pins {"foo":1} plus both wrong-type shapes. The Gh error variant remains a recorded decision (remote-served content).

Leftover polish item — hardcoded "latest.json" in TagDetailView.tsx:155 now reads UPDATER_MANIFEST_NAME from src/lib/git/api.ts:544, which carries the KEEP-IN-SYNC note at Rust's UPDATER_MANIFEST; the two remaining literals in that file are the checkbox label and a comment.

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.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-08-01

Originally posted by: theBGuy

Round 4 disposition: the wording nit is accepted and applied in the working tree — the rejection message now names what the gate actually checks ("needs a string version and an object platforms"), matching the doc comment. It rides the author's next push if one happens; if the PR merges without it, it lands with the next change to this file (the message is factually imperfect only for present-but-mistyped keys, and the gate's behavior is unaffected either way).

With that, the board is converged: round 4 raised nothing beyond this wording polish, CI is green on the final head, both inline threads carry recorded resolutions, and every review round's findings are fixed or declined with evidence on this record. Merge-ready — the merge is the author's call.

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

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-08-01

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

The only change since the last review is the rejection-message wording in patch_updater_notes; it lands the round-4 nit and introduces no collateral (the \ continuations keep the intended single spaces, the inline {UPDATER_MANIFEST} capture is unchanged, and the doc comment above already described the gate as "string version + object platforms", so the two now match).

Resolved since last review

src-tauri/src/github/release.rs:433-436 — the message now reads "isn't a Tauri updater manifest (needs a string version and an object platforms) — it was left unchanged.", which is exactly what the gate checks and what patch_updater_notes_rejects_a_foreign_same_named_asset pins for the present-but-mistyped shapes.

Copilot's trim-divergence note on TagDetailView.tsx:419 is not an issue: gh_release_edit trims notes server-side (src-tauri/src/github/release.rs:261) before building --notes, so the release body and the synced manifest receive the same string.

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.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2026-08-01

Ticket changed by: theBGuy

status: open --> closed
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

feat(releases,ai,git,mcp): sync updater notes when editing a release

Milestone

Searches

Help

#135 feat(releases,ai,git,mcp): sync updater notes when editing a release

Updater-manifest sync

Release notes editor sizing

AI-ignore matching

Untracked names in branch-name generation

Stash zero-match reporting

Unignore exact match

Documentation

Related

Discussion

Deploying gitdesktop with Cloudflare Pages

Correctness

UI

Nits

Tests

Correctness

Nits

Resolved since last review

Correctness

Resolved since last review

Leftover polish (non-blocking)

Readability

Resolved since last review

Resolved since last review

Latest commit:	`674ae71`
Status:	✅ Deploy successful!
Preview URL:	https://91a6c0d2.gitdesktop.pages.dev
Branch Preview URL:	https://feat-release-updater-sync-ai.gitdesktop.pages.dev