Skip to content

fix(activity): keep PR timelines and recency authoritative - #913

Open
wesm wants to merge 18 commits into
mainfrom
activity-sync-still-broken
Open

fix(activity): keep PR timelines and recency authoritative#913
wesm wants to merge 18 commits into
mainfrom
activity-sync-still-broken

Conversation

@wesm

@wesm wesm commented Aug 14, 2026

Copy link
Copy Markdown
Member

Activity could show an old parent time while detail already contained newer activity because filter state and feed recency were coupled.

  • Treat Commits as a top-level default-branch filter; PR commits remain in PR timelines.
  • Carry each PR or issue authoritative last activity through Activity and use it for desktop and mobile grouping and ordering.
  • Reconcile visible PR, issue, and Activity lists after every successful initial detail read, even if selection changes before display.
  • Keep periodic full Activity snapshots authoritative for cursor-hidden events.

Activity's cursor poll can miss events persisted behind newer leading rows. A
successful detail read already contains those events, but visible Activity,
Pulls, and Issues can continue to show an older last-change time.

Reconcile visible lists after the initial detail envelope is accepted. Make
the scheduled full Activity poll authoritative so the feed also self-heals
without detail navigation or server-sent events.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

Activity after reconciliation: the synthetic fixture-bot event was persisted behind the previous feed cursor, and the PR row now shows it at just now.

activity-recency-consistency.png

@roborev-ci

roborev-ci Bot commented Aug 14, 2026

Copy link
Copy Markdown

roborev: Combined Review (b53703f)

Changes need a medium-severity fix for initial-read reconciliation during selection changes.

Medium

  • frontend/src/lib/stores/detail.svelte.ts:982 and frontend/src/lib/stores/issues.svelte.ts:867 — Initial-read reconciliation runs only after generation and installation checks. If the user closes or changes the selection while a successful GET completes, the response is discarded without refreshing the visible list or Activity, leaving newly persisted behind-cursor events hidden. Trigger reconcileListsAfterDetailSync() immediately after a successful detail read, before generation-gated installation, and add coverage for closing or changing selection during the initial GET.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 2m32s

The Commits control is a default-branch filter, but Activity also removed PR
commits from its requests. Threaded rows then derived their age from the
remaining visible children, so Activity could contradict an already-synced PR
or issue detail.

Keep PR commits in item-scoped Activity results and carry each parent's true
last-activity time through the feed. Successful initial detail reads also
invalidate visible lists even when the selection changes before installation.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm wesm changed the title fix(activity): keep list recency consistent after detail reads fix(activity): keep PR timelines and recency authoritative Aug 14, 2026
@roborev-ci

roborev-ci Bot commented Aug 14, 2026

Copy link
Copy Markdown

roborev: Combined Review (508de0e)

The changes are generally sound, but two medium-severity gaps remain in filter persistence and full-stack coverage.

Medium

  • frontend/src/lib/stores/activity.svelte.ts:56,705 — Filter encoding is not round-trippable. Deselecting all event types writes types=commit,notification, but reload normalization changes it to none,notification, unexpectedly hiding item threads. The default-branch Commits toggle state is similarly lost. Encode item scope and the default-branch commit toggle independently, and test hydration with a fresh store instance after URL normalization.

  • frontend/tests/e2e-full/activity-live-refresh.spec.ts:142 — Full-stack coverage is incomplete. Initial-read reconciliation is tested only for pull requests; the issue path and authoritative parent-recency ordering are covered only by unit/API tests. Add HTTP-and-SQLite end-to-end cases for an issue initial read and for a filtered newer event changing parent ordering and timestamp.


Reviewers: 2 done | Synthesis: codex, 20s | Total: 10m26s

Activity rebuilt its filter controls from the API type list. That list must
retain pull request commits and can omit hidden default-branch rows, so reloads
could change the item scope or the Commits toggle.

Persist item and event selections independently. Keep filtered child events
authoritative for parent recency and cover both pull request and issue detail
reads through the real HTTP and SQLite path.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

Addressed both medium findings in a0954c0.

  • Activity URLs now persist item scope (item_types) and event toggles (event_types) independently. The backend types list is derived request state only. Fresh-store tests cover empty event selection, independent issue scope and Commits state, and hidden default-branch rows.
  • Added real HTTP-and-SQLite browser cases for issue initial-read reconciliation and for a filtered newer child event advancing its parent timestamp and ordering.
  • Updated the failing Activity filter end-to-end case to use the canonical URL state and to assert the product contract: PR timeline commits stay visible; the Commits toggle controls only top-level default-branch commits.

Verification includes 3,498 frontend unit tests, 45 affected full-stack browser tests, 25 affected mock browser tests, frontend type/effect checks, Go tests, formatting, and lint.

@roborev-ci

roborev-ci Bot commented Aug 14, 2026

Copy link
Copy Markdown

roborev: Combined Review (a0954c0)

Review verdict: Three medium-severity issues require attention.

Medium

  • frontend/src/lib/stores/activity.svelte.ts:742 — Existing Activity bookmarks and session routes using types are silently reset to defaults because hydration ignores types, then syncToURL deletes it. Migrate the legacy types value into item/event selections when the new parameters are absent, with coverage for bookmarked and session-restored routes.

  • frontend/src/lib/stores/activity.svelte.ts:58 — The empty-event special case depends on notifications being enabled and both item types being selected. Disabling Notifications or one item type can therefore expose unrelated new_pr/new_issue rows. Make opening-row inclusion independent of notification and item-scope state, and test these transitions.

  • internal/db/queries_activity.go:99 — Parent recency is returned but does not determine eligibility or the safety-cap window, which remain based on created_at. A parent opened before the selected range can be omitted despite recent matching activity, leaving the frontend without an anchor row to display or reorder. Select parent anchors using authoritative recency before applying the time window and cap, or return a separate parent-summary projection; cover a parent created before the range with a hidden child event inside it.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 7m13s

Activity could drop a pull request or issue from its time window when the
only recent child event was hidden by a filter. Event selection then decided
the parent's timestamp and position even though detail had newer provider
state.

Return a separate parent snapshot ordered by last activity, and merge it into
threaded views without inventing timeline events. Migrate legacy route filters
into independent item and event selections so existing bookmarks keep their
meaning.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

Addressed all three findings in 4ac670f:

  • Legacy Activity types routes now migrate into the canonical item and event selections when those parameters are absent. Bookmark normalization and session-route restoration are covered.
  • Opening rows are included only when timeline event types are selected, independent of notification and item-scope toggles. Pull request timeline commits remain independent of the top-level Commits toggle.
  • Activity now returns a separate authoritative parent snapshot. Parent eligibility, ordering, time-window filtering, and the safety cap use last activity rather than creation time. Desktop and mobile threaded views merge that snapshot without inventing visible events.

Coverage includes database and API tests plus full-stack HTTP-and-SQLite verification for an old parent promoted by a newer hidden child event. The full pre-commit gate passed, including generated API checks, frontend checks, Go short tests, and lint.

@roborev-ci

roborev-ci Bot commented Aug 14, 2026

Copy link
Copy Markdown

roborev: Combined Review (4ac670f)

Review found one medium-severity issue; no high or critical findings.

Medium

  • internal/server/huma_routes.go:1460 — Pull-request subjects use the wire type "pr", while workspace snapshots are keyed by "pull_request". Direct lookup prevents authoritative PR parent rows from receiving their workspace reference.
    • Fix: Convert the subject item type with workspaceItemTypeFromActivity before lookup, and add coverage for a PR subject with a workspace.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 9m13s

The authoritative parent snapshot emits Activity wire item types, while
workspace references use canonical workspace item types. Pull request parents
therefore lost their workspace association when the response was built.

Normalize only the snapshot lookup key and keep the wire response unchanged.
Treat every matching parent as valid in the mobile repository filter check.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

Addressed the remaining review finding in 625c762.

  • Parent-summary PR item types now normalize from the Activity wire type "pr" to the workspace type "pull_request" before snapshot lookup. The response wire type stays "pr".
  • Added HTTP-and-SQLite coverage proving that a filtered authoritative PR parent retains its workspace reference.
  • Updated the linked mobile e2e assertion to verify that every returned parent matches the selected repository without assuming the authoritative snapshot contains only one parent.

Verification: the full mobile Chromium file passed 17/17, the focused case passed in Firefox, focused server tests passed, and the final commit and push hooks passed. The first commit-hook run encountered an unrelated tmux cleanup timing failure under host load; that test passed in isolation and the complete hook passed on retry.

@roborev-ci

roborev-ci Bot commented Aug 14, 2026

Copy link
Copy Markdown

roborev: Combined Review (625c762)

Review found three medium-severity issues affecting legacy filter restoration, commit-toggle migration, and activity counts.

Medium

  • frontend/src/lib/stores/router.svelte.ts:567 — An explicit legacy types bookmark can be overridden by canonical item_types and event_types restored from session storage. Hydration prioritizes the restored canonical values, causing the bookmark’s filters to be ignored. Treat types as satisfying both canonical filter dimensions during restoration, and add a router/store integration test covering a canonical stored route with an explicit legacy URL.

  • frontend/src/lib/stores/activity.svelte.ts:135 — Legacy URLs using hide_branch=1 with Commits enabled contain commit but omit default_branch_commit. Migration infers the Commits toggle solely from default_branch_commit, incorrectly disabling it. During legacy migration, account for hide_branch and use commit as the toggle signal when branch activity was hidden.

  • frontend/src/lib/components/layout/StatusBar.svelte:134 — Threaded and mobile views render parent-only subjects from item_activity, but status-bar counts inspect only event items. With filtered events, the feed can display open threads while reporting zero or substantially fewer pull requests and issues. Include authoritative parent subjects in the counts, deduplicate them against event- and workspace-backed subjects, and test a parent-only response.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 7m15s

Explicit legacy Activity bookmarks could inherit canonical session filters,
and hidden-branch URLs could silently disable Commits during normalization.
Filtered feeds could also show parent-only threads while the status bar
reported no pull requests or issues.

Keep legacy route ownership intact during migration. Derive Activity totals
from one deduplicated subject projection, with authoritative parent lifecycle
state taking precedence over event and workspace snapshots.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

Addressed all three findings in 18f81f065.

  • An explicit legacy types URL now owns both item and event filter dimensions, so canonical session filters cannot override the bookmark during router/store hydration.
  • Legacy hide_branch=1 URLs now use commit to preserve the Commits toggle when the intentionally hidden default_branch_commit marker is absent.
  • Activity status counts now merge event, authoritative parent, and workspace subjects by full item identity. Parent lifecycle state wins, and duplicate representations count once.

Validation includes the three focused regression files (95 tests), the full frontend unit suite (3,515 passed, 1 skipped), frontend formatting/lint/Svelte/Effect checks, and the Chromium app-shell count smoke test.

@roborev-ci

roborev-ci Bot commented Aug 14, 2026

Copy link
Copy Markdown

roborev: Combined Review (18f81f0)

The PR needs changes for two medium-severity regressions; no material security issues were found.

Medium

  • Legacy activity URL normalization regressionfrontend/src/lib/stores/activity.svelte.ts:138
    Legacy URLs with a stale default_branch_commit but no commit now re-enable the Commits toggle, reversing previous bookmark-normalization behavior. Treat the legacy commit token as authoritative for the old toggle and restore coverage for stale default_branch_commit URLs.

  • Cap state conflates events and parent subjectsinternal/server/huma_routes.go:1381
    capped now represents both capped events and capped parent subjects. Incremental polling interprets either as event overflow, causing unnecessary full reloads on every poll; flat mode may also claim 5,000 events were capped when only parent subjects exceeded the limit. Expose separate event and parent-snapshot cap indicators, and use only the event cap for polling replacement and the existing warning.


Reviewers: 2 done | Synthesis: codex, 13s | Total: 8m49s

Activity bookmarks could re-enable default-branch commits from a stale token,
and parent snapshot overflow was being treated as event overflow.

Keep the legacy commit token authoritative. Report parent truncation separately
so polling and event-count warnings respond only to event overflow.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

Addressed both findings in 6e96b13ff:

  • Legacy bookmark migration now requires the old commit token before enabling the Commits toggle, so a stale default_branch_commit cannot revive it.
  • /activity keeps capped event-only and exposes parent truncation separately as item_activity_capped; incremental polling and the existing 5,000-event warning continue to use only capped.
  • Regression coverage includes the stale hidden-branch bookmark and a real HTTP/SQLite response with 5,001 parent subjects and no matching events.

Validation: 3,516 frontend unit tests passed; the full server package, frontend package checks, API generation, and commit/push hooks passed.

@roborev-ci

roborev-ci Bot commented Aug 14, 2026

Copy link
Copy Markdown

roborev: Combined Review (6e96b13)

Review found one medium-severity filtering regression; no security issues were identified.

Medium

  • frontend/src/lib/stores/activity.svelte.ts:59 — Selecting only the Commits toggle also enables new_pr and new_issue because opening events are added whenever enabledEvents.size > 0. Since Commits now controls only default-branch commits, commit-only filtering unexpectedly shows opening rows.
    • Fix: Add opening events only when a timeline-controlled type—such as comment, review, or force-push—is selected, and add end-to-end coverage for commit-only filtering.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 10m26s

The Commits toggle represents default-branch activity, but selecting it alone
also requested pull request and issue opening rows. A commit-only feed therefore
displayed unrelated item activity.

Require a timeline-controlled event before requesting opening rows. Keep pull
request timeline commits independent from the top-level Commits toggle.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

Addressed in 814c6f8af.

  • Opening rows now require a timeline-controlled event: Comments, Reviews, or Force pushes. Commits alone requests top-level default-branch commits plus mandatory pull request timeline commits, without new_pr or new_issue rows.
  • Added store-level and Playwright regression coverage for selecting only Commits.

Validation: Activity store tests passed (73/73), the complete default-branch Activity Playwright suite passed (17 passed, 1 existing Firefox mobile skip), and make frontend-check-no-deps passed.

@roborev-ci

roborev-ci Bot commented Aug 14, 2026

Copy link
Copy Markdown

roborev: Combined Review (814c6f8)

The change is generally sound, but one medium-severity truncation issue can silently undercount activity.

Medium

  • frontend/src/lib/stores/activity.svelte.ts:531item_activity_capped is discarded in every projection, causing feeds and status totals to silently omit parent items beyond the 5,000-subject cap.
    • Fix: Track parent truncation separately from event capped, and display a distinct notice that does not trigger event-reload behavior.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 10m12s

Activity can return a complete event page with a capped parent snapshot. The
frontend dropped that second cap flag, so feed and status totals appeared
authoritative while omitting items.

Preserve parent truncation through every projection and warn separately. Only
event overflow can trigger event replacement.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

Addressed in b35a7d7f6.

  • The Activity store now preserves item_activity_capped across foreground loads, reconciliation, scheduled snapshots, replacement polls, and incremental polls.
  • Desktop and mobile feeds show a distinct parent-truncation notice. Only event capped can trigger replacement polling or the event-overflow warning.
  • Added store coverage proving parent truncation does not reload events, component coverage for both views, and Playwright coverage for the separate notice.

Validation: full frontend unit suite passed (3,521 passed, 1 existing skip), the complete default-branch Activity Playwright suite passed (19 passed, 1 existing Firefox mobile skip), and make frontend-check-no-deps passed.

@roborev-ci

roborev-ci Bot commented Aug 14, 2026

Copy link
Copy Markdown

roborev: Combined Review (b35a7d7)

The PR needs one medium-severity compatibility fix before approval.

Medium

  • frontend/src/lib/stores/activity.svelte.ts:130 — Legacy types=commit and types=commit,notification bookmarks no longer preserve their previous commit-filter behavior. The former disables Commits unless default_branch_commit is also present, while the latter is reinterpreted as default item scope with no event toggles.
    • Fix: Treat commit as enabling the legacy Commits toggle, and reserve only types=notification for the historical empty-event/default-item encoding. Ignore a stale default_branch_commit when commit is absent.

Reviewers: 2 done | Synthesis: codex, 15s | Total: 7m13s

Legacy Activity URLs still exist in bookmarks and session-restored routes.
Treat their commit token as the old Commits toggle so normalization does not
silently disable default-branch commits.

Keep the notification-only sentinel distinct, and ignore obsolete
default-branch tokens without commit.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

Fixed in 590a8b3e5.

Legacy Activity URL migration now treats commit as the authoritative old Commits toggle. Only types=notification retains the historical default-item/no-event sentinel; types=commit and types=commit,notification normalize to event_types=commit, while stale default_branch_commit without commit stays disabled.

Regression coverage includes store hydration with a fresh instance and Chromium browser loading of the legacy bookmark. Verification: 3,522 frontend unit tests passed (1 skipped), 11 default-branch Activity Playwright tests passed, and make frontend-check-no-deps passed.

@roborev-ci

roborev-ci Bot commented Aug 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (590a8b3)

Overall verdict: Two medium-severity issues require follow-up; no security vulnerabilities were identified.

Medium

  • Unstable repository identity can corrupt activity groupingfrontend/src/lib/stores/activity.svelte.ts:682
    Incremental polls replace parent snapshots while retaining older event rows, but consumers group them by mutable repo_path. After a repository rename, this can produce duplicate threads/counts; after route reuse, it can temporarily combine events from the old repository with a parent from the new repository. Preserve platform_repo_id or another stable identifier in Activity responses and use it for grouping and deduplication. Add full-stack coverage for repository renames and route reuse.

  • Legacy bookmark migration lacks full-stack regression coveragefrontend/src/lib/stores/activity.svelte.ts:115
    The browser test stubs /api/v1/activity, so it does not verify that a legacy URL normalized to types=commit,notification behaves correctly through the real HTTP API and SQLite. Add an e2e-full test that loads the legacy URL and verifies normalization and default-branch commit visibility using seeded data.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 12m19s

Incremental Activity polling keeps old event rows while replacing parent
snapshots. Route-based grouping can split one repository after a rename or
combine two repositories when a route is reused.

Carry provider-stable repository identity through every Activity projection
and use it for threaded, mobile, and status grouping. Keep the existing route
key for legacy payloads without a stable ID.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

Addressed both findings in 21c809568.

  • Activity responses now preserve platform_repo_id for event rows, authoritative parent snapshots, and workspace subjects. Threaded, mobile, flat-row, and status grouping use that stable identity, with the established route key retained only for legacy payloads that lack it. A full-stack regression keeps old events across the real incremental poll, renames the repository, reuses its old route, and verifies that the threads stay correct and distinct.
  • Added a real HTTP-and-SQLite Playwright regression for types=commit,notification. It verifies canonical URL normalization, default-branch commit visibility, and exclusion of pull request and issue opening rows.

Verified in Chromium and Firefox, plus focused frontend tests, Go DB/server tests, API generation, and the full pre-commit hook suite.

@roborev-ci

roborev-ci Bot commented Aug 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (21c8095)

High-severity stale-route navigation and a medium-severity mobile repository-label ambiguity remain.

High

  • frontend/src/lib/stores/activity.svelte.ts:516, frontend/src/lib/components/ActivityThreaded.svelte:524 — Incremental polls update authoritative parent subjects but leave retained event rows with stale mutable repository routes. After a rename and route reuse, flat rows and threaded event/header clicks can navigate to the replacement repository’s same-numbered item.
    • Fix: Reconcile retained PR/issue events with parent subjects using stable repository and item identities, updating route, state, title, URL, author, and workspace data before projection. Add rename/reuse navigation coverage.

Medium

  • frontend/src/lib/views/MobileActivityView.svelte:317 — The mobile repository-label formatter excludes visibleItemActivity, so parent-only cards cannot disambiguate repositories sharing a name, route, host, or provider combination.
    • Fix: Include parent-subject repository identities in the formatter input, as ActivityThreaded does.

Reviewers: 2 done | Synthesis: codex, 20s | Total: 10m34s

Incremental Activity polling keeps old event rows while replacing parent
snapshots. Route-based grouping can split one repository after a rename or
combine two repositories when a route is reused.

Carry provider-stable repository identity through every Activity projection
and use it for threaded, mobile, and status grouping. Keep the existing route
key for legacy payloads without a stable ID.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
fix(activity): reconcile retained events with parent snapshots

Incremental polls keep earlier event rows while refreshing authoritative
parent snapshots. After a repository rename, those rows could still point at
an old route that a different repository later reused. A click could then open
the wrong pull request or issue.

Reconcile retained rows by provider-stable repository and item identity before
downstream projections. This keeps parent metadata and mobile labels current
without merging route-reused histories.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

Addressed both findings in 50e10b6.

  • Incremental Activity polls now reconcile retained PR/issue events from authoritative parent subjects using stable repository and item identity. Current route, title, lifecycle state, item URL, item author, and workspace metadata are projected before any view consumes the rows, while event authors and notification read state stay event-owned.
  • Mobile repository-label disambiguation now includes parent-only subjects.

Coverage now clicks both the renamed original and the replacement that reused its old route through real HTTP and SQLite data and verifies each destination. Focused store/mobile suites pass (97 tests), the Chromium full-stack scenario passes, and make frontend-check-no-deps passes.

@roborev-ci

roborev-ci Bot commented Aug 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (50e10b6)

The change is generally sound, but “Hide bots” is bypassed for authoritative parent snapshots.

Medium

  • Bot-authored parents remain visible and counted when “Hide bots” is enabled
    Locations: frontend/src/lib/components/ActivityFeed.svelte:321, frontend/src/lib/views/MobileActivityView.svelte:144, frontend/src/lib/components/layout/StatusBar.svelte:162
    Bot events are removed from displayItems, but their PR/issue parent summaries can still appear as empty threaded or mobile rows and remain included in status totals. Apply the bot-author predicate to parent and workspace subjects before rendering or counting them, and add regression coverage for bot-authored parents in threaded and mobile views.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 12m11s

Hide bots filtered event actors, but authoritative parent and workspace
snapshots bypassed that filter. Bot-authored pull requests and issues could
still appear as empty threads and inflate Activity totals.

Apply the same visibility rule to every Activity projection. Keep source
snapshots intact so toggling the preference does not discard authoritative
state.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

Addressed in 38db8c8.

  • Hide bots now applies to authoritative parent and workspace subjects before threaded/mobile rendering and status counting. Event rows still test the event actor, while parent/workspace summaries test item_author.
  • Raw authoritative snapshots remain intact, so toggling the preference only changes presentation and totals.
  • Regression coverage includes bot-authored parent-only and workspace-only subjects in threaded Activity, mobile Activity, and the status bar.

Verified with 63 focused component tests, Svelte analysis, make frontend-check-no-deps, commit hooks, and push hooks.

@roborev-ci

roborev-ci Bot commented Aug 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (38db8c8)

Code changes are directionally sound, but one high-severity test gap and one medium-severity incremental-search bug remain.

High

  • frontend/src/lib/components/ActivityFeed.svelte:318 — The bot-filter fix for authoritative parent/workspace subjects, mobile cards, and status totals lacks the repository-required HTTP-and-SQLite end-to-end regression coverage. Existing full-stack coverage tests only flat event rows.
    • Fix: Add an e2e case with bot-authored parent-only and workspace-only subjects, then verify threaded Activity, mobile Activity, and status totals after enabling Hide bots.

Medium

  • internal/db/queries_activity.go:439 — Parent snapshots apply search only to the parent title and author. If a search matches an event body or actor, incremental polling can omit its parent, preventing retained events from reconciling after title/state changes or repository renames. During route reuse, a stale row may consequently open the replacement repository until the next full refresh.
    • Fix: During incremental polling, include authoritative parents for search-matched provider events using the full non-cursor search result, as already done for workspace subjects.

Reviewers: 2 done | Synthesis: codex, 13s | Total: 11m4s

An incremental Activity search can keep a matching event from an earlier page
while the cursor-scoped response omits that event. Parent selection searched
only titles and authors, so it could omit the authoritative parent and leave
retained metadata or routes stale.

Use the full search result to retain matching parent identities. Cover bot
filtering against parent-only and workspace-only subjects through the real
HTTP and SQLite path.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

Addressed both findings in 6f3a26c.

  • Incremental search now reuses the full non-cursor provider-event matches to include authoritative parents selected by event body or actor.
  • Full-stack HTTP and SQLite coverage now proves Hide bots removes bot-authored parent-only and workspace-only subjects from threaded Activity, mobile Activity, and status totals. The affected spec passes in Chromium and Firefox.

@roborev-ci

roborev-ci Bot commented Aug 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (6f3a26c)

One medium-severity reconciliation issue remains; no security vulnerabilities were identified.

Medium

  • frontend/src/lib/stores/activity.svelte.ts:545 — Parent reconciliation updates item_url and repository metadata but leaves activity_url unchanged. Because flat-feed provider links prefer the cached activity_url, a repository rename followed by route reuse can direct notification links to the replacement repository.
    • Fix: Replace or clear activity_url using the authoritative parent URL for reconciled notification rows, and add a regression test covering the notification provider link after rename/route reuse.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 13m48s

An incremental Activity search can keep a matching event from an earlier page
while the cursor-scoped response omits that event. Parent selection searched
only titles and authors, so it could omit the authoritative parent and leave
retained metadata or routes stale.

Use the full search result to retain matching parent identities. Cover bot
filtering against parent-only and workspace-only subjects through the real
HTTP and SQLite path.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
fix(activity): reconcile notification provider links

Incremental Activity polling refreshed authoritative parent routes but left a
notification's cached provider URL unchanged. After a repository rename and
route reuse, the flat feed could open the replacement repository's item.

Notification URLs are parent links, so refresh them from the current parent
snapshot while preserving event-specific deep links.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@wesm

wesm commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

Addressed in 644fdd06a.

  • Reconciled notification rows now replace their cached provider URL with the authoritative parent URL. Event-specific deep links remain event-owned.
  • Added store coverage plus an HTTP/SQLite Playwright regression that renames a repository, reuses its former route, switches the retained feed to flat mode, and verifies the notification provider button opens the renamed parent.
  • Updated the Activity reconciliation invariant to make notification provider-target ownership explicit.

The linked CI failure was a five-second timeout in TestSSHFleetWebSocketTerminalClaimsSharedResizeOwnership; this branch does not change Fleet code. The same test with CI's shuffle seed passed locally in 0.803s. This push starts a fresh CI run.

Local verification:

  • Frontend Node/jsdom: 3,528 passed, 1 skipped.
  • Frontend browser: 229 passed.
  • Full-stack Activity spec: 30 passed across Chromium and Firefox.
  • Frontend format, lint, type, Svelte, and Effect checks passed.
  • cmd/e2e-server, golangci-lint, pre-commit, and pre-push hooks passed.

@roborev-ci

roborev-ci Bot commented Aug 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (644fdd0)

The change is generally sound, but one medium-severity stale-notification metadata issue remains.

Medium

  • frontend/src/lib/stores/activity.svelte.ts:542 — Notification metadata is reconciled only when its parent survives the independently capped 5,000-item item_activity snapshot. A visible notification whose parent falls outside that window can retain a stale activity_url, title, and item URL after a repository rename; with route reuse, the provider button may open the replacement repository.

    Suggested fix: Populate notification parent metadata directly from the joined pull request or issue in ListActivity, or guarantee that parents of returned events are included in a bounded reconciliation set regardless of the parent-summary cap.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 21m44s

Notification rows in the Activity union carried the title and URL that
were persisted when the notification synced. After a repository rename,
route reuse, or title edit those values were stale, and the feed only
repaired them when the frontend matched the row against the item_activity
parent snapshot, which is independently capped. A visible notification
whose parent fell outside that window kept a stale item_url, activity_url,
and title, so its provider link could open the old or replacement route.

ListActivity already LEFT JOINs the linked pull request or issue for
author, lifecycle state, and recency. Take item_title, item_url, and
activity_url from that same join, falling back to the persisted
subject_title and web_url only when the subject has not synced yet.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Aug 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (b322d20)

No Medium, High, or Critical findings were identified.


Reviewers: 2 done | Synthesis: codex, 6s | Total: 8m25s

Activity used each pull request's and issue's last_activity_at as its
authoritative recency, and that column is the provider's updated_at.
GitHub bumps updated_at for bookkeeping that is not activity: mergeability
recomputation after a base-branch push, head-branch deletion after merge,
and similar. A stacked pull request therefore rose to the top of the feed
and read "16 minutes ago" while its newest visible timeline entry was a
comment from fourteen hours earlier, and merged pull requests reappeared
as recent days after merging.

Define Activity recency from the ledger the feed can render instead: the
opening, the newest issue_comment/review/commit/force_push event for pull
requests (issue_comment for issues), merge, and close. Ledger rows the feed
never renders, such as cross references and assignments, do not count, and
provider updated_at no longer participates. The same expression drives the
parent snapshot's time window and ordering and each event row's
item_last_activity_at, so events and parents agree, and event visibility
filters still cannot hide the responsible event.

The event page is materialized before the recency lookup so the correlated
ledger walk runs once per returned row rather than once per candidate row;
on a real database this adds roughly 20 ms per poll.

The e2e fixture endpoint now reports the ledger comment time and bumps
provider updated_at further ahead so the scenario proves the ledger wins.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Aug 15, 2026

Copy link
Copy Markdown

roborev: Combined Review (3757aeb)

One medium-severity issue remains: the Author picker can omit authors of older items with recent close or merge activity.

Medium

  • internal/db/queries_activity.go:629 — Parent snapshots include old PRs and issues when merged_at or closed_at is recent, but author candidates still filter by opening/event created_at. As a result, authors of these visible parents are missing from the Author picker.
    • Fix: Use the same prActivityAtExpr and issueActivityAtExpr expressions as ListActivitySubjects, and add coverage for an old item that was recently closed or merged.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 12m16s

Issue opening rows were requested whenever any timeline event toggle was
on, including Reviews and Force pushes, which cannot occur on an issue.
Turning Force pushes off could therefore make every issue "opened" row
disappear. Opening rows now follow only the timeline events that exist for
that item kind: comments, reviews, and force pushes for pull requests, and
comments for issues.

Authoritative parent summaries also replaced a thread's author with an
empty string when the parent had no recorded author, dropping the event
actor the thread previously showed. Keep the existing author when the
summary has none.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant