GitDesktop / Tickets / #110 feat(lan,companion): add read-only tags, code TODOs, and Discussions

Anonymous - 2026-07-24

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

Deploying gitdesktop with Cloudflare Pages

Latest commit: 40b5135

Status: ✅ Deploy successful!

Preview URL: https://07e40512.gitdesktop.pages.dev

Branch Preview URL: https://feat-companion-tags-todos-di.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-24

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-24

Originally posted by: theBGuy

Review context — deliberate decisions in this slice (pre-scoping so review rounds ground on them):

Read-only surface by design. No write routes; discussion upvote counts render display-only (no upvote/reply/compose affordances). The viewerDidAuthor/viewerHasUpvoted wire fields are served (core fns unchanged) but deliberately unstyled on the phone.

Server-side forge gate is new, and load-bearing. The desktop gates Discussions client-side (forgeSupports), so the gh_discussion_* core fns have no host guard of their own — the LAN routes therefore gate via detect_non_github before any gh invocation, minting 400 { kind: "discussionsUnavailable" }. That kind string is a verbatim cross-layer contract with companion/src/lib/api.ts (isDiscussionsUnavailable); both sides carry sync comments. None/GHE arms proceed on purpose (gh-default routing).

?category= is a GraphQL category node id, not a name — verified against gh_discussion_list's query variables and the desktop's own call sites.

TODO scan never polls (deliberate divergence from the other hooks — a 15s git grep sweep is waste); freshness = staleTime + Rescan. The default marker set exists in three synced copies by design (desktop markers.ts, server fallback, companion chips), each cross-referenced in comments.

Segment visibility is data-driven (meta.data?.hasDiscussionsEnabled, not isSuccess) because react-query v5 retains data but flips status on a transient same-key refetch failure — an isSuccess gate would flash the segment on wifi blips. It's also hoisted above IssuesBody's early returns so an issues-disabled-but-discussions-enabled fork still has a path to Discussions.

Hide-AI untouched: none of the three surfaces are AI features.

Docs-sync: README/site/help remain on the epic-wide deferral (epic→master close); this PR amends the existing changelog fragment only.

Validation: 926 cargo tests + clippy -D warnings clean (4 new route tests incl. a gitlab-remote gate test) · both tsc + full build green · 4-lens adversarial review panel (8 findings, all fixed pre-PR) · live curl E2E on a cold-start worktree build: all 5 routes 200 with real data, alias==scoped byte-identical, BAD*MARKER → 400 (injection guard), no-cookie/post-revoke → 401, nonexistent number/category → structured {kind:"gh"} errors. Known not-live-exercised: discussions detail with real content (this repo had zero discussions — empty-list path validated; thread rendering verified statically), and the three new screens await a phone pass.

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-24

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

This PR adds three read-only companion surfaces (Tags, Code TODOs, GitHub Discussions) to the LAN phone app plus their Rust routes, and it's well-executed: the TS wire shapes match the desktop's serde structs I verified, the server-side forge gate is sound, routing degrades safely, keyboard nav is wired, and the Rust routes have real tests (newest-first tags, default-marker scan, bad-marker rejection, non-GitHub gate). No blockers. A couple of should-fixes and a nit below.

Docs / conventions

should-fix — src/features/help/content.ts, the "What you can see on your phone" section (~lines 1682–1710). This PR adds Tags, Code TODOs, and Discussions as browsable companion surfaces, but the in-app guide's bulleted list of phone surfaces still enumerates only Status / PRs / Issues / Changes / History / Branches / CI / Agents. Since Changes/History/Branches (also drill-ins) are already listed there, the three new surfaces belong in the same list, and per the repo's docs-sync rule the guide should stay accurate in the same change. The changelog fragment was extended, so this looks like an oversight rather than a deliberate skip. Fix: add Tags, Code TODOs, and Discussions bullets to that list (and confirm README Features / the marketing capabilities list don't need the same, or that they're intentionally batched for the epic→master close).

Performance / consistency

should-fix — companion/src/lib/queries.ts, queryClient retry predicate (lines 59–63). The predicate excludes isUnauthorized/isNoActiveRepo/isNoSuchRepo from retry but not isDiscussionsUnavailable, which is equally definitive (a retry can't turn Discussions on). Concrete path: IssuesDiscussionsSegment calls useDiscussionMeta on every Issues-screen visit; on a GitLab/Bitbucket repo /discussions/meta returns 400 discussionsUnavailable, which then gets retried once — a doomed second request each meta fetch, exactly the "don't retry a definitive error" behavior the surrounding comments go out of their way to avoid. Fix: add err.isDiscussionsUnavailable to the exclusion tuple. Low impact (meta has a 5-min staleTime), but it's a clean consistency fix.

Readability

nit — companion/src/screens/Discussions.tsx, DiscussionRow. The list is server-ordered by UPDATED_AT DESC (the Rust LIST_QUERY), but each row shows timeAgo(discussion.createdAt). So the top ("most recent") row can read e.g. "created 2 years ago" for a thread with recent activity, which reads as inconsistent with the ordering. If it's meant to match the desktop, fine; otherwise consider surfacing an updated timestamp or a small "active" cue.

nit — companion/src/screens/Discussions.tsx, CategoryChips.onKeyDown moves focus via e.currentTarget.parentElement?.children[next], coupling to DOM structure, while the sibling MarkerToggles in Todos.tsx uses a ref array and tracks focusIndex in state. The two new roving groups also differ in tab-stop behavior (CategoryChips pins the stop to the selected chip; MarkerToggles pins it to the last-focused). A ref-based approach in CategoryChips would be more robust and align the two.

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-24

Originally posted by: theBGuy

🤖 GitDesktop AI security audit · opus · automated

I verified the two attacker-controlled sinks this PR newly exposes over the LAN companion. The todos marker path is safe (is_valid_marker restricts markers to [A-Za-z][A-Za-z0-9_-]* and they enter git grep -E as a single argv regex — no shell, no arg injection). The discussions category path is not.

Severity: High — Confidence: 7/10

argument-injection / arbitrary-file-read — src-tauri/src/lan/routes/forge.rs (discussions_list → DiscussionListQuery.category), reaching crate::github::discussion::gh_discussion_list which builds gh api graphql -F category={cat}.

Exploit scenario: The new LAN route GET /api/forge/discussions?category=… (and its scoped twin /api/repos/{id}/discussions?category=…) takes category: Option<String> straight from the query string with no validation beyond non-empty, and forwards it into gh_discussion_list, which appends -F + format!("category={cat}") to the gh api graphql argv (discussion.rs:246–248). gh's -F/--field performs magic value conversion: a value beginning with @ is read from a file on the local filesystem (@- = stdin). So an authenticated companion client on a GitHub-origin shared repo (the gate discussions_allowed passes for GitHub, so the request reaches gh) can send:

GET /api/forge/discussions?category=%40/home/<user>/.ssh/id_rsa GET /api/forge/discussions?category=%40C:\Users\<user>\.gitconfig

gh reads that file off the desktop host and sends its contents to GitHub as the $category: ID GraphQL variable. GitHub can't resolve it and returns a GraphQL error that echoes the supplied value (e.g. "Could not resolve to a node with the global id of '<file contents="">'"</file>); run_gh wraps that stderr into AppError::Gh(msg) (runner.rs:100–106), and the LAN boundary serializes the message into the 502 response body (app_error_response) — so the file contents are reflected back to the caller. Even absent perfect reflection, the file is exfiltrated to GitHub under the desktop user's credentials. This escapes the companion's intended "read-only git/forge data for shared repos" boundary into arbitrary host-file disclosure (SSH keys, tokens, .gitconfig, /etc/passwd, /proc/self/environ, etc.).

I verified: (1) the route forwards category unvalidated (diff, forge.rs); (2) gh_discussion_list uses -F (magic-value field), not -f (raw field) — discussion.rs:246–248; (3) the error path returns gh's stderr in the response body — runner.rs + app_error_response. The residual uncertainty is only how completely GitHub's error reflects multi-line content, which is why this is 7/10 rather than higher; the local file read + exfil-to-GitHub is certain regardless.

Remediation: Treat category as the opaque node id it is — pass it with -f (--raw-field, no magic conversion) instead of -F, or validate it against the GitHub node-id charset (^[A-Za-z0-9_=-]+$) before forwarding. Note the same -F magic applies to any other free-form string forwarded to gh api from an attacker-reachable route, so audit siblings (the after cursor already correctly uses -f).

(Copilot's three comments on this PR are all documentation/comment-accuracy nits — help-guide docs-sync and JSDoc wording about the disabled-vs-non-GitHub state ladder — not security issues; nothing to action here.)

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-24

Originally posted by: theBGuy

Disposition — GitDesktop AI review (all four findings triaged; fixes are in the branch awaiting the next push):

Help guide (should-fix) — accepted, fixed. You're right that this was an oversight, not the deferral: the guide's phone-surfaces list is per-slice maintained (it already carries the slice-6 drill-ins). Added Tags, Code TODOs, and Discussions (incl. the Issues | Discussions switch and when it hides) to src/features/help/content.ts. README/site remain the epic-close batch, as pre-scoped.

Retry predicate (should-fix) — accepted, fixed. isDiscussionsUnavailable added to the no-retry exclusions with a comment explaining why it's definitive (a retry can't change the host). Sharp catch — it's exactly the definitive-error class the predicate exists for, and the segment's per-visit meta fetch made it a real doomed-request path on GitLab/Bitbucket repos.

createdAt on an UPDATED_AT DESC list (nit) — declined, desktop parity + wire limit. The desktop's own Discussions list renders formatRelativeTime(createdAt) against the same server ordering, and DiscussionInfo carries no updatedAt field — surfacing one would be a wire-shape change to a long-shipped desktop struct, out of scope for a read-only companion slice. Noted as a possible joint desktop+companion polish later.

Roving-pattern divergence (nit) — accepted as noted. The tab-stop difference is deliberate single- vs multi-select semantics (selected-pinned for the category filter, last-focused for the marker toggles), and the DOM-walk matches the BottomNav's shipped house pattern — but the ref-array approach is the more robust idiom, and aligning CategoryChips to it is a fair follow-up if either row ever restructures.

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-24

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

This PR adds three read-only companion surfaces (Tags, Code TODOs, GitHub Discussions) plus their LAN routes; the wire shapes mirror the desktop structs, the server-side forge gate short-circuits before any gh call, routing degrades safely, and the Rust routes have real tests. It's sound and nothing is blocking — one should-fix around a UI inconsistency, plus resolved items from the last round.

UX consistency

should-fix — companion/src/screens/Discussions.tsx, DiscussionsBody. The IssuesDiscussionsSegment is rendered only in the final content return; every earlier bail-out path returns without it, including the list's first-load branch:
jsx const data = list.data; if (!data) { if (list.isError) return <ErrorState … />; // no segment return <SkeletonRows />; // no segment }
IssuesBody was deliberately restructured to hoist the segment above the equivalent states ({segment}<SkeletonRows/>, {segment}<ErrorState/>, etc.), but DiscussionsBody does not mirror that. Concrete path: from an Issues screen on a GitHub repo with Discussions enabled, meta is already cached (5-min staleTime) so metaOk is true; tapping Discussions navigates in, the list's first fetch has no data yet (keepPreviousData has nothing to hold on the first request), so if (!data) returns a bare <SkeletonRows /> — the [ Issues | Discussions ] switch the user just tapped flashes away, then reappears when the list resolves. On a first-load list error it's worse: the switch is absent for the whole error state (the user falls back to the bottom-nav Issues tab, so it's not a dead-end, but it contradicts the segment's stated no-flash / "reads as one surface" intent that Issues upholds). Fix: hoist the segment above the if (!data) list branch the same way IssuesBody does.

nit — companion/src/screens/Todos.tsx (TodoList rows) vs companion/src/screens/Tags.tsx (TagsBody rows). Two sibling new roving lists differ in tab-stop behavior: Tags tracks focusName in state so the single tab stop follows the last-focused row (Tab away and back returns to it), while Todos hard-pins tabIndex={i === 0 ? 0 : -1} to row 0, so after arrowing down and tabbing away/back you land on row 0, not where you were. Aligning Todos on the Tags approach would make the two consistent.

Resolved since last review

src/features/help/content.ts — the "What you can see on your phone" list now carries Discussions, Tags, and Code TODOs bullets; docs-sync gap closed. (README/marketing site remain deliberately batched for the epic→master close per the author's note — not re-flagging. Also covers Copilot's docs-sync comment.)

companion/src/lib/queries.ts — err.isDiscussionsUnavailable is now in the no-retry exclusion tuple, so the per-visit meta probe on a GitLab/Bitbucket repo no longer double-fires.

companion/src/lib/api.ts (isDiscussionsUnavailable JSDoc) and companion/src/screens/Discussions.tsx header — both now correctly state the two-shape contract (400 discussionsUnavailable = non-GitHub host only; feature-off = 200 with hasDiscussionsEnabled: false), matching the state ladder the code implements. (Resolves the Copilot comments on those two lines.)

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-24

Originally posted by: theBGuy

Disposition — round 2 (security audit + second AI review). All findings fixed; changes are in the working tree, uncommitted, pending push.

🔒 Security audit — High, confirmed and fixed (the headline)

The audit is correct: the new ?category= LAN param flowed unvalidated into gh api graphql -F category={cat}, and gh's -F magic-converts a leading @ into a file read — so an authenticated companion client on a GitHub-shared repo could read arbitrary host files (SSH keys, tokens) and exfil them to GitHub. The desktop was never exposed (it only passes trusted node-ids from the category dropdown); this PR's query param is what opened the sink. Fixed in two layers:

Sink (the complete fix) — src-tauri/src/github/discussion.rs: category now travels as a raw field (-f, no magic conversion) instead of -F, exactly as the adjacent after cursor already does. $category is a GraphQL ID (string-compatible), so this is behavior-preserving for every legitimate caller (desktop included).

Boundary (defense-in-depth) — src-tauri/src/lan/routes/forge.rs: discussions_list now validates a present category against the GitHub node-id charset ([A-Za-z0-9_\-=+/]) and returns 400 invalidArgument on anything else, so a hostile value never reaches gh at all.

Tests (offline, no gh/network): a unit test of the validator (accepts real modern/legacy node-id shapes, rejects @…, @-, embedded @, whitespace, empty) + a router-level test proving GET /api/forge/discussions?category=@/etc/passwd on a GitHub-origin repo (gate passes) returns 400 invalidArgument before any gh call. Full suite now 929 passing + clippy -D clean.

(The audit also confirmed the todos marker path is safe via is_valid_marker, and that Copilot's three comments were docs/comment-accuracy — matches my round-1 triage.)

GitDesktop AI review (round 2)

Segment hoist in DiscussionsBody (should-fix) — accepted, fixed. You caught that IssuesBody was hoisted but DiscussionsBody wasn't: its if (!list.data) first-load branch returned a bare skeleton, so the [ Issues | Discussions ] switch flashed away on the way in. Hoisted the segment above the list's early returns (it's only reached once meta is confirmed enabled, so the meta-gated teaching states stay bare on purpose — the segment self-hides there anyway).

Todos vs Tags roving tab-stop (nit) — accepted, fixed. The Todos results rows now track the last-focused hit by stable path:line identity (falling back to row 0), matching TagsBody/BranchesBody — Tab-away-and-back returns to your row, and a rescan keeps the stop on the same hit.

Gates on the full tree: 929 cargo + clippy -D clean · both tsc + full pnpm build green · biome clean. Round-2 footprint = 5 files (+142/−10): discussion.rs, forge.rs, lan/mod.rs, Discussions.tsx, Todos.tsx.

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-24

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

This PR adds three read-only companion surfaces (Tags, Code TODOs, GitHub Discussions) with their LAN routes, and closes out the security and UX items from the prior round. It's sound and nothing is blocking — the category-injection fix is correct and well-tested, and both prior findings are addressed.

Resolved since last review

companion/src/screens/Discussions.tsx — segment hoist. DiscussionsBody now defines segment once meta is confirmed enabled and renders it above the if (!data) list branch ({segment}{list.isError ? <ErrorState/> : <SkeletonRows/>}), mirroring IssuesBody. The [ Issues | Discussions ] switch no longer flashes away during the list's first-load skeleton/error. Confirmed in the current diff.

companion/src/screens/Todos.tsx — roving tab-stop parity. TodoList now tracks focusKey by stable path:line identity with a hoisted membership check and tabIndex={(activeKey != null ? key === activeKey : i === 0) ? 0 : -1}, matching TagsBody/BranchesBody. The two sibling lists are now consistent. Confirmed in the current diff.

Security

The -F → -f change in gh_discussion_list (src-tauri/src/github/discussion.rs) is the correct sink fix: -f sends a raw string field with no @-file-read magic, and $category is a GraphQL ID so string passing is behavior-preserving for the desktop's trusted node-ids. The is_valid_category_id boundary guard in discussions_list (rejecting anything outside [A-Za-z0-9_\-=+/], including empty) is sound defense-in-depth, and the two node-id charset variants (base64url DIC_kwDO…, padded base64 MDE…==) both pass. The offline router-level injection test plus the unit tests cover the vector well. No issue.

Readability

nit — companion/src/screens/Todos.tsx, TodosResults truncation banner. The string "Showing the first 2,000 matches …" hardcodes the server's hit cap, which lives independently in git_todo_scan (the todos handler comment in git.rs states "caps hits at 2000"). Since the companion never sends maxHits, the two match today, but if the server cap ever changes the banner silently lies. Consider surfacing the effective cap (or at least cross-referencing it in a comment) so the number can't drift.

Docs-sync: the changelog fragment and in-app help guide are updated for all three surfaces; README/marketing-site are deliberately batched for the epic→master close per the author's recorded decision — not re-flagging. Copilot's three comments (docs-sync, api.ts:110 JSDoc, Discussions.tsx:10 header) are all addressed in the current diff.

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-24

Originally posted by: theBGuy

Disposition — round 3 (re-review). Thanks for the clean re-review — confirming the security fix and both prior UX fixes as resolved. One new nit addressed:

Truncation-banner cap drift (nit) — accepted, fixed. You're right that the hardcoded "2,000" duplicates the server's DEFAULT_MAX_HITS independently. Surfacing the effective cap would need a wire-shape change (only the truncated boolean is on the wire, not the numeric cap) — disproportionate for a nit — so I applied this slice's established coupling idiom instead (the same "keep in sync" cross-reference the DEFAULT_MARKERS copies use): a comment above the banner naming DEFAULT_MAX_HITS in src-tauri/src/git/todos.rs, so a grep DEFAULT_MAX_HITS from a maintainer changing the Rust cap now reaches the companion string. Verified the symbol exists (const DEFAULT_MAX_HITS: u32 = 2000). Comment-only change, +3 lines, tsc + biome clean.

No open threads remain. All rounds resolved: Copilot 3/3 (docs-sync + two comment-accuracy), AI review R1 4 findings (2 fixed, 1 declined-with-evidence, 1 noted), security audit High (2-layer fix + tests), AI review R2/R3 (segment hoist, roving tab-stop, banner comment).

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-24

Originally posted by: theBGuy

🤖 GitDesktop AI review · opus · automated

This PR adds three read-only companion surfaces (Tags, Code TODOs, GitHub Discussions) with their LAN routes plus a server-side forge gate and a category-injection guard. It's sound, well-tested, and nothing is blocking — the security fix and boundary guard are correct and the state ladders mirror the existing Issues/Branches screens faithfully.

Resolved since last review

companion/src/screens/Todos.tsx — TodosResults truncation banner. The prior nit (hardcoded "2,000" silently drifting from the server cap) is addressed: a comment now names DEFAULT_MAX_HITS in src-tauri/src/git/todos.rs as the coupling point with a grep DEFAULT_MAX_HITS breadcrumb. Confirmed in the current diff. This is the only substantive change since the last round; both earlier resolved items (segment hoist, identity-keyed roving tab-stop) remain resolved.

Correctness / edge cases

nit — companion/src/lib/queries.ts, useDiscussion. refetchInterval is on ? POLL_MS : false where on = repoId != null && number != null, so the detail view keeps polling even after a definitive failure. Concrete path: deep-linking #r/{id}/discussions/5 on a repo whose discussion [#5] doesn't exist (or, less cheaply, a GitHub repo where the number is bogus) leaves the screen in its error/teaching state while react-query re-fires the doomed request every POLL_MS. The retry-predicate change correctly stops retries but not the interval. This mirrors useIssue's existing behavior, so it may be intentional parity — but if you want the teaching state to be quiet, gate refetchInterval off on a definitive error (e.g. return false from a function form when isDiscussionsUnavailable). The list query is already immune (it's enabled-gated on metaOk); only the detail view has this. Low impact since the non-GitHub case 400s before any gh call.

Security

The -F → -f sink fix in gh_discussion_list is correct and complete on its own (-f never triggers @-file-read magic, and $category is a GraphQL ID so string passing is behavior-preserving for the desktop's trusted node-ids), and is_valid_category_id is sound belt-and-braces: it excludes @/whitespace and accepts both node-id charsets (base64url and padded base64 with + / =), and the companion encodeURIComponents +// so legacy ids survive the round-trip and pass the charset check. The offline injection test (%40%2Fetc%2Fpasswd → 400 invalidArgument) plus the validator unit tests cover the vector well. No issue.

Notes

Docs-sync: the changelog fragment and help guide cover all three surfaces; README/marketing-site are the epic-close batch per the author's recorded standing deferral — not re-flagging.

The three Copilot comments (changelog docs-sync, api.ts:110 JSDoc, Discussions.tsx:10 header) are all addressed in the current diff — the JSDoc and header now correctly split the 400-host case from the 200 hasDiscussionsEnabled:false case; not issues.

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

Related

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

Anonymous - 2026-07-24

Originally posted by: theBGuy

Disposition — round 4 (re-review). Clean pass confirming the banner-comment fix; one new nit, declined with evidence:

useDiscussion polls after a definitive failure (nit) — declined, parity + the suggested fix wouldn't catch the described case.

Two reasons:

It's the codebase-wide detail-hook pattern, not a useDiscussion quirk. refetchInterval: on ? POLL_MS : false is identical across every companion detail hook — usePr, useCiRun, useIssue, usePrTimeline, usePrThreads (queries.ts lines 148/167/193/203/326/411). useDiscussion mirrors its sibling useIssue exactly, which is deliberate (the other reviewers have been flagging sibling inconsistencies — introducing a divergence here for one hook would create one). If the quiet-teaching-state behavior is wanted, it's a dedicated cross-hook change, not something this slice should fork one hook to do.

The suggested fix (refetchInterval: false on a definitive error) wouldn't stop the scenario you describe. A bogus discussion number returns 502 {kind:"gh"} (confirmed in the live E2E: /api/forge/discussions/99999 → 502 "Could not resolve to a Discussion…"), which is transient-classed — none of the definitive getters (isNoSuchRepo, isDiscussionsUnavailable, …) match kind:"gh", so a definitive-error gate on the interval would leave the bogus-number poll running. And isDiscussionsUnavailable on the detail is near-unreachable (the detail is only entered once the segment/list already confirmed availability). So the fix would neither address the main case nor apply to the reachable one.

Low impact confirmed (non-GitHub 400s before any gh call; bogus-number is a deep-link-only footgun the UI never generates). Flagging it on the record as a candidate epic-close pass across all detail hooks if we ever want definitive-error polls to go quiet.

All prior findings remain resolved; no open threads. I consider the review converged — happy to address anything further if another round surfaces.

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-24

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(lan,companion): add read-only tags, code TODOs, and Discussions

Milestone

Searches

Help

#110 feat(lan,companion): add read-only tags, code TODOs, and Discussions

LAN server routes (Rust)

Companion screens & navigation (frontend)

Documentation

Discussion

Deploying gitdesktop with Cloudflare Pages

Docs / conventions

Performance / consistency

Readability

UX consistency

Resolved since last review

🔒 Security audit — High, confirmed and fixed (the headline)

GitDesktop AI review (round 2)

Resolved since last review

Security

Readability

Resolved since last review

Correctness / edge cases

Security

Notes

Related

Latest commit:	`40b5135`
Status:	✅ Deploy successful!
Preview URL:	https://07e40512.gitdesktop.pages.dev
Branch Preview URL:	https://feat-companion-tags-todos-di.gitdesktop.pages.dev