feat: add notification inbox triage

[mariusvniekerk]

Summary

• Add an Inbox backed by GitHub notifications for monitored repositories
• Support filtering, sorting, selection-based bulk read/done actions, and background GitHub read propagation
• Wire notification sync, generated API clients, UI route/navigation, and design-system examples

[roborev-ci]

roborev: Combined Review (d5a2f41)

Notification inbox changes look directionally sound, but several medium/high issues should be addressed before merge.

High

• internal/server/notifications_test.go:43
  Notification inbox coverage is limited to narrow unit/httptest cases despite adding new API/data-flow/UI behavior and core bulk triage actions. Full-stack e2e coverage for listing, filtering, syncing, selecting, and marking notifications done/read is missing.
  Fix: Add an e2e test using the real HTTP API and SQLite, plus Playwright coverage for the Inbox route and bulk actions.

Medium

• internal/server/notifications.go:61
  Bulk read/done/undone endpoints return all requested IDs as succeeded even when an ID does not exist or no row was updated.
  Fix: Validate requested IDs against existing notification rows before mutation, compare affected rows, and return unknown/unmutated IDs in failed.

• internal/github/notifications_sync.go:24
  Notification sync and read propagation stop at the first host error, so one failing GitHub host can prevent other configured hosts from syncing or propagating queued reads.
  Fix: Process each host independently, log or aggregate per-host errors, and continue syncing remaining hosts.

• packages/ui/src/stores/notifications.svelte.ts:149
triggerSync() only starts the background sync and never reloads or polls notifications afterward, so the Inbox can remain stale after the user clicks “Sync notifications.”
  Fix: After a successful sync trigger, poll/reload the list until new data or sync status is observed, and surface sync failures.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (03986da)

Notification inbox changes look mostly sound, but two medium issues need follow-up before merge.

Medium

• internal/db/queries_notifications.go:251 - Inbox visibility is scoped by rows in middleman_repos, not by the current monitored repo configuration. If a repo was previously monitored and remains in SQLite, its notifications can still appear in unread, active, and all after removal from monitoring.

  • Fix: Scope notification list/summary queries against the active configured repo set, or persist an active/monitored flag and update it when config changes.

• internal/github/notifications_sync.go:31 - Closed/merged linked notifications are only auto-completed during notification sync. Normal PR/issue sync paths that discover closure do not call this, and repo sync and notification sync start independently, so closed items can remain active until a later notification sync.

  • Fix: Invoke MarkClosedLinkedNotificationsDone after PR/issue state updates in repository sync paths, or after repository sync completes before refreshing notifications.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (5f02d95)

Notifications feature has one high-risk state regression and two medium coverage/performance gaps to address before merge.

High

• [internal/db/queries_notifications.go] UpsertNotifications: Notification sync overwrites locally queued read state with unread = excluded.unread. If a user marks a notification read locally and GitHub still reports it unread before propagation succeeds, the row can reappear in the unread inbox despite the queued local action.
  Fix: Preserve unread = 0 while github_read_queued_at IS NOT NULL AND github_read_synced_at IS NULL, unless GitHub reports newer unread activity that should reactivate the row.

Medium

• [internal/github/notifications_sync.go] syncNotificationsForHost: Every notification sync fetches all pages with All: true and no Since watermark. On accounts with many notifications, the 2-minute loop can repeatedly walk the full notification backlog and waste rate limit.
  Fix: Persist a per-host successful notification sync watermark and use Since with a small overlap after the initial full sync.

• [internal/server/notifications_test.go]: The new notification workflow lacks e2e coverage for closed linked PR/issue auto-completion and GitHub read propagation retry/status behavior, both core flows introduced by this feature.
  Fix: Add full-stack HTTP + SQLite coverage that seeds linked notifications, closes/merges the linked item, verifies auto-done behavior, and exercises queued read propagation success/failure metadata.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (fb76f6b)

Adds a useful notification inbox, but there are Medium issues around failed sync visibility and GitHub Enterprise PR routing.

Medium

• internal/server/notifications.go:74 - /notifications/sync drops background sync errors, so a failed refresh still returns 202 and the UI only polls stale data with no visible failure state. Record notification sync running/error status and expose it to the UI, or log and surface the failure through an API status endpoint the inbox store polls.

• packages/ui/src/views/InboxView.svelte:50 - Opening PR notifications always navigates to /pulls/{owner}/{repo}/{number} without platform_host, while issue navigation preserves non-github.com hosts. GitHub Enterprise PR notifications can open the wrong local item or fail when owner/repo/number overlaps. Add platform-host support to PR routes/detail loading, or open web_url externally for non-github.com PR notifications until hosted PR routing exists.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (949ef69)

Manual sync still bypasses notification disablement, so the PR needs one medium fix before merge.

Medium

• internal/server/huma_routes.go:1938 - triggerSync always starts SyncNotifications, even when [notifications].enabled = false. The config only disables the periodic loops in cmd/middleman/main.go, so a normal manual sync can still call the GitHub notifications API.
  • Fix: pass notification enablement into the server and gate both the global sync hook and /notifications/sync on it.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (abb7246)

Medium issues found: notification counts and sync error status can misrepresent inbox state.

Medium

• internal/db/queries_notifications.go in NotificationSummary

  • Summary counts reuse the full list filter, including State, so state=unread reports done=0 and total_active only counts unread rows. This makes Inbox tab/header counts misleading and hides read-but-active backlog counts.
  • Fix: build summary/facet queries from non-state filters, or run separate count queries for unread, active, and done while preserving repo/reason/type/search filters.

• cmd/middleman/notifications.go:29 and internal/server/huma_routes.go:1941

  • Periodic notification sync and global /sync notification refresh only log failures and do not update the notification sync status returned by /notifications. Users only see errors from the dedicated /notifications/sync endpoint, so background/global sync failures are not surfaced.
  • Fix: route all notification sync entry points through shared begin/finish status tracking, or move notification sync status into the syncer so ticker, global sync, and endpoint sync all update the same state.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (7bf157c)

Summary verdict: Two medium correctness issues remain; no critical or high-severity findings were reported.

Medium

• internal/db/queries_notifications.go:399
  Notification repo facets are keyed only by owner/name, which can merge notifications from different hosts, such as github.com/acme/widget and ghe.example.com/acme/widget, even though the feature is host-scoped elsewhere.
  Fix: Include platform_host in the facet key, or return structured repo facet objects with host, owner, and name.

• internal/github/client.go:234
  Live GitHub notification normalization never copies the notification participating flag, so real synced rows always report participating=false.
  Fix: Populate thread.Participating from the GitHub notification payload during normalization.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (af4b586)

Medium issue found: notification repo facets are not usable as returned for host-qualified repositories.

Medium

• internal/server/notifications.go:27
  The notifications API returns repo facet keys as platform_host/owner/name, but the repo query filter only accepts owner/name. This makes returned facet keys unusable as filters and also cannot distinguish repositories with the same owner/name across GitHub hosts, such as github.com/acme/widget and ghe.example.com/acme/widget.

  Suggested fix: Accept host-qualified repo filters like platform_host/owner/name and pass PlatformHost into ListNotificationsOpts.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (ab3af6d)

Changes are mostly sound, but two Medium issues should be addressed before merge.

Medium

• cmd/middleman/notifications.go:23
time.NewTicker(interval) will panic if notifications.sync_interval or notifications.propagation_interval is configured as 0s or a negative duration. Validation currently only checks parseability.
  Fix: Reject non-positive notification intervals during config validation.

• packages/ui/src/stores/notifications.svelte.ts:100
  The inbox store never sends a sort parameter and the UI exposes no sort control, so the default list uses backend updated_desc ordering instead of the intended actionable priority ordering for mentions/review requests.
  Fix: Add sort state/control and default it to priority, or make the API default sort match the intended inbox priority behavior.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (3895fb3)

Medium issues remain in the notification inbox changes.

Medium

• internal/db/queries_notifications.go:563 - MarkNotificationsRead updates by platform_thread_id only, but notification thread IDs are only unique together with platform_host. In multi-host setups, marking one GitHub thread read could also mark a GitHub Enterprise thread with the same ID read and clear its queued/error metadata. Include platform_host in the method contract and WHERE clause, or update by local notification ID.

• frontend/src/lib/stores/router.svelte.ts:105 - The /inbox route ignores notification filter query parameters, and InboxView does not write state/search filters back to the URL. Reloading or sharing an Inbox view resets to the default unread view, breaking triage filter routing. Parse/build state, reason, type, repo, q, and sort on the inbox route, and navigate with updated query params when filters change.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (72407ad)

Medium issues remain around notification read propagation and missing Inbox filters.

Medium

• internal/db/queries_notifications.go:723
  Read propagation success unconditionally sets unread = 0 by notification ID. If the worker selects a queued row, then a newer GitHub notification update syncs before success is recorded, the newer unread activity can be marked read locally.
  Fix: Record propagation success conditionally against the queued attempt, such as by passing the selected github_read_queued_at / github_updated_at and only clearing unread when the row still represents that queued state.

• packages/ui/src/views/InboxView.svelte:185
  The Inbox UI exposes state, search, and sort controls, but does not expose reason, type, or repository filters even though the API/store/router support them and the feature promises those filters.
  Fix: Add visible reason/type/repo controls wired through updateFilters, and cover them in the inbox e2e test.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (05c5408)

Notifications changes look mostly sound, but two medium-risk concurrency/lifecycle issues should be addressed before merge.

Medium

• internal/db/queries_notifications.go, MarkNotificationReadPropagationResult
  The failure path records retry metadata by id only, while the success path guards on the original github_read_queued_at and github_updated_at. If newer unread activity arrives after the worker dequeues the notification but before MarkThreadRead fails, the stale failure result can write github_read_error, attempt count, and next-attempt metadata onto the newer activity.
  Fix: Apply the same queuedAt and githubUpdatedAt guard to the failure update, and treat a non-matching row as a stale worker result.

• cmd/middleman/notifications.go:18
  Notification sync and propagation loops are launched as bare goroutines and are not tracked or waited on during shutdown. Context cancellation asks them to stop, but an in-flight DB or GitHub operation can continue while the main shutdown path proceeds to stop shared services.
  Fix: Register these loops with the existing lifecycle/wait-group pattern, or return a stop/wait handle from startNotificationLoops and wait for in-flight notification work before tearing down shared resources.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (60b5482)

Medium risk: one retry-state/UI mismatch should be fixed before merge.

Medium

• internal/github/notifications_sync.go:226, packages/ui/src/views/InboxView.svelte:139
  • Rows that hit max_attempts_exceeded are excluded from future propagation retries, but the worker still records a next-attempt timestamp and the UI says the GitHub update “will retry automatically.”
  • Fix: when the retry cap is reached, persist a terminal state without github_read_next_attempt_at, and render copy that says GitHub sync stopped after repeated failures.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (5f590c3)

The PR is mostly ready, but two Medium issues should be addressed before merge.

Medium

• packages/ui/src/stores/notifications.svelte.ts:203
pollNotificationsAfterSync can stop too early when syncStatus.running is still false immediately after triggering a background sync. The Inbox may remain stale after “Sync notifications.”
  Fix: Poll until the notification sync timestamp advances, an error appears, or a retry budget expires.

• packages/ui/src/views/InboxView.svelte:301
  Rows with notificationDestination === null still render as enabled buttons, but clicking silently does nothing. External-only notifications need a clear unavailable state.
  Fix: Render these rows disabled or otherwise non-actionable with visible or accessible “link unavailable” copy.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (53e3eab)

No Medium, High, or Critical findings were reported.

All review agents found the code clean or reported no actionable issues.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (fb8ee46)

Medium severity issues need attention before merge.

Medium

• cmd/middleman/notifications.go:48 - Notification sync uses context.WithTimeout(ctx, interval), so a slow initial or full sync is canceled exactly at the polling interval and may never reach the watermark update. On large inboxes or slow GitHub responses, each tick can repeat the same full sync indefinitely.

  • Fix: Use the parent cancellation context directly, or introduce a separate longer configurable timeout while skipping overlapping ticks.

• internal/github/client.go:268 - notificationItem only extracts PR numbers from /pulls/{number} URLs. GitHub notification payloads can expose PR subjects through issue-style /issues/{number} URLs, which means rows may be stored as item_type="pr" without item_number or an internal link, and closed-linked PR notifications may not be auto-completed.

  • Fix: When subjectType is PullRequest, accept both /pulls/{number} and /issues/{number} API URL shapes, then generate the PR web URL from the extracted number.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (2a85e14)

No Medium, High, or Critical issues were found.

All review agents reported no actionable findings.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (f6f44c9)

No Medium, High, or Critical issues found.

All reported findings were below the requested severity threshold or clean reviews.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (07786b1)

No Medium, High, or Critical findings were reported.

All reported issues were below the requested severity threshold, so there are no findings to include in the PR comment.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (65763ca)

Notification inbox is close, but two medium backend sync gaps should be fixed before merge.

Medium

• internal/github/notifications_sync.go:190 — Synced GitHub notifications never enrich ItemAuthor from local PR/issue data, so real synced rows can have an empty author even though the UI exposes author metadata and backend search includes item_author. Existing fixtures manually seed authors, which can hide the gap.

  • Fix: Enrich notification rows from synced PR/issue records during upsert/list response construction, or join local PR/issue tables when listing notifications.

• internal/github/sync.go:1229 — Closed-linked notification completion is only wired after full repo sync. Detail/list update paths that upsert a closed PR or issue can leave the linked notification active until another repo/notification sync happens.

  • Fix: Call MarkClosedLinkedNotificationsDone after individual PR and issue sync/update paths that persist closed or merged state, and cover those paths with full-stack tests.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (29b341c)

Review verdict: one medium-severity security issue should be addressed before merging.

Medium

Cloned private SQLite data may be created world-readable

File: scripts/dev-clone-db.sh:30-38

make dev-clone-db copies the user’s configured middleman.db into tmp/dev-db-clone/middleman.db, but the clone directory and SQLite file are created with default permissions. With a typical umask 022, this can create a 0755 directory and 0644 database, allowing another local user or process to read private repository metadata, notification subjects, issue/PR data, and other synced content.

Remediation: create the clone directory with owner-only permissions and force cloned DB/config files to 0600, for example by setting umask 077 before file creation and/or applying chmod 600 after writing. Consider also refusing symlinked clone paths or temp files before writing.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (b8675d7)

Medium-severity issues remain around notification read state consistency and inbox filtering.

Medium

• internal/db/queries_notifications.go:558
QueueNotificationIDsRead sets github_last_read_at when a read is only queued locally, before GitHub propagation succeeds. If the worker later fails or dead-letters, API/UI metadata still claims GitHub was read at that time.
  Fix: Leave github_last_read_at unchanged when queueing local reads; update it only after successful propagation or when a later GitHub sync reports the thread read.

• packages/ui/src/views/InboxView.svelte:223
  The Inbox type filter only exposes PRs and issues, even though the backend stores and returns release, commit, and other notification subjects. Those rows are visible in “All types” but cannot be filtered from the UI.
  Fix: Add release, commit, and other options, or derive type options from returned facets/static supported item types.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (a7e9157)

Medium-risk issues remain around notification state handling and GitHub retry behavior.

Medium

• internal/server/notifications.go:149 - markNotificationsUndone clears done_at even for notifications linked to PRs/issues that are already closed or merged, so those rows can reappear in the active inbox after using Undone. Re-run MarkClosedLinkedNotificationsDone after undone mutations, or avoid clearing done_at for rows whose linked item is closed/merged.

• internal/github/notifications_sync.go:263 - The read propagation worker treats all GitHub errors as per-row retry failures and continues the batch, so rate-limit or abuse-detection responses can burn attempts across many queued rows and eventually dead-letter them. Detect rate-limit/secondary-limit responses, pause processing for that host, and retry later without consuming attempts for every remaining row in the batch.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (b220d8a)

Notifications feature looks mostly clean, but there is one medium-severity retry/backoff issue to address.

Medium

• internal/github/notifications_sync.go:259
  Rate-limit errors during notification read propagation return immediately without recording retry/backoff metadata or a host throttle. The same due row remains queued with zero attempts and no visible row error, so the worker retries it on every propagation tick and the UI cannot show the promised retry state.
  Fix: Persist a next-attempt time or host-level throttle when rate-limited, and skip retries until that time without consuming normal per-row attempts.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (aad8f0d)

Summary verdict: One medium issue should be fixed before merge.

Medium

• internal/server/notifications.go:112 - Notification read/done mutations still queue GitHub read propagation when notifications.enabled = false, but the propagation worker and sync endpoint are disabled in that mode. This leaves rows permanently showing queued GitHub updates.
  • Fix: When notifications are disabled, either reject read propagation mutations or force local-only behavior by not setting github_read_queued_at and returning no queued IDs.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (191f2a5)

Medium issue found; no Critical or High findings.

Medium

• packages/ui/src/api/csrf.ts:15 - csrfFetch now decides whether to add Content-Type: application/json from the caller’s explicit headers instead of the constructed Request. This can overwrite automatically generated content types for bodies like FormData or URLSearchParams, including multipart boundaries, breaking non-JSON mutation requests.

  Suggested fix: Preserve request.headers.has("Content-Type") for body types that set their own content type, and only default to JSON for generated JSON or empty mutation cases.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (477f509)

No Medium, High, or Critical issues found.

All review agents reported no actionable findings.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (c085fc6)

Review verdict: one Medium issue should be addressed before merge.

Medium

Stale read propagation can hide newer GitHub notification activity

File: internal/github/notifications_sync.go:261
Related: internal/db/queries_notifications.go:236

ProcessQueuedNotificationReads reads queued rows, then later calls MarkNotificationThreadRead without revalidating that the queued row still represents the latest thread activity. If a maintainer marks an old notification done/read and someone comments or mentions them on the same thread before the worker runs, the stale worker call can mark the newer GitHub notification read.

On the next sync, UpsertNotifications only reactivates local done_at when GitHub reports unread=true, so the newer activity can remain locally done/read and hidden from the Inbox.

Suggested fix: track a propagation generation and revalidate before calling GitHub. Skip propagation if github_updated_at or github_read_queued_at changed, and ensure post-propagation syncs surface rows where GitHub reports unread=false but github_updated_at is newer than the propagated generation.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (3646faa)

Review verdict: one medium-severity issue should be fixed before merge.

Medium

• internal/db/queries_notifications.go, UpsertNotifications done_at update logic
  Locally done notifications can be reopened even when GitHub reports them as read. The done_at CASE clears local done state when excluded.unread = 0 if github_updated_at is newer than github_read_generation_at, which contradicts the intended rule that done rows should only re-enter the inbox on newer unread activity.

  Fix: Clear done_at / done_reason only when excluded.unread = 1 and excluded.github_updated_at > middleman_notifications.done_at; keep read-only updates from reopening locally completed rows.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (4e750fb)

Review incomplete: no code issues were reported, but one reviewer could not access the diff.

High

• /tmp/roborev-snapshot-3623591796/roborev-snapshot-content.diff: Gemini could not review the changes because the diff file was outside the allowed workspace directories. Move the diff into an accessible workspace path or adjust permissions, then rerun review.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (baa1936)

No Medium-or-higher code findings were identified.

The security review found no issues. One agent could not access its diff snapshot, but that is a review tooling problem rather than an actionable PR finding.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (c350254)

No Medium, High, or Critical findings were reported.

All agents agree the reviewed diff is clean.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (e5b19b7)

Notifications feature has two issues to address before merge: one High data consistency bug and one Medium empty-state API bug.

High

• internal/db/queries_notifications.go:238
UpsertNotifications unconditionally overwrites source_updated_at and unread from the incoming payload. An older sync payload can clear or roll back newer unread activity if notification sync overlaps with read propagation or a later refresh.
  Fix: Guard conflict updates by generation, such as ignoring payloads where excluded.source_updated_at < middleman_notification_items.source_updated_at, or use CASE guards per state field. Add a regression test for stale read-after-newer-unread behavior.

Medium

• internal/server/notifications.go:70
  When the tracked repo set is empty, the code returns a blank NotificationRepoFilter, but notificationWhere rejects blank platforms before it can convert that filter into 0 = 1. As a result, notification list and bulk endpoints return 500 instead of an empty inbox when notifications are enabled with no tracked repos.
  Fix: Represent an empty tracked set explicitly, or skip blank repo filters before platform validation so the query resolves to 0 = 1.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (bdae4bb)

Summary verdict: No Medium, High, or Critical findings were reported.

All agents found the reviewed changes clean.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (8130624)

No Medium, High, or Critical findings were reported.

All review agents that provided findings reported the code as clean.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (48260a8)

No Medium, High, or Critical findings were reported.

All provided reviews are clean.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (b4f93ce)

Medium severity issue found.

Medium

• scripts/dev-clone-db.sh:79 - clone_config.write_text(config_text) follows an existing symlink at tmp/dev-db-clone/config.toml. If an attacker pre-places that symlink in the worktree, running make dev-clone-db can overwrite the symlink target as the maintainer, potentially causing arbitrary file overwrite as that user.

  Remediation: write the config to a newly created temp file in clone_dir with safe permissions, then use os.replace(temp_config, clone_config) so an existing symlink is replaced rather than followed. Also reject non-regular existing clone files before writing.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (b4f93ce)

Medium severity finding needs attention before merge.

Medium

• scripts/dev-clone-db.sh:79
clone_config.write_text(config_text) follows an existing symlink at tmp/dev-db-clone/config.toml. If someone with write access to the worktree pre-creates that symlink to another file owned by the maintainer, running make dev-clone-db can overwrite the symlink target and chmod it.

  Suggested fix: write to a newly created temp file inside clone_dir with mode 0600, then replace with os.replace(temp_config, clone_config) so an existing symlink is replaced rather than followed. Also reject non-regular existing clone outputs before writing.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (1d6fe42)

No Medium, High, or Critical findings were reported; the reviewed code is clean.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (1d6fe42)

Summary verdict: No Medium, High, or Critical findings were reported.

All review agents found no actionable issues for this PR.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Pass

No issues found.

---

Review type: | Agent: codex | Job: 19261