Skip to content

fix(workflows): page batched definition reads - #325

Open
matt2e wants to merge 4 commits into
mainfrom
workflows-api-usage
Open

matt2e wants to merge 4 commits into
mainfrom
workflows-api-usage

Conversation

@matt2e

@matt2e matt2e commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Issue:

  • A channel with 101 workflows showed only 100 in the Workflows overview because discovery stopped after the first page.
  • Navigating away and back repeated the same relay reads because completed results were not retained.

Summary:

  • Batch joined workflow channel definition reads into a single relay filter per 128-channel batch.
  • Page full aggregate definition reads with exact relay cursors while keeping single-channel detail reads bounded.
  • Document the updated read behavior and add coverage for pagination, retries, access loss, and broker integration.

Bring over the combined-channel batching from 26935cb without its session overview cache or pagination. Read up to 128 joined channels in one kind-30620 filter with a shared 100-event limit, and mark every channel in a saturated batch as partial.

Cover shared-limit boundaries, deduplication, partial-result UI, and the real reader/authenticated broker path. Validation: 76 focused tests and TypeScript passed; live-relay and packaged-native acceptance remain untested.

Signed-off-by: Matt Toohey <contact@matttoohey.com>
Bring over pagination from 26935cb without the session overview cache. Continue full 100-event batches with exact timestamp and event-ID cursors, reset cursors between batches and retries, and retain bounded single-channel detail reads.

Preserve deduplication, access checks, cancellation, and per-request deadlines. Cover same-second boundaries, failed and non-advancing pages, lifecycle interruptions, landing queue behavior, and signed pagination through the real reader and authenticated broker.

Validation: 95 focused tests and TypeScript passed; live-relay, browser, and packaged-native acceptance remain untested.
Signed-off-by: Matt Toohey <contact@matttoohey.com>
@matt2e
matt2e requested review from a team, comp615 and wesbillman as code owners September 28, 2026 00:53

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Star Lord’s automated source review via Wes’s account.

No actionable findings in this revision. Reviewed the eight-file change and the supported landing/session/reader/broker paths: one bounded multi-channel filter, exact same-timestamp cursor advancement, coordinate folding, retry/refresh reset, per-page deadlines, and cancellation/access-loss handling. The regression additions use the appropriate capability, mounted-component, and broker layers. I also checked the incoming target-branch changes and the relay source’s multi-channel/cursor contract.

Reviewed head: a3b7e1cac98bae58630bef7d67639cd30949398e
Target base: 59d9d6fb9e22771ad9d380900d4c9d7fe152219c
Diff merge base: 3e0a4087b9c09df911f080a3c3a3534d5d7bffab

Validation limits: Source inspection only, using hash-verified pinned files; no PR code, tests, app, or live workflow execution. The 2026-09-28 01:00 UTC hosted-check snapshot had JavaScript, DCO, security checks, browser measurements, and one Chromium journey shard passing; Rust/tool integration and five browser journey shards were still running, with Windows validation skipped. Runtime/native acceptance and human testing are unverified. This COMMENT is not an approval or merge-readiness attestation.

Retain completed workflow definition batch reads inside the session capability so ordinary landing navigation can reuse fresh batches and warm stale cards while refreshing.

Keep explicit refreshes, save readback, configuration invalidations, disconnects, access clear and disposal on the relay path, and cover cache reuse, staleness and landing remount behavior.

Signed-off-by: Matt Toohey <contact@matttoohey.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Star Lord’s automated source review via Wes’s account — follow-up review.

No actionable findings in the session-cache follow-up from a3b7e1cac98bae58630bef7d67639cd30949398e to the head below. Source-review assessment: 9/10 for minimalness, elegance, and correctness within this follow-up’s scope; this is not approval or runtime validation.

Reviewed head: 239c501dc34a421fc817e98d86a6461454d235b8
Pinned base: 59d9d6fb9e22771ad9d380900d4c9d7fe152219c
Merge base: 3e0a4087b9c09df911f080a3c3a3534d5d7bffab

Checked the seven-file follow-up, session-owned cache freshness/retention and invalidation, landing remount/refresh/retry behavior, save-readback callers, access-loss/disconnect/disposal integration, and retained pagination coverage. The incoming base changes were inspected for overlap. The existing reader/outbox ownership and single-channel read limits remain unchanged.

Validation limits: source-only, using 27 hash-verified app blobs; no PR code, tests, builds, app, or live workflow operations were executed. One hosted-check snapshot at this head showed JavaScript, browser measurements, DCO, and security checks passing; Rust/tool integration and all six browser-journey shards were still running, and Windows validation was skipped. Native/live behavior, cache performance in the app, and human acceptance remain unverified.

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 One cache-consistency finding; see inline comment.

if (cached) cached.lastUsedAt = now;
return cached;
}
function retainDefinitions(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 [P2] Invalidate overlapping batches after save readback

If the landing’s [a,b] read finishes before a newly saved workflow is visible, it retains the pre-save inventory under a:b. The verified save readback then refreshes [a] (WorkflowLanding.tsx:367), but retaining that result under a leaves a:b fresh. Navigating away and remounting All workflows within ten seconds therefore reuses the old batch without a read, making the just-saved workflow disappear from the landing. A mounted regression probe observed reads after remount: 2, saved visible: false after the card had been visible before remount. Manual Refresh bypasses reuse; expiry alone does not establish an automatic refresh.

Evict overlapping channel-set entries when retaining newer definitions, or invalidate them on save readback, and add a regression where the initial batch does not yet contain the saved revision. This preserves the save-readback bypass/purge contract in docs/workflows.md:87.

Signed-off-by: Matt Toohey <contact@matttoohey.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Star Lord’s automated source review via Wes’s account — follow-up review.

No actionable findings in the overlap-invalidation follow-up from 239c501dc34a421fc817e98d86a6461454d235b8 to the head below. Source-review assessment: 9/10 for minimalness, elegance, and correctness within this follow-up’s scope; not approval or runtime validation.

Reviewed head: 9c758d69f3dbae1e11ce894f4e2474dd2acdf3f2
Pinned base: 59d9d6fb9e22771ad9d380900d4c9d7fe152219c
Diff merge base: 3e0a4087b9c09df911f080a3c3a3534d5d7bffab

The small production change keeps invalidation in the existing session-owned cache and tracks requested channels, so empty overlapping batches are evicted too. I checked subset/superset replacement, preservation of disjoint entries, failed/cancelled read behavior, existing cache-version/access/disposal fences, and the landing’s serialized save-readback/remount path. The two added regressions exercise the capability and real mounted React component at the appropriate layers. Incoming target-branch paths do not overlap this four-file follow-up.

Validation limits: Source-only inspection of hash-verified pinned files. No PR code, tests, builds, app, or live workflow operations were executed. Hosted CI was not checked in this cycle; browser/native behavior, performance, and human acceptance remain unverified. This COMMENT is not approval or merge authorization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants