Release v1.28.1: improvements and bug fixes since v1.28.0

[luluiz]

User-Facing Changes

This patch release bundles the accumulated bug fixes, performance improvements, and refinements merged into `develop` since `v1.28.0` was cut. See the full commit list in this PR's diff/commits tab for details (347 commits).

Description

`main` was still exactly at tag `v1.28.0` with no commits ahead. This PR merges the current state of `develop` into `main` via a `hotfix/*` branch so that, per this repo's release automation (`.github/workflows/release.yml`), merging it will trigger an automatic patch version bump (`1.28.0` -> `1.28.1`), tag creation, GitHub Release, npm publish, and GHCR image push.

Notable areas of change since v1.28.0 include (non-exhaustive):

• Bug fixes: worker memory bounds for large multi-MCAP sessions, layout validation/userNodes handling, State Transitions panel fixes, RawMessagesVirtual alignment, npm audit/vulnerability fixes, H.265 decode perf, remote MCAP read coalescing, and more.
• Chores: dependency bumps (webpack, electron, monaco-editor, tar, etc.) and audit fixes.
• Docs: agent spec and release-process documentation updates.

Note: This branch was created from `develop` in full (not a curated cherry-pick), because the fix/perf commits are interdependent with intervening feature and refactor commits, making a partial cherry-pick operationally unsafe. As a result, this PR does include some `feat:`-prefixed commits alongside the fixes -- flagging this for reviewer awareness since it is being shipped under a patch version number rather than a minor bump.

Checklist

• The web version was tested and it is running ok
• The desktop version was tested and it is running ok
• This change is covered by unit tests
• Files constants.ts, types.ts and *.style.ts have been checked and relevant code snippets have been relocated

---

Opened as part of the guided release runbook (`.github/prompts/lb-release.prompt.md`). Do not merge until CI passes and a maintainer confirms the scope (full develop merge vs curated fix-only patch) is acceptable for a patch release.

Summary by CodeRabbit

• New Features

  • Added text filtering for settings, including nested items.
  • Added configurable loading limits, concurrency, read-ahead buffering, and efficient source reuse for multi-file and remote data.
  • Added multi-platform container images for Linux systems.

• Bug Fixes

  • Improved cleanup and recovery during interrupted or failed data loading.
  • Fixed iterator cancellation and remote file stream shutdown behavior.
  • Layout data can now omit optional user-defined nodes.

• Documentation

  • Added workflows for feature adoption, reviews, releases, synchronization, and development.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9e0d31c9-d3ed-43e6-91d5-98d3afe885ab

📥 Commits

Reviewing files that changed from the base of the PR and between a07fb5e and ddd30b7.

📒 Files selected for processing (1)

• .github/workflows/stale.yml

---

Walkthrough

This change adds bounded MCAP hydration and caching, lifecycle cleanup, settings filtering, safer message-path memoization, optional layout fields, workflow prompts and skills, release automation, and dependency updates.

Changes

Runtime and product behavior

Layer / File(s)	Summary
Iterable hydration, caching, and cleanup packages/suite-base/src/players/IterablePlayer/..., packages/suite-base/src/util/...	Adds weighted source pooling, bounded initialization, configurable read-ahead, uncached large reads, stream recovery, and termination cleanup.
Product behavior updates packages/suite-base/src/components/..., packages/suite-base/src/util/layout*	Adds settings-tree text filtering, content-safe message-path memoization, and optional userNodes layout data.

Agent, release, and maintenance updates

Layer / File(s)	Summary
Agent workflows and skills .github/agents/*, .github/prompts/*, .github/skills/*, .github/copilot-instructions.md, CONTRIBUTING.md, docs/ai-agents/README.md	Adds workflow prompts, skill metadata, release and caching documentation, and updated prompt catalogs.
Release automation and dependencies .github/workflows/*, package.json, packages/*/package.json	Adds multi-platform Docker builds, merge-commit synchronization, an action upgrade, and package version updates.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DataSourceFactory
  participant WorkerSerializedIterableSource
  participant MultiIterableSource
  participant McapIterableSource
  participant HydratedSourcePool
  participant RemoteFileReadable

  DataSourceFactory->>WorkerSerializedIterableSource: initialize with hydration overrides
  WorkerSerializedIterableSource->>MultiIterableSource: create multi-source session
  MultiIterableSource->>HydratedSourcePool: create shared bounded pool
  MultiIterableSource->>McapIterableSource: initialize sources
  McapIterableSource->>HydratedSourcePool: acquire indexed reader
  HydratedSourcePool->>RemoteFileReadable: open or reuse cached reader
  McapIterableSource->>HydratedSourcePool: release reader
  WorkerSerializedIterableSource->>MultiIterableSource: terminate
  MultiIterableSource->>HydratedSourcePool: terminate pool

Loading

Possibly related PRs

• lichtblick-suite/lichtblick#1150: Overlaps in MCAP remote caching and read-ahead behavior.
• lichtblick-suite/lichtblick#1262: Overlaps in hydration pooling and lifecycle handling.
• lichtblick-suite/lichtblick#1282: Overlaps in release prompts, skills, and catalog updates.

Suggested labels: documentation, performance

Suggested reviewers: rodrigo-rodrigues-ctw

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the v1.28.1 release and summarizes the included improvements and bug fixes.
Description check	✅ Passed	The description includes all required template sections and provides release scope, notable changes, automation details, and merge conditions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch hotfix/v1.28.1

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

putComment timed out

[coderabbitai]

Actionable comments posted: 16

🧹 Nitpick comments (10)

packages/suite-base/src/components/MessagePathSyntax/messagePathsForDatatype.ts (1)

204-204: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a direct cache-key regression test.

The included tests do not execute messagePathsForStructure: structureAllItemsByPath.test.ts mocks it, and useStructureItemsByPath.test.ts mocks its caller.

Add a colocated Jest test that calls messagePathsForStructure with ["a,b", "c"] and ["a", "b,c"]. Use a structure that matches only one input. This verifies that the second call does not reuse the first cached result.

As per coding guidelines, use a Jest *.test.ts file and the Given-When-Then pattern.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/suite-base/src/components/MessagePathSyntax/messagePathsForDatatype.ts`
at line 204, Add a colocated Jest *.test.ts regression test that directly
invokes messagePathsForStructure twice with validTypes ["a,b", "c"] and ["a",
"b,c"], using a structure that matches only one input. Follow the
Given-When-Then pattern and assert the second call returns only its own matching
result, proving distinct cache keys prevent reuse of the first result.

Source: Coding guidelines

packages/suite-base/src/components/SettingsTreeEditor/NodeEditor.test.tsx (1)

25-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test text filtering through the rendered FieldEditor.

This mock captures FieldEditor action handlers and invokes them directly. It bypasses the input event and does not verify the FieldEditor to NodeEditor integration.

Use userEvent to enter text in the rendered filter field. Keep the assertions on the visible tree nodes.

As per coding guidelines, "Avoid mocking internals — prefer testing behavior over implementation."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/components/SettingsTreeEditor/NodeEditor.test.tsx`
around lines 25 - 49, Update the NodeEditor tests to exercise text filtering
through the rendered FieldEditor using userEvent rather than invoking
capturedActionHandlers directly. Remove the direct text-filter handler helper
and any internal FieldEditor mocking needed only for it, while preserving
assertions against the visible tree nodes and validating the rendered
FieldEditor-to-NodeEditor interaction.

Sources: Coding guidelines, Path instructions

packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.ts (1)

301-308: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Clear #eagerInner after terminating it.

terminate() clears #persistentReadable, so the readable closes exactly once. It does not clear #eagerInner. A second terminate() call therefore invokes #eagerInner.terminate() again. McapIterableSource.test.ts line 557 covers double termination only for the pooled/url case, where #eagerInner is undefined.

Clear the field so termination is idempotent for unindexed and unpooled sources too.

♻️ Proposed fix

     await this.#eagerInner?.terminate?.();
+    this.#eagerInner = undefined;
     // The pool's hydrator.close() intentionally does not close this — it is session-persistent
     // (see `#persistentReadable`) and is only closed when the whole source is torn down.
     this.#persistentReadable?.close();
     this.#persistentReadable = undefined;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.ts`
around lines 301 - 308, Update McapIterableSource.terminate() to clear
`#eagerInner` immediately after awaiting its termination, ensuring subsequent
terminate() calls do not invoke it again while preserving the existing
persistent-readable cleanup.

packages/suite-base/src/util/CachedFilelike.test.ts (1)

196-228: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider dropping the debug-log assertion.

Line 223 asserts on the exact debug message "Ignoring degenerate". That couples the test to log wording. The behavioral guarantee is already covered by lines 221-222: exactly one fetch, for the real range. Removing the log assertion keeps the test green when the message is reworded.

The jest.doMock("./getNewConnection") stub is acceptable here, because an inverted range cannot be produced through the public API.

As per path instructions for **/*.test.ts: "Avoid mocking internals — prefer testing behavior over implementation."

♻️ Proposed test simplification

         expect(getNewConnection).toHaveBeenCalledTimes(2);
         expect(fetch).toHaveBeenCalledTimes(1);
         expect(fetch).toHaveBeenCalledWith(0, 4);
-        expect(debug).toHaveBeenCalledWith(expect.stringContaining("Ignoring degenerate"));

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/util/CachedFilelike.test.ts` around lines 196 - 228,
Remove the debug-log assertion from the test around MockedCachedFilelike.read,
keeping the behavioral assertions that getNewConnection is called twice and
fileReader.fetch runs once for the real range. Leave the existing
getNewConnection mock setup and cleanup unchanged.

Source: Path instructions

packages/suite-base/src/util/CachedFilelike.types.ts (1)

8-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider typing the data listener chunk as Uint8Array.

The generic on<T>(event: "data", listener: (chunk: T) => void) lets a caller declare any chunk type. Both implementations (BrowserHttpReader/FetchReader and the in-memory test readers) emit Uint8Array. A concrete parameter type makes the contract exact and removes the need for callers to annotate the chunk.

As per path instructions for **/*.types.ts: "Check for accurate and comprehensive type definitions."

♻️ Proposed type tightening

 export type FileStream = {
-  on<T>(event: "data", listener: (chunk: T) => void): void;
+  on(event: "data", listener: (chunk: Uint8Array) => void): void;
   on(event: "error", listener: (err: Error) => void): void;
   on(event: "end", listener: () => void): void;
   destroy: () => void;
 };

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/util/CachedFilelike.types.ts` around lines 8 - 13,
Update the FileStream type’s "data" overload to accept a listener receiving
Uint8Array instead of using the unconstrained generic on<T>. Preserve the
existing error, end, and destroy signatures so the stream contract matches
BrowserHttpReader, FetchReader, and the in-memory readers.

Source: Path instructions

packages/suite-base/src/players/IterablePlayer/WorkerSerializedIterableSource.test.ts (1)

37-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Replace the as any casts with a cast through unknown.

The coding guidelines forbid any in TypeScript sources. The mock only needs to satisfy the ComlinkWrap return type, so a cast through unknown to that type keeps the intent and removes any.

-      remote: jest.fn().mockResolvedValue(remote) as any,
+      remote: jest.fn().mockResolvedValue(remote) as unknown as ReturnType<
+        typeof ComlinkWrap
+      >["remote"],

As per coding guidelines, "Use TypeScript strict mode, avoid any, and prefer undefined over null."

Also applies to: 59-62, 72-75

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/suite-base/src/players/IterablePlayer/WorkerSerializedIterableSource.test.ts`
around lines 37 - 40, Replace the as any casts in each
mockComlinkWrap.mockReturnValueOnce setup with casts through unknown to the
expected ComlinkWrap type, including the occurrences around the remote/dispose
mocks. Preserve the existing mock behavior while eliminating all explicit any
usage.

Source: Coding guidelines

packages/suite-base/src/dataSources/RemoteDataSourceFactory.test.tsx (1)

37-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test the extension check through initialize() instead of casting to any.

(factory as any).checkExtensionMatch(...) uses any and couples the test to a private method. The same three cases are observable through the public initialize() entry point: a single URL, two URLs with the same extension, and two URLs with different extensions (which must throw "All sources need to be from the same type").

-  it("should throw an error if the comparator and comparing extensions are different", () => {
-    expect(() => (factory as any).checkExtensionMatch(".mcap", ".bag")).toThrow(
-      "All sources need to be from the same type",
-    );
+  it("should throw an error when the urls have different extensions", () => {
+    const mockArgs = setupArgs({
+      url: "https://example.com/test1.mcap,https://example.com/test2.bag",
+    });
+
+    expect(() => factory.initialize(mockArgs)).toThrow("All sources need to be from the same type");

As per coding guidelines, "Use TypeScript strict mode, avoid any", and as per path instructions, "Avoid mocking internals — prefer testing behavior over implementation."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/dataSources/RemoteDataSourceFactory.test.tsx` around
lines 37 - 52, Rewrite these extension-match tests to call the public
factory.initialize() entry point instead of casting the factory to any and
invoking checkExtensionMatch directly. Cover a single URL, two URLs sharing an
extension, and two URLs with different extensions; preserve the expected
returned extension and the "All sources need to be from the same type" error
assertion.

Sources: Coding guidelines, Path instructions

packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.test.ts (1)

475-541: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The characterization test depends on an exact microtask count, so it is brittle.

The test flushes exactly five microtasks (lines 517-519) to observe that the evicted instance's close() has started but not finished. That count encodes the current internal await sequence of release() and #evictBeyondCapacity(). Any refactor of those awaits changes the required count and fails the test, even when the observable pool contract is unchanged.

Two lower-risk options:

1. Replace the fixed loop with a deterministic signal: resolve a promise inside the close() mock (closeStarted) and await that promise instead of counting microtasks.
2. Drop the in-flight-overlap assertions and keep only the assertions about pool bookkeeping (open call count, pool.size, pins), which is the contract you actually want to protect.

♻️ Deterministic wait instead of a microtask count

+      let signalCloseStarted: (() => void) | undefined;
+      const closeStartedPromise = new Promise<void>((resolve) => {
+        signalCloseStarted = resolve;
+      });
       const hydratorA: SourceHydrator<unknown> & { open: jest.Mock; close: jest.Mock } = {
         open: jest.fn().mockResolvedValue("A-instance"),
         close: jest.fn().mockImplementation(async () => {
           closeStarted();
+          signalCloseStarted?.();
           await new Promise<void>((resolve) => {
             resolveClose = resolve;
           });
           closeFinished();
         }),
       };
@@
-      // Flush microtasks so the abandoned eviction sweep's own close() call gets a chance to
-      // start (it awaits `entry.value` first), without allowing it to finish.
-      for (let i = 0; i < 5; i++) {
-        await Promise.resolve();
-      }
+      // Wait until the abandoned eviction sweep actually entered close(), without letting it
+      // finish.
+      await closeStartedPromise;

As per path instructions, "Avoid mocking internals — prefer testing behavior over implementation."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.test.ts`
around lines 475 - 541, Replace the fixed five-microtask loop in the
characterization test with a deterministic synchronization signal from the
hydratorA.close mock, awaiting confirmation that closeStarted has been invoked
before asserting closeFinished remains pending. Keep the observable overlap and
pool bookkeeping assertions intact, without relying on release() or
`#evictBeyondCapacity`()’s internal await sequence.

Source: Path instructions

packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.ts (1)

124-132: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

A terminate() during an in-flight acquire() leaks the hydrated value and inflates #totalWeight.

acquire() inserts the entry, then awaits entry.value. If terminate() runs during that await, it clears #entries and resets #totalWeight to 0, and it closes only the snapshot it captured. The pending open() promise resolves afterwards, so this path:

• adds entry.weight back into #totalWeight although no entry is resident, and
• returns a hydrated value that is no longer tracked by the pool, so hydrator.close(value) is never called.

admit() and the not-resident branch of acquire() already guard on #terminated; the post-await path does not. The impact is limited when the pool lives in a worker that is killed right after termination, but MultiIterableSource can also run the pool without a worker teardown.

Re-check the terminal flag after the await.

♻️ Proposed guard

     this.#entries.set(token, entry);
     try {
       const value = (await entry.value) as T;
+      if (this.#terminated) {
+        // terminate() already cleared the map (and reset `#totalWeight`) while open() was in
+        // flight, so this value is untracked: close it instead of retaining or counting it.
+        entry.pins -= 1;
+        if (this.#entries.get(token) === entry) {
+          this.#entries.delete(token);
+        }
+        await hydrator.close(value);
+        throw new Error("HydratedSourcePool has been terminated");
+      }
       entry.weight = Math.max(0, hydrator.weigh?.(value) ?? 1);
       this.#totalWeight += entry.weight;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.ts`
around lines 124 - 132, Update the post-await path in acquire() to re-check
`#terminated` immediately after awaiting entry.value, before assigning
entry.weight or increasing `#totalWeight`. If termination occurred, close the
resolved hydrated value via the hydrator and return without re-admitting or
exposing it; preserve the existing behavior for non-terminated acquisitions.

packages/suite-base/src/util/getNewConnection.ts (1)

125-133: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Cap read-ahead at maxRequestSize.

CachedFilelike passes maxRequestSize: this.#cacheSizeInBytes and readAheadBufferBytes: this.#readAheadBufferBytes separately, and callers can configure readAheadBufferBytes larger than cacheSizeInBytes. Clamping the extension avoids downloading bytes the LRU cache cannot hold.

♻️ Optional clamp

     return {
       ...notDownloadedRanges[0],
       end: Math.max(
         notDownloadedRanges[0].end,
-        Math.min(readRequestRange.start + readAheadBufferBytes, fileSize),
+        Math.min(
+          readRequestRange.start + Math.min(readAheadBufferBytes, maxRequestSize),
+          fileSize,
+        ),
       ),
     };

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/util/getNewConnection.ts` around lines 125 - 133,
Update the read-ahead calculation in getNewConnection to cap the requested end
at maxRequestSize, while preserving the existing fileSize cap and ensuring the
range never shrinks below notDownloadedRanges[0].end. Use the existing
maxRequestSize configuration alongside readAheadBufferBytes.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/agents/lb-player.agent.md:
- Line 309: Update the lifecycle guidance to retain only the durable rule for
safely handling initialize()/terminate() races and the reference to
WorkerSerializedIterableSource. Remove the session-relative wording and the
statement that WorkerIterableSource has not yet been audited; track that
follow-up separately if needed.

In @.github/prompts/lb-review-pr.prompt.md:
- Line 15: Update the prose references in the prompt to capitalize “GitHub,”
including “GitHub MCP server” at the locations associated with lines 15, 56, and
93; preserve the lowercase github/list_pull_request_review_comments tool
identifiers unchanged.

In @.github/prompts/lb-setup-worktree.prompt.md:
- Line 65: Update the fetch command in the worktree setup workflow to use the
existing upstream remote when it is available, falling back to origin otherwise;
preserve the pull-request ref and local pr-{id} target.
- Around line 101-106: Update the teardown commands in the worktree setup
instructions to remove the PR-specific worktree using the PR worktree name
format, such as pr-{id}, instead of the optional {short-name}; keep git worktree
prune unchanged.
- Line 73: Update the installation block in the worktree setup prompt to use
Corepack: enable it first, then run the dependency installation through Corepack
so the declared Yarn 4.17.0 version is honored. Keep the existing installation
flow otherwise unchanged.

In @.github/prompts/lb-upstream-sync.prompt.md:
- Around line 54-61: Add a dedicated “Rollback plan” item to the required output
format in the prompt, alongside “Risk and compatibility report,” so Phase 2
explicitly requires both the risk matrix and rollback plan.

In @.github/skills/release-process/SKILL.md:
- Around line 90-92: The release-process documentation currently describes a
non-functional manual trigger. Update the documented workflow guidance around
manual workflow_dispatch to require and use an explicit tag input for checkout
and Docker version tagging, or remove workflow_dispatch from the documented
trigger options; ensure manual runs cannot depend on
github.event.release.tag_name.
- Line 97: Align the Yarn version referenced by the release rules with the
repository-wide dependency-manager version: update the Yarn 4.17.0 reference in
the release-process guidance and any related configuration or documentation so
it consistently matches the documented Yarn 3.6.3 version, or update all
repository rules to the configured version if that is the intended standard.

In @.github/skills/remote-caching/SKILL.md:
- Around line 455-458: Update the unindexed-source handling in
McapIterableSource so McapUnindexedIterableSource instances do not remain
unbounded in `#eagerInner` for the entire session. Add a bounded fallback or
enforce a separate memory/count limit before retaining or releasing these
sources, while preserving the existing pool bypass and indexed-source behavior.

In @.github/workflows/release-sync.yml:
- Around line 72-78: Update the “Enable auto-merge (merge commit) for clean
syncs” step to propagate a nonzero status when gh pr merge "$BRANCH_NAME"
--merge --auto fails. Remove the success-swallowing fallback while preserving
the existing warning context and clean-sync condition.

In @.github/workflows/stale.yml:
- Line 17: Update the stale workflow permissions associated with actions/stale
so the job grants contents: write for deleting expired pull-request branches and
actions: write for the action’s state/cache operations, while preserving the
existing issues: write and pull-requests: write permissions.

In `@package.json`:
- Line 3: Restore the release version to 1.28.1 in package.json at lines 3-3,
packages/suite/package.json at lines 3-3, and sonar-project.properties at lines
4-4, updating the root and suite package versions plus sonar.projectVersion
consistently.

In `@packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.ts`:
- Around line 184-192: Update `#openUnindexedUrlFallback` to validate response.ok
immediately after fetch and throw an error that preserves the HTTP status before
inspecting or streaming response.body. Also validate the parsed Content-Length
and reject non-numeric or otherwise invalid sizes before constructing the
fallback source.

In
`@packages/suite-base/src/players/IterablePlayer/shared/utils/mergeSequentialIterators.ts`:
- Around line 65-92: Wrap the eager activation, seek-time activation,
pending-source advancement, and drain loop in one try/finally so failures from
any activateSource call are cleaned up. Keep nextSourceIndex, activateSource,
and activateNextSource declared before the try, and ensure the existing finally
closes every iterator tracked in activeIterators.

In `@packages/suite-base/src/util/CachedFilelike.ts`:
- Around line 419-445: Bound retries in the interruption handling around
`#keepReconnectingCallback` and `#updateState`: when a premature stream end makes
zero progress, track repeated zero-progress attempts for the same range and
apply a retry cap or backoff before destroying the connection. Ensure the limit
is enforced even when keepReconnectingCallback is set, while preserving normal
reconnection behavior when bytes are received or progress resumes.

In `@packages/suite-base/src/util/layout.test.ts`:
- Around line 715-730: Update the validateLayoutData assertions in the userNodes
validation tests to use double-quoted error text instead of single quotes,
matching the repository’s Biome quote rule for ts/tsx/js/jsx files. Keep the
existing expectations and test names unchanged; only adjust the string literals
in the two affected expect(...).toThrow calls.

---

Nitpick comments:
In
`@packages/suite-base/src/components/MessagePathSyntax/messagePathsForDatatype.ts`:
- Line 204: Add a colocated Jest *.test.ts regression test that directly invokes
messagePathsForStructure twice with validTypes ["a,b", "c"] and ["a", "b,c"],
using a structure that matches only one input. Follow the Given-When-Then
pattern and assert the second call returns only its own matching result, proving
distinct cache keys prevent reuse of the first result.

In `@packages/suite-base/src/components/SettingsTreeEditor/NodeEditor.test.tsx`:
- Around line 25-49: Update the NodeEditor tests to exercise text filtering
through the rendered FieldEditor using userEvent rather than invoking
capturedActionHandlers directly. Remove the direct text-filter handler helper
and any internal FieldEditor mocking needed only for it, while preserving
assertions against the visible tree nodes and validating the rendered
FieldEditor-to-NodeEditor interaction.

In `@packages/suite-base/src/dataSources/RemoteDataSourceFactory.test.tsx`:
- Around line 37-52: Rewrite these extension-match tests to call the public
factory.initialize() entry point instead of casting the factory to any and
invoking checkExtensionMatch directly. Cover a single URL, two URLs sharing an
extension, and two URLs with different extensions; preserve the expected
returned extension and the "All sources need to be from the same type" error
assertion.

In `@packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.ts`:
- Around line 301-308: Update McapIterableSource.terminate() to clear
`#eagerInner` immediately after awaiting its termination, ensuring subsequent
terminate() calls do not invoke it again while preserving the existing
persistent-readable cleanup.

In
`@packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.test.ts`:
- Around line 475-541: Replace the fixed five-microtask loop in the
characterization test with a deterministic synchronization signal from the
hydratorA.close mock, awaiting confirmation that closeStarted has been invoked
before asserting closeFinished remains pending. Keep the observable overlap and
pool bookkeeping assertions intact, without relying on release() or
`#evictBeyondCapacity`()’s internal await sequence.

In `@packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.ts`:
- Around line 124-132: Update the post-await path in acquire() to re-check
`#terminated` immediately after awaiting entry.value, before assigning
entry.weight or increasing `#totalWeight`. If termination occurred, close the
resolved hydrated value via the hydrator and return without re-admitting or
exposing it; preserve the existing behavior for non-terminated acquisitions.

In
`@packages/suite-base/src/players/IterablePlayer/WorkerSerializedIterableSource.test.ts`:
- Around line 37-40: Replace the as any casts in each
mockComlinkWrap.mockReturnValueOnce setup with casts through unknown to the
expected ComlinkWrap type, including the occurrences around the remote/dispose
mocks. Preserve the existing mock behavior while eliminating all explicit any
usage.

In `@packages/suite-base/src/util/CachedFilelike.test.ts`:
- Around line 196-228: Remove the debug-log assertion from the test around
MockedCachedFilelike.read, keeping the behavioral assertions that
getNewConnection is called twice and fileReader.fetch runs once for the real
range. Leave the existing getNewConnection mock setup and cleanup unchanged.

In `@packages/suite-base/src/util/CachedFilelike.types.ts`:
- Around line 8-13: Update the FileStream type’s "data" overload to accept a
listener receiving Uint8Array instead of using the unconstrained generic on<T>.
Preserve the existing error, end, and destroy signatures so the stream contract
matches BrowserHttpReader, FetchReader, and the in-memory readers.

In `@packages/suite-base/src/util/getNewConnection.ts`:
- Around line 125-133: Update the read-ahead calculation in getNewConnection to
cap the requested end at maxRequestSize, while preserving the existing fileSize
cap and ensuring the range never shrinks below notDownloadedRanges[0].end. Use
the existing maxRequestSize configuration alongside readAheadBufferBytes.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 311d350a-9680-47de-8075-85378326e077

📥 Commits

Reviewing files that changed from the base of the PR and between ffc9b20 and 5e362cd.

⛔ Files ignored due to path filters (1)

• yarn.lock is excluded by !**/yarn.lock, !**/*.lock, !yarn.lock

📒 Files selected for processing (101)

• .github/agents/lb-message-pipeline.agent.md
• .github/agents/lb-orchestrator.agent.md
• .github/agents/lb-panel-image.agent.md
• .github/agents/lb-panel-log.agent.md
• .github/agents/lb-panel-map.agent.md
• .github/agents/lb-panel-plot.agent.md
• .github/agents/lb-panel-state-transitions.agent.md
• .github/agents/lb-panel-user-scripts.agent.md
• .github/agents/lb-panels-general.agent.md
• .github/agents/lb-player.agent.md
• .github/agents/lb-remote-connection.agent.md
• .github/agents/lb-websocket-connection.agent.md
• .github/copilot-instructions.md
• .github/prompts/lb-bug-fix.prompt.md
• .github/prompts/lb-feature-adopt.prompt.md
• .github/prompts/lb-feature-develop.prompt.md
• .github/prompts/lb-open-pr.prompt.md
• .github/prompts/lb-release.prompt.md
• .github/prompts/lb-review-pr.prompt.md
• .github/prompts/lb-setup-worktree.prompt.md
• .github/prompts/lb-upstream-sync.prompt.md
• .github/skills/3d-rendering/SKILL.md
• .github/skills/caching-internals/SKILL.md
• .github/skills/deserialization/SKILL.md
• .github/skills/electron-internals/SKILL.md
• .github/skills/extensions-internals/SKILL.md
• .github/skills/layouts-internals/SKILL.md
• .github/skills/mcap-format/SKILL.md
• .github/skills/message-path/SKILL.md
• .github/skills/message-pipeline/SKILL.md
• .github/skills/panel-extension-api/SKILL.md
• .github/skills/panel-image/SKILL.md
• .github/skills/panel-log/SKILL.md
• .github/skills/panel-map/SKILL.md
• .github/skills/panel-raw-messages/SKILL.md
• .github/skills/panel-state-transitions/SKILL.md
• .github/skills/panel-user-scripts/SKILL.md
• .github/skills/performance/SKILL.md
• .github/skills/player-internals/SKILL.md
• .github/skills/plot-internals/SKILL.md
• .github/skills/release-process/SKILL.md
• .github/skills/remote-caching/SKILL.md
• .github/skills/theme/SKILL.md
• .github/skills/unit-testing/SKILL.md
• .github/skills/web-workers/SKILL.md
• .github/workflows/post-release.yml
• .github/workflows/release-sync.yml
• .github/workflows/stale.yml
• CONTRIBUTING.md
• docs/ai-agents/README.md
• package.json
• packages/suite-base/package.json
• packages/suite-base/src/components/MessagePathSyntax/messagePathsForDatatype.ts
• packages/suite-base/src/components/MessagePathSyntax/structureAllItemsByPath.test.ts
• packages/suite-base/src/components/MessagePathSyntax/useStructureItemsByPath.test.ts
• packages/suite-base/src/components/MessagePathSyntax/useStructureItemsByPath.ts
• packages/suite-base/src/components/SettingsTreeEditor/NodeEditor.test.tsx
• packages/suite-base/src/components/SettingsTreeEditor/NodeEditor.tsx
• packages/suite-base/src/components/SettingsTreeEditor/types.ts
• packages/suite-base/src/context/CurrentLayoutContext/actions.ts
• packages/suite-base/src/dataSources/McapLocalDataSourceFactory.test.ts
• packages/suite-base/src/dataSources/McapLocalDataSourceFactory.ts
• packages/suite-base/src/dataSources/RemoteDataSourceFactory.test.tsx
• packages/suite-base/src/dataSources/RemoteDataSourceFactory.tsx
• packages/suite-base/src/i18n/en/settingsEditor.ts
• packages/suite-base/src/players/IterablePlayer/IIterableSource.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.test.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.types.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSourceWorker.worker.test.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSourceWorker.worker.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/RemoteFileReadable.test.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/RemoteFileReadable.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/RemoteFileReadable.types.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/readerWeight.test.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/readerWeight.ts
• packages/suite-base/src/players/IterablePlayer/WorkerSerializedIterableSource.test.ts
• packages/suite-base/src/players/IterablePlayer/WorkerSerializedIterableSource.ts
• packages/suite-base/src/players/IterablePlayer/WorkerSerializedIterableSourceWorker.ts
• packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.test.ts
• packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.ts
• packages/suite-base/src/players/IterablePlayer/shared/MultiIterableSource.test.ts
• packages/suite-base/src/players/IterablePlayer/shared/MultiIterableSource.ts
• packages/suite-base/src/players/IterablePlayer/shared/multiFileHydrationOptions.test.ts
• packages/suite-base/src/players/IterablePlayer/shared/multiFileHydrationOptions.ts
• packages/suite-base/src/players/IterablePlayer/shared/types.ts
• packages/suite-base/src/players/IterablePlayer/shared/utils/mergeSequentialIterators.test.ts
• packages/suite-base/src/players/IterablePlayer/shared/utils/mergeSequentialIterators.ts
• packages/suite-base/src/util/BrowserHttpReader.ts
• packages/suite-base/src/util/CachedFilelike.test.ts
• packages/suite-base/src/util/CachedFilelike.ts
• packages/suite-base/src/util/CachedFilelike.types.ts
• packages/suite-base/src/util/getNewConnection.test.ts
• packages/suite-base/src/util/getNewConnection.ts
• packages/suite-base/src/util/layout.test.ts
• packages/suite-base/src/util/layout.ts
• packages/suite-desktop/package.json
• packages/suite-web/package.json
• packages/suite/package.json
• packages/theme/package.json
• sonar-project.properties

[coderabbitai]

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 16

🧹 Nitpick comments (10)

packages/suite-base/src/components/MessagePathSyntax/messagePathsForDatatype.ts (1)

204-204: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a direct cache-key regression test.

The included tests do not execute messagePathsForStructure: structureAllItemsByPath.test.ts mocks it, and useStructureItemsByPath.test.ts mocks its caller.

Add a colocated Jest test that calls messagePathsForStructure with ["a,b", "c"] and ["a", "b,c"]. Use a structure that matches only one input. This verifies that the second call does not reuse the first cached result.

As per coding guidelines, use a Jest *.test.ts file and the Given-When-Then pattern.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/suite-base/src/components/MessagePathSyntax/messagePathsForDatatype.ts`
at line 204, Add a colocated Jest *.test.ts regression test that directly
invokes messagePathsForStructure twice with validTypes ["a,b", "c"] and ["a",
"b,c"], using a structure that matches only one input. Follow the
Given-When-Then pattern and assert the second call returns only its own matching
result, proving distinct cache keys prevent reuse of the first result.

Source: Coding guidelines

packages/suite-base/src/components/SettingsTreeEditor/NodeEditor.test.tsx (1)

25-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test text filtering through the rendered FieldEditor.

This mock captures FieldEditor action handlers and invokes them directly. It bypasses the input event and does not verify the FieldEditor to NodeEditor integration.

Use userEvent to enter text in the rendered filter field. Keep the assertions on the visible tree nodes.

As per coding guidelines, "Avoid mocking internals — prefer testing behavior over implementation."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/components/SettingsTreeEditor/NodeEditor.test.tsx`
around lines 25 - 49, Update the NodeEditor tests to exercise text filtering
through the rendered FieldEditor using userEvent rather than invoking
capturedActionHandlers directly. Remove the direct text-filter handler helper
and any internal FieldEditor mocking needed only for it, while preserving
assertions against the visible tree nodes and validating the rendered
FieldEditor-to-NodeEditor interaction.

Sources: Coding guidelines, Path instructions

packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.ts (1)

301-308: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Clear #eagerInner after terminating it.

terminate() clears #persistentReadable, so the readable closes exactly once. It does not clear #eagerInner. A second terminate() call therefore invokes #eagerInner.terminate() again. McapIterableSource.test.ts line 557 covers double termination only for the pooled/url case, where #eagerInner is undefined.

Clear the field so termination is idempotent for unindexed and unpooled sources too.

♻️ Proposed fix

     await this.#eagerInner?.terminate?.();
+    this.#eagerInner = undefined;
     // The pool's hydrator.close() intentionally does not close this — it is session-persistent
     // (see `#persistentReadable`) and is only closed when the whole source is torn down.
     this.#persistentReadable?.close();
     this.#persistentReadable = undefined;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.ts`
around lines 301 - 308, Update McapIterableSource.terminate() to clear
`#eagerInner` immediately after awaiting its termination, ensuring subsequent
terminate() calls do not invoke it again while preserving the existing
persistent-readable cleanup.

packages/suite-base/src/util/CachedFilelike.test.ts (1)

196-228: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider dropping the debug-log assertion.

Line 223 asserts on the exact debug message "Ignoring degenerate". That couples the test to log wording. The behavioral guarantee is already covered by lines 221-222: exactly one fetch, for the real range. Removing the log assertion keeps the test green when the message is reworded.

The jest.doMock("./getNewConnection") stub is acceptable here, because an inverted range cannot be produced through the public API.

As per path instructions for **/*.test.ts: "Avoid mocking internals — prefer testing behavior over implementation."

♻️ Proposed test simplification

         expect(getNewConnection).toHaveBeenCalledTimes(2);
         expect(fetch).toHaveBeenCalledTimes(1);
         expect(fetch).toHaveBeenCalledWith(0, 4);
-        expect(debug).toHaveBeenCalledWith(expect.stringContaining("Ignoring degenerate"));

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/util/CachedFilelike.test.ts` around lines 196 - 228,
Remove the debug-log assertion from the test around MockedCachedFilelike.read,
keeping the behavioral assertions that getNewConnection is called twice and
fileReader.fetch runs once for the real range. Leave the existing
getNewConnection mock setup and cleanup unchanged.

Source: Path instructions

packages/suite-base/src/util/CachedFilelike.types.ts (1)

8-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider typing the data listener chunk as Uint8Array.

The generic on<T>(event: "data", listener: (chunk: T) => void) lets a caller declare any chunk type. Both implementations (BrowserHttpReader/FetchReader and the in-memory test readers) emit Uint8Array. A concrete parameter type makes the contract exact and removes the need for callers to annotate the chunk.

As per path instructions for **/*.types.ts: "Check for accurate and comprehensive type definitions."

♻️ Proposed type tightening

 export type FileStream = {
-  on<T>(event: "data", listener: (chunk: T) => void): void;
+  on(event: "data", listener: (chunk: Uint8Array) => void): void;
   on(event: "error", listener: (err: Error) => void): void;
   on(event: "end", listener: () => void): void;
   destroy: () => void;
 };

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/util/CachedFilelike.types.ts` around lines 8 - 13,
Update the FileStream type’s "data" overload to accept a listener receiving
Uint8Array instead of using the unconstrained generic on<T>. Preserve the
existing error, end, and destroy signatures so the stream contract matches
BrowserHttpReader, FetchReader, and the in-memory readers.

Source: Path instructions

packages/suite-base/src/players/IterablePlayer/WorkerSerializedIterableSource.test.ts (1)

37-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Replace the as any casts with a cast through unknown.

The coding guidelines forbid any in TypeScript sources. The mock only needs to satisfy the ComlinkWrap return type, so a cast through unknown to that type keeps the intent and removes any.

-      remote: jest.fn().mockResolvedValue(remote) as any,
+      remote: jest.fn().mockResolvedValue(remote) as unknown as ReturnType<
+        typeof ComlinkWrap
+      >["remote"],

As per coding guidelines, "Use TypeScript strict mode, avoid any, and prefer undefined over null."

Also applies to: 59-62, 72-75

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/suite-base/src/players/IterablePlayer/WorkerSerializedIterableSource.test.ts`
around lines 37 - 40, Replace the as any casts in each
mockComlinkWrap.mockReturnValueOnce setup with casts through unknown to the
expected ComlinkWrap type, including the occurrences around the remote/dispose
mocks. Preserve the existing mock behavior while eliminating all explicit any
usage.

Source: Coding guidelines

packages/suite-base/src/dataSources/RemoteDataSourceFactory.test.tsx (1)

37-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test the extension check through initialize() instead of casting to any.

(factory as any).checkExtensionMatch(...) uses any and couples the test to a private method. The same three cases are observable through the public initialize() entry point: a single URL, two URLs with the same extension, and two URLs with different extensions (which must throw "All sources need to be from the same type").

-  it("should throw an error if the comparator and comparing extensions are different", () => {
-    expect(() => (factory as any).checkExtensionMatch(".mcap", ".bag")).toThrow(
-      "All sources need to be from the same type",
-    );
+  it("should throw an error when the urls have different extensions", () => {
+    const mockArgs = setupArgs({
+      url: "https://example.com/test1.mcap,https://example.com/test2.bag",
+    });
+
+    expect(() => factory.initialize(mockArgs)).toThrow("All sources need to be from the same type");

As per coding guidelines, "Use TypeScript strict mode, avoid any", and as per path instructions, "Avoid mocking internals — prefer testing behavior over implementation."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/dataSources/RemoteDataSourceFactory.test.tsx` around
lines 37 - 52, Rewrite these extension-match tests to call the public
factory.initialize() entry point instead of casting the factory to any and
invoking checkExtensionMatch directly. Cover a single URL, two URLs sharing an
extension, and two URLs with different extensions; preserve the expected
returned extension and the "All sources need to be from the same type" error
assertion.

Sources: Coding guidelines, Path instructions

packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.test.ts (1)

475-541: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The characterization test depends on an exact microtask count, so it is brittle.

The test flushes exactly five microtasks (lines 517-519) to observe that the evicted instance's close() has started but not finished. That count encodes the current internal await sequence of release() and #evictBeyondCapacity(). Any refactor of those awaits changes the required count and fails the test, even when the observable pool contract is unchanged.

Two lower-risk options:

1. Replace the fixed loop with a deterministic signal: resolve a promise inside the close() mock (closeStarted) and await that promise instead of counting microtasks.
2. Drop the in-flight-overlap assertions and keep only the assertions about pool bookkeeping (open call count, pool.size, pins), which is the contract you actually want to protect.

♻️ Deterministic wait instead of a microtask count

+      let signalCloseStarted: (() => void) | undefined;
+      const closeStartedPromise = new Promise<void>((resolve) => {
+        signalCloseStarted = resolve;
+      });
       const hydratorA: SourceHydrator<unknown> & { open: jest.Mock; close: jest.Mock } = {
         open: jest.fn().mockResolvedValue("A-instance"),
         close: jest.fn().mockImplementation(async () => {
           closeStarted();
+          signalCloseStarted?.();
           await new Promise<void>((resolve) => {
             resolveClose = resolve;
           });
           closeFinished();
         }),
       };
@@
-      // Flush microtasks so the abandoned eviction sweep's own close() call gets a chance to
-      // start (it awaits `entry.value` first), without allowing it to finish.
-      for (let i = 0; i < 5; i++) {
-        await Promise.resolve();
-      }
+      // Wait until the abandoned eviction sweep actually entered close(), without letting it
+      // finish.
+      await closeStartedPromise;

As per path instructions, "Avoid mocking internals — prefer testing behavior over implementation."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.test.ts`
around lines 475 - 541, Replace the fixed five-microtask loop in the
characterization test with a deterministic synchronization signal from the
hydratorA.close mock, awaiting confirmation that closeStarted has been invoked
before asserting closeFinished remains pending. Keep the observable overlap and
pool bookkeeping assertions intact, without relying on release() or
`#evictBeyondCapacity`()’s internal await sequence.

Source: Path instructions

packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.ts (1)

124-132: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

A terminate() during an in-flight acquire() leaks the hydrated value and inflates #totalWeight.

acquire() inserts the entry, then awaits entry.value. If terminate() runs during that await, it clears #entries and resets #totalWeight to 0, and it closes only the snapshot it captured. The pending open() promise resolves afterwards, so this path:

• adds entry.weight back into #totalWeight although no entry is resident, and
• returns a hydrated value that is no longer tracked by the pool, so hydrator.close(value) is never called.

admit() and the not-resident branch of acquire() already guard on #terminated; the post-await path does not. The impact is limited when the pool lives in a worker that is killed right after termination, but MultiIterableSource can also run the pool without a worker teardown.

Re-check the terminal flag after the await.

♻️ Proposed guard

     this.#entries.set(token, entry);
     try {
       const value = (await entry.value) as T;
+      if (this.#terminated) {
+        // terminate() already cleared the map (and reset `#totalWeight`) while open() was in
+        // flight, so this value is untracked: close it instead of retaining or counting it.
+        entry.pins -= 1;
+        if (this.#entries.get(token) === entry) {
+          this.#entries.delete(token);
+        }
+        await hydrator.close(value);
+        throw new Error("HydratedSourcePool has been terminated");
+      }
       entry.weight = Math.max(0, hydrator.weigh?.(value) ?? 1);
       this.#totalWeight += entry.weight;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.ts`
around lines 124 - 132, Update the post-await path in acquire() to re-check
`#terminated` immediately after awaiting entry.value, before assigning
entry.weight or increasing `#totalWeight`. If termination occurred, close the
resolved hydrated value via the hydrator and return without re-admitting or
exposing it; preserve the existing behavior for non-terminated acquisitions.

packages/suite-base/src/util/getNewConnection.ts (1)

125-133: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Cap read-ahead at maxRequestSize.

CachedFilelike passes maxRequestSize: this.#cacheSizeInBytes and readAheadBufferBytes: this.#readAheadBufferBytes separately, and callers can configure readAheadBufferBytes larger than cacheSizeInBytes. Clamping the extension avoids downloading bytes the LRU cache cannot hold.

♻️ Optional clamp

     return {
       ...notDownloadedRanges[0],
       end: Math.max(
         notDownloadedRanges[0].end,
-        Math.min(readRequestRange.start + readAheadBufferBytes, fileSize),
+        Math.min(
+          readRequestRange.start + Math.min(readAheadBufferBytes, maxRequestSize),
+          fileSize,
+        ),
       ),
     };

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/suite-base/src/util/getNewConnection.ts` around lines 125 - 133,
Update the read-ahead calculation in getNewConnection to cap the requested end
at maxRequestSize, while preserving the existing fileSize cap and ensuring the
range never shrinks below notDownloadedRanges[0].end. Use the existing
maxRequestSize configuration alongside readAheadBufferBytes.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/agents/lb-player.agent.md:
- Line 309: Update the lifecycle guidance to retain only the durable rule for
safely handling initialize()/terminate() races and the reference to
WorkerSerializedIterableSource. Remove the session-relative wording and the
statement that WorkerIterableSource has not yet been audited; track that
follow-up separately if needed.

In @.github/prompts/lb-review-pr.prompt.md:
- Line 15: Update the prose references in the prompt to capitalize “GitHub,”
including “GitHub MCP server” at the locations associated with lines 15, 56, and
93; preserve the lowercase github/list_pull_request_review_comments tool
identifiers unchanged.

In @.github/prompts/lb-setup-worktree.prompt.md:
- Line 65: Update the fetch command in the worktree setup workflow to use the
existing upstream remote when it is available, falling back to origin otherwise;
preserve the pull-request ref and local pr-{id} target.
- Around line 101-106: Update the teardown commands in the worktree setup
instructions to remove the PR-specific worktree using the PR worktree name
format, such as pr-{id}, instead of the optional {short-name}; keep git worktree
prune unchanged.
- Line 73: Update the installation block in the worktree setup prompt to use
Corepack: enable it first, then run the dependency installation through Corepack
so the declared Yarn 4.17.0 version is honored. Keep the existing installation
flow otherwise unchanged.

In @.github/prompts/lb-upstream-sync.prompt.md:
- Around line 54-61: Add a dedicated “Rollback plan” item to the required output
format in the prompt, alongside “Risk and compatibility report,” so Phase 2
explicitly requires both the risk matrix and rollback plan.

In @.github/skills/release-process/SKILL.md:
- Around line 90-92: The release-process documentation currently describes a
non-functional manual trigger. Update the documented workflow guidance around
manual workflow_dispatch to require and use an explicit tag input for checkout
and Docker version tagging, or remove workflow_dispatch from the documented
trigger options; ensure manual runs cannot depend on
github.event.release.tag_name.
- Line 97: Align the Yarn version referenced by the release rules with the
repository-wide dependency-manager version: update the Yarn 4.17.0 reference in
the release-process guidance and any related configuration or documentation so
it consistently matches the documented Yarn 3.6.3 version, or update all
repository rules to the configured version if that is the intended standard.

In @.github/skills/remote-caching/SKILL.md:
- Around line 455-458: Update the unindexed-source handling in
McapIterableSource so McapUnindexedIterableSource instances do not remain
unbounded in `#eagerInner` for the entire session. Add a bounded fallback or
enforce a separate memory/count limit before retaining or releasing these
sources, while preserving the existing pool bypass and indexed-source behavior.

In @.github/workflows/release-sync.yml:
- Around line 72-78: Update the “Enable auto-merge (merge commit) for clean
syncs” step to propagate a nonzero status when gh pr merge "$BRANCH_NAME"
--merge --auto fails. Remove the success-swallowing fallback while preserving
the existing warning context and clean-sync condition.

In @.github/workflows/stale.yml:
- Line 17: Update the stale workflow permissions associated with actions/stale
so the job grants contents: write for deleting expired pull-request branches and
actions: write for the action’s state/cache operations, while preserving the
existing issues: write and pull-requests: write permissions.

In `@package.json`:
- Line 3: Restore the release version to 1.28.1 in package.json at lines 3-3,
packages/suite/package.json at lines 3-3, and sonar-project.properties at lines
4-4, updating the root and suite package versions plus sonar.projectVersion
consistently.

In `@packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.ts`:
- Around line 184-192: Update `#openUnindexedUrlFallback` to validate response.ok
immediately after fetch and throw an error that preserves the HTTP status before
inspecting or streaming response.body. Also validate the parsed Content-Length
and reject non-numeric or otherwise invalid sizes before constructing the
fallback source.

In
`@packages/suite-base/src/players/IterablePlayer/shared/utils/mergeSequentialIterators.ts`:
- Around line 65-92: Wrap the eager activation, seek-time activation,
pending-source advancement, and drain loop in one try/finally so failures from
any activateSource call are cleaned up. Keep nextSourceIndex, activateSource,
and activateNextSource declared before the try, and ensure the existing finally
closes every iterator tracked in activeIterators.

In `@packages/suite-base/src/util/CachedFilelike.ts`:
- Around line 419-445: Bound retries in the interruption handling around
`#keepReconnectingCallback` and `#updateState`: when a premature stream end makes
zero progress, track repeated zero-progress attempts for the same range and
apply a retry cap or backoff before destroying the connection. Ensure the limit
is enforced even when keepReconnectingCallback is set, while preserving normal
reconnection behavior when bytes are received or progress resumes.

In `@packages/suite-base/src/util/layout.test.ts`:
- Around line 715-730: Update the validateLayoutData assertions in the userNodes
validation tests to use double-quoted error text instead of single quotes,
matching the repository’s Biome quote rule for ts/tsx/js/jsx files. Keep the
existing expectations and test names unchanged; only adjust the string literals
in the two affected expect(...).toThrow calls.

---

Nitpick comments:
In
`@packages/suite-base/src/components/MessagePathSyntax/messagePathsForDatatype.ts`:
- Line 204: Add a colocated Jest *.test.ts regression test that directly invokes
messagePathsForStructure twice with validTypes ["a,b", "c"] and ["a", "b,c"],
using a structure that matches only one input. Follow the Given-When-Then
pattern and assert the second call returns only its own matching result, proving
distinct cache keys prevent reuse of the first result.

In `@packages/suite-base/src/components/SettingsTreeEditor/NodeEditor.test.tsx`:
- Around line 25-49: Update the NodeEditor tests to exercise text filtering
through the rendered FieldEditor using userEvent rather than invoking
capturedActionHandlers directly. Remove the direct text-filter handler helper
and any internal FieldEditor mocking needed only for it, while preserving
assertions against the visible tree nodes and validating the rendered
FieldEditor-to-NodeEditor interaction.

In `@packages/suite-base/src/dataSources/RemoteDataSourceFactory.test.tsx`:
- Around line 37-52: Rewrite these extension-match tests to call the public
factory.initialize() entry point instead of casting the factory to any and
invoking checkExtensionMatch directly. Cover a single URL, two URLs sharing an
extension, and two URLs with different extensions; preserve the expected
returned extension and the "All sources need to be from the same type" error
assertion.

In `@packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.ts`:
- Around line 301-308: Update McapIterableSource.terminate() to clear
`#eagerInner` immediately after awaiting its termination, ensuring subsequent
terminate() calls do not invoke it again while preserving the existing
persistent-readable cleanup.

In
`@packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.test.ts`:
- Around line 475-541: Replace the fixed five-microtask loop in the
characterization test with a deterministic synchronization signal from the
hydratorA.close mock, awaiting confirmation that closeStarted has been invoked
before asserting closeFinished remains pending. Keep the observable overlap and
pool bookkeeping assertions intact, without relying on release() or
`#evictBeyondCapacity`()’s internal await sequence.

In `@packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.ts`:
- Around line 124-132: Update the post-await path in acquire() to re-check
`#terminated` immediately after awaiting entry.value, before assigning
entry.weight or increasing `#totalWeight`. If termination occurred, close the
resolved hydrated value via the hydrator and return without re-admitting or
exposing it; preserve the existing behavior for non-terminated acquisitions.

In
`@packages/suite-base/src/players/IterablePlayer/WorkerSerializedIterableSource.test.ts`:
- Around line 37-40: Replace the as any casts in each
mockComlinkWrap.mockReturnValueOnce setup with casts through unknown to the
expected ComlinkWrap type, including the occurrences around the remote/dispose
mocks. Preserve the existing mock behavior while eliminating all explicit any
usage.

In `@packages/suite-base/src/util/CachedFilelike.test.ts`:
- Around line 196-228: Remove the debug-log assertion from the test around
MockedCachedFilelike.read, keeping the behavioral assertions that
getNewConnection is called twice and fileReader.fetch runs once for the real
range. Leave the existing getNewConnection mock setup and cleanup unchanged.

In `@packages/suite-base/src/util/CachedFilelike.types.ts`:
- Around line 8-13: Update the FileStream type’s "data" overload to accept a
listener receiving Uint8Array instead of using the unconstrained generic on<T>.
Preserve the existing error, end, and destroy signatures so the stream contract
matches BrowserHttpReader, FetchReader, and the in-memory readers.

In `@packages/suite-base/src/util/getNewConnection.ts`:
- Around line 125-133: Update the read-ahead calculation in getNewConnection to
cap the requested end at maxRequestSize, while preserving the existing fileSize
cap and ensuring the range never shrinks below notDownloadedRanges[0].end. Use
the existing maxRequestSize configuration alongside readAheadBufferBytes.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 311d350a-9680-47de-8075-85378326e077

📥 Commits

Reviewing files that changed from the base of the PR and between ffc9b20 and 5e362cd.

⛔ Files ignored due to path filters (1)

• yarn.lock is excluded by !**/yarn.lock, !**/*.lock, !yarn.lock

📒 Files selected for processing (101)

• .github/agents/lb-message-pipeline.agent.md
• .github/agents/lb-orchestrator.agent.md
• .github/agents/lb-panel-image.agent.md
• .github/agents/lb-panel-log.agent.md
• .github/agents/lb-panel-map.agent.md
• .github/agents/lb-panel-plot.agent.md
• .github/agents/lb-panel-state-transitions.agent.md
• .github/agents/lb-panel-user-scripts.agent.md
• .github/agents/lb-panels-general.agent.md
• .github/agents/lb-player.agent.md
• .github/agents/lb-remote-connection.agent.md
• .github/agents/lb-websocket-connection.agent.md
• .github/copilot-instructions.md
• .github/prompts/lb-bug-fix.prompt.md
• .github/prompts/lb-feature-adopt.prompt.md
• .github/prompts/lb-feature-develop.prompt.md
• .github/prompts/lb-open-pr.prompt.md
• .github/prompts/lb-release.prompt.md
• .github/prompts/lb-review-pr.prompt.md
• .github/prompts/lb-setup-worktree.prompt.md
• .github/prompts/lb-upstream-sync.prompt.md
• .github/skills/3d-rendering/SKILL.md
• .github/skills/caching-internals/SKILL.md
• .github/skills/deserialization/SKILL.md
• .github/skills/electron-internals/SKILL.md
• .github/skills/extensions-internals/SKILL.md
• .github/skills/layouts-internals/SKILL.md
• .github/skills/mcap-format/SKILL.md
• .github/skills/message-path/SKILL.md
• .github/skills/message-pipeline/SKILL.md
• .github/skills/panel-extension-api/SKILL.md
• .github/skills/panel-image/SKILL.md
• .github/skills/panel-log/SKILL.md
• .github/skills/panel-map/SKILL.md
• .github/skills/panel-raw-messages/SKILL.md
• .github/skills/panel-state-transitions/SKILL.md
• .github/skills/panel-user-scripts/SKILL.md
• .github/skills/performance/SKILL.md
• .github/skills/player-internals/SKILL.md
• .github/skills/plot-internals/SKILL.md
• .github/skills/release-process/SKILL.md
• .github/skills/remote-caching/SKILL.md
• .github/skills/theme/SKILL.md
• .github/skills/unit-testing/SKILL.md
• .github/skills/web-workers/SKILL.md
• .github/workflows/post-release.yml
• .github/workflows/release-sync.yml
• .github/workflows/stale.yml
• CONTRIBUTING.md
• docs/ai-agents/README.md
• package.json
• packages/suite-base/package.json
• packages/suite-base/src/components/MessagePathSyntax/messagePathsForDatatype.ts
• packages/suite-base/src/components/MessagePathSyntax/structureAllItemsByPath.test.ts
• packages/suite-base/src/components/MessagePathSyntax/useStructureItemsByPath.test.ts
• packages/suite-base/src/components/MessagePathSyntax/useStructureItemsByPath.ts
• packages/suite-base/src/components/SettingsTreeEditor/NodeEditor.test.tsx
• packages/suite-base/src/components/SettingsTreeEditor/NodeEditor.tsx
• packages/suite-base/src/components/SettingsTreeEditor/types.ts
• packages/suite-base/src/context/CurrentLayoutContext/actions.ts
• packages/suite-base/src/dataSources/McapLocalDataSourceFactory.test.ts
• packages/suite-base/src/dataSources/McapLocalDataSourceFactory.ts
• packages/suite-base/src/dataSources/RemoteDataSourceFactory.test.tsx
• packages/suite-base/src/dataSources/RemoteDataSourceFactory.tsx
• packages/suite-base/src/i18n/en/settingsEditor.ts
• packages/suite-base/src/players/IterablePlayer/IIterableSource.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.test.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSource.types.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSourceWorker.worker.test.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/McapIterableSourceWorker.worker.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/RemoteFileReadable.test.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/RemoteFileReadable.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/RemoteFileReadable.types.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/readerWeight.test.ts
• packages/suite-base/src/players/IterablePlayer/Mcap/readerWeight.ts
• packages/suite-base/src/players/IterablePlayer/WorkerSerializedIterableSource.test.ts
• packages/suite-base/src/players/IterablePlayer/WorkerSerializedIterableSource.ts
• packages/suite-base/src/players/IterablePlayer/WorkerSerializedIterableSourceWorker.ts
• packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.test.ts
• packages/suite-base/src/players/IterablePlayer/shared/HydratedSourcePool.ts
• packages/suite-base/src/players/IterablePlayer/shared/MultiIterableSource.test.ts
• packages/suite-base/src/players/IterablePlayer/shared/MultiIterableSource.ts
• packages/suite-base/src/players/IterablePlayer/shared/multiFileHydrationOptions.test.ts
• packages/suite-base/src/players/IterablePlayer/shared/multiFileHydrationOptions.ts
• packages/suite-base/src/players/IterablePlayer/shared/types.ts
• packages/suite-base/src/players/IterablePlayer/shared/utils/mergeSequentialIterators.test.ts
• packages/suite-base/src/players/IterablePlayer/shared/utils/mergeSequentialIterators.ts
• packages/suite-base/src/util/BrowserHttpReader.ts
• packages/suite-base/src/util/CachedFilelike.test.ts
• packages/suite-base/src/util/CachedFilelike.ts
• packages/suite-base/src/util/CachedFilelike.types.ts
• packages/suite-base/src/util/getNewConnection.test.ts
• packages/suite-base/src/util/getNewConnection.ts
• packages/suite-base/src/util/layout.test.ts
• packages/suite-base/src/util/layout.ts
• packages/suite-desktop/package.json
• packages/suite-web/package.json
• packages/suite/package.json
• packages/theme/package.json
• sonar-project.properties

🛑 Comments failed to post (5)

.github/prompts/lb-review-pr.prompt.md (1)

15-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Capitalize GitHub in prose.

Use GitHub MCP server on Lines 15, 56, and 93. Keep the lowercase github/list_pull_request_review_comments identifiers on Lines 95-96 unchanged because those are tool names.

Also applies to: 56-56, 93-93

🧰 Tools

🪛 LanguageTool

[uncategorized] ~15-~15: The official name of this software platform is spelled with a capital “H”.
Context: ...### Inputs - PR number or URL (use the github MCP server to fetch diff and review th...

(GITHUB)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/prompts/lb-review-pr.prompt.md at line 15, Update the prose
references in the prompt to capitalize “GitHub,” including “GitHub MCP server”
at the locations associated with lines 15, 56, and 93; preserve the lowercase
github/list_pull_request_review_comments tool identifiers unchanged.

Source: Linters/SAST tools

.github/prompts/lb-setup-worktree.prompt.md (3)

65-65: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fetch pull-request refs from the base repository.

When upstream exists, this workflow identifies a fork contributor and uses upstream/develop as the base. In that case, origin is the contributor fork, so git fetch origin pull/{id}/head:pr-{id} cannot fetch the upstream pull-request ref. Select upstream when present, and origin otherwise.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/prompts/lb-setup-worktree.prompt.md at line 65, Update the fetch
command in the worktree setup workflow to use the existing upstream remote when
it is available, falling back to origin otherwise; preserve the pull-request ref
and local pr-{id} target.

---

73-73: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== candidate files =="
fd -i "package.json|yarn.lock|corepack|lb-setup-worktree" . || true

echo
echo "== prompt lines around command =="
if [ -f .github/prompts/lb-setup-worktree.prompt.md ]; then
  sed -n '60,80p' .github/prompts/lb-setup-worktree.prompt.md | cat -n
fi

echo
echo "== all yarn install / corepack usage in prompt =="
rg -n "yarn install|corepack|yarnVersion|packageManager" .github/prompts package.json yarn.lock 2>/dev/null || true

echo
echo "== package manager declarations =="
python3 - <<'PY'
import json
from pathlib import Path
p=Path('package.json')
if p.exists():
    data=json.loads(p.read_text())
    print("packageManager:", data.get("packageManager"))
    print("engines:", data.get("engines"))
PY

Repository: lichtblick-suite/lichtblick

Length of output: 3056

---

Use Corepack for dependency installation.

This package declares yarn@4.17.0 in package.json, and the prompt still runs yarn install directly. Replace the installation block with corepack enable followed by corepack yarn install, or change the package manager declaration if Yarn 3.6.3 is required here.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/prompts/lb-setup-worktree.prompt.md at line 73, Update the
installation block in the worktree setup prompt to use Corepack: enable it
first, then run the dependency installation through Corepack so the declared
Yarn 4.17.0 version is honored. Keep the existing installation flow otherwise
unchanged.

Source: Coding guidelines

---

101-106: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Use the PR worktree name during teardown.

Review mode creates ../lichtblick.worktree/pr-{id}, but this command removes ../lichtblick.worktree/{short-name}. Since short-name is optional for PR reviews, the command can fail and leave the worktree behind.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/prompts/lb-setup-worktree.prompt.md around lines 101 - 106, Update
the teardown commands in the worktree setup instructions to remove the
PR-specific worktree using the PR worktree name format, such as pr-{id}, instead
of the optional {short-name}; keep git worktree prune unchanged.

.github/prompts/lb-upstream-sync.prompt.md (1)

54-61: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add the rollback plan to the required output.

Phase 2 requires a risk matrix and rollback plan. The output contract lists only Risk and compatibility report. An agent can omit the rollback plan and still satisfy the prompt. Add a dedicated Rollback plan output item.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/prompts/lb-upstream-sync.prompt.md around lines 54 - 61, Add a
dedicated “Rollback plan” item to the required output format in the prompt,
alongside “Risk and compatibility report,” so Phase 2 explicitly requires both
the risk matrix and rollback plan.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud