• What changed? On streamed (lazy-loaded) workbooks, the Find dialog now searches the whole sheet instead of only the rows already loaded into Univer's grid. A wrapper find provider is registered with IFindReplaceService: the built-in sheets model keeps owning everything inside the loaded window (including its canvas highlights), while the wrapper extends each session with out-of-window matches paged from the underlying file via readSheetRangeMapped — session journal edits included, coordinates already covered by the loaded window excluded. Focusing an out-of-window match activates its sheet, starts loading its range (ensureLazyRangeLoaded), scrolls to it, and selects it, so the grid shows real data instead of jumping to a blank region. Replace All reports out-of-window formula hits as failures instead of silently skipping them, consistent with the existing guard that blocks bulk replaces until the workbook is fully loaded. When a scan hits the budget or indexing lag, a status message says results may be incomplete (new appFindScanTruncated string, all 19 locales).
• Why is this change needed? The Find dialog iterates Univer's in-memory cell matrix, so on large lazy-loaded sheets every not-yet-streamed row was silently skipped: users searched for values that exist and got "no results", concluding the data does not exist. The AI-side findInLazyWorkbook already pages the underlying file for exactly this reason; this brings the UI dialog to parity using the same primitives (readSheetRangeMapped, ensureLazyRangeLoaded) and the same budgets (FILE_READ_BATCH_CELLS, MAX_SCAN_CELLS, now exported from ai/workbook-search.ts).

Implementation notes:

• Uses Univer's public extension point (registerFindReplaceProvider); no prototype patching of Univer internals. The wrapper re-adopts whichever sheets provider is registered, so it survives workbook switches, and delegates unchanged for demo/fully-preloaded workbooks.
• Matching mirrors the built-in semantics (substring vs whole-cell with space-only trimming, case sensitivity, formula-vs-value look-in); needles are lower-cased once up front like the built-in parser does. Formula hits are never value-replaced, mirroring hitCell.
• Out-of-window hits are deduped against the live inner list on every getMatches(), so as regions stream in or evict, counts stay correct and navigation hands back to the native model once a jumped-to cell is materialized (the inner model re-runs on mutations).
• New module apps/sheets/src/renderer/lazy-find.ts (pure helpers exported for tests) wired from the App mount effect; two new direct deps already present transitively: @univerjs/find-replace, rxjs.

Known limitations (deliberate scope cuts, happy to iterate):

• Filtered-out rows are not excluded from out-of-window hits (the in-window scan still skips them natively).
• Extras refresh when a search starts; between searches only the inner list refreshes live.
• Replace on an unloaded plain-cell hit writes straight onto the cell; the edit journal replays it onto later viewport installs, so saves and AI reads stay correct.

Closes #113

• [x] npm run format:check
• [x] npm run lint — scoped ESLint run over all changed files: 0 errors (the 3 pre-existing react-hooks/exhaustive-deps warnings in App.tsx's untouched cleanup block remain warnings)
• [x] npm run typecheck
• [x] npm test — new suite apps/sheets/tests/lazy-find.test.ts (20 tests) passes, and the rest of the sheets suite passes except tests that require the Rust sidecar binary / long perf timeouts, which cannot run on this Windows machine (no MSVC toolchain for the cargo build; see below). Those code paths are untouched by this PR and are exercised by CI's Ubuntu job, which builds the sidecar before running vitest.

List any checks not run and explain why:

• Sidecar-dependent tests (xlsx-sidecar, xlsx-recalc, xlsx-streaming-save, one xlsx-borders case) fail locally with ENOENT ...xlsx-sidecar.exe because building the Rust sidecar here requires MSVC Build Tools that are not installed; cargo build fails at the linker step. No Rust code is modified by this PR, and CI builds the sidecar before running the suite.
• npm run licenses passes (the two newly declared deps were already in the tree and allowlisted).

Manual verification path for reviewers without a large fixture handy: open any .xlsx big enough to stream (> ~10k cells works), do not scroll, press Ctrl+F, and search for text that exists far below row 0 — before this PR it reports no matches; after, the count appears and Enter jumps to and selects the real cell.

Not applicable — no visible chrome changes; the difference is the Find dialog's match count/jump behavior on large streamed workbooks.

• [x] The change is focused and does not include unrelated reformatting or refactoring.
• [x] User-facing strings use the existing i18n resources (appFindScanTruncated added to all 19 locale blocks in strings-app.ts).
• [x] File open/save changes include an appropriate round-trip or fidelity test. — N/A: nothing in the open/save paths is touched; replacements go through the existing edit journal that save already replays.

• What changed? GenOffice Sheets gains an opt-in "reading mode": a translucent band covers the active cell's entire row and another covers its column, tracking the selection as you navigate. It is toggled from the View tab's Show group ("Highlight active row & column", next to Gridlines/Headings), the choice persists in localStorage like the auto-save flag, and it is off by default.
• Why is this change needed? Feature request tracked in #112: while working in wide sheets, it is easy to lose track of which row/column a cell belongs to. Excel-style cross-highlighting of the active row and column fixes that at a glance.

Implementation notes:

• The bands are float-DOM layers anchored to ranges — the same mechanism the page-break preview and formula-audit traces use — so they follow scroll and zoom with no extra rendering work. Row/column extents come from the file-backed sheet extent under lazy streaming (with getLastRow/getLastColumn fallback for demo workbooks), clamped to the same caps the page-break preview draws (20k rows / 2k columns) so huge sheets cannot freeze the grid.
• A single installer subscribes to SelectionChanged/ActiveSheetChanged, debounces moves (60 ms), and skips reinstallation when the active cell did not change; bands are disposed/re-added only when the cell actually moves. Everything tears down with the app's mount effect.
• Styling goes through new --sheets-crosshair-bg / --sheets-crosshair-line tokens defined in all three blocks in styles.css (:root, [data-theme='dark'], and the prefers-color-scheme fallback), per the theming rules; the bands themselves only reference tokens (no raw colors), and the layers are pass-through float DOM so clicks and edits land on the grid.
• User-facing strings (appCrossHighlight, appCrossHighlightOn, appCrossHighlightOff) added to all 19 locale blocks.

Closes #112

• [x] npm run format:check
• [x] npm run check:theme-colors — the only raw values added sit on token-definition lines
• [x] npm run check:english-comments
• [x] npm run lint — scoped ESLint run over all changed/new files: 0 errors
• [x] npm run typecheck
• [x] npm test — new suite apps/sheets/tests/cross-highlight.test.ts (5 tests) passes alongside the existing page-break preview suite

List any checks not run and explain why:

• The full sheets vitest suite was exercised on this machine earlier from an identical tree; tests that require the Rust sidecar binary cannot run locally here (no MSVC toolchain to link the cargo build). This PR touches renderer-only files and no Rust code; CI builds the sidecar before running the suite.

Not applicable — I could not capture Electron screenshots from this environment. Visual result when enabled: the active row shows a faint blue band across the full sheet width, the active column the same down its full height, both with a slightly stronger edge line toward the active cell; both adapt to dark mode via the new tokens. Reviewers can reproduce in one click from the View tab.

• [x] The change is focused and does not include unrelated reformatting or refactoring.
• [x] User-facing strings use the existing i18n resources.
• [x] File open/save changes include an appropriate round-trip or fidelity test. — N/A: display-only feature; nothing touches the open/save paths.

Why no screenshot: the strip renders identically before and after — the change is what survives a re-open (the buffer), not anything visible in a static frame. The one observable difference is a negative (an edit no longer disappears), which a screenshot cannot show; the vitest cases pin it.

Re-opening a file that is already open as a document tab silently discards its unsaved edits. A document tab's content field is the live editor buffer — MarkdownPanel writes edits back through onContentChange → patchTab({ content }) — but every file-open affordance (file chips, tool lines, the Files tab, the file picker) routes through handleFileOpen → openFile, which re-reads the file from disk and hands it to upsert. upsertInBucket merges onto an existing tab with a spread, so the disk bytes replaced the buffer. The edits were gone with no prompt and no undo; the close guard never fired because from the panel's perspective the buffer simply changed (fixes #1441).

This is silent, unrecoverable data loss on a mainstream gesture: clicking any second affordance for a file you are editing reverts your work. Editors conventionally keep the live buffer when a document is re-opened (the hook's own docstring already promises "opening a document that's already open focuses its tab instead of duplicating it" — the implementation just also replaced its content).

Telling "the user edited this tab" apart from "the file changed on disk" needs a baseline, so each file tab now carries one:

• PanelTab.savedContent records the on-disk bytes the buffer last matched.
• openFile compares the existing tab's buffer against that baseline. Dirty → upsert a patch that omits content/savedContent, so the spread refreshes everything around the buffer (focus, reveal target, slot, diff-mode preference) and keeps the text. Clean → take the fresh disk bytes and restamp the baseline, exactly as before.
• Every path that moves DISK truth into the buffer restamps the baseline in the same write: successful saves (handleFileSave), cold-tab hydration (success and its error placeholder — an unreadable-file message is not unsaved work, so the next click retries the read instead of "protecting" it), and the two disk-originated panel paths (file watch and Refresh), which MarkdownPanel now routes through a new optional onDiskContent callback that the side panel wires to a content+baseline patch. The panel also receives savedBaseline={tab.savedContent} so its dirty guard computes from the same truth.
• serializeBucket strips savedContent along with content: the baseline mirrors a file body ("can be MBs"), so persisting it would re-create exactly the localStorage-quota problem the strip exists to prevent. A restored tab without a baseline is dirty-by-default until hydration re-establishes both.

Alternatives considered: preserving the buffer unconditionally on every re-open needs no new state but makes re-open useless as an external-change refresh for clean tabs; keying off content !== diskBytes alone cannot distinguish an edited buffer from a stale one and would freeze stale buffers in place. The baseline gives both cases their right answer, and the panel already enforces the same contract elsewhere — its own Refresh control is disabled while dirty ("save or discard changes first").

Six cases in website/src/test/usePanelTabs.test.ts:

• Re-opening a path whose tab holds edits focuses the tab and keeps the buffer (baseline untouched).
• Re-opening a clean tab still refreshes content + baseline from disk.
• After a save stamps the baseline, a later open refreshes again.
• A buffered tab without a baseline is treated as dirty, not reverted.
• A disk-originated refresh (content + savedContent patched together) re-arms refresh-on-reopen.
• Persistence stays metadata-only: the localStorage payload carries neither content nor savedContent, while path survives.

The pre-existing dedupe test ("same path merges fresh content") still passes unchanged — it describes the clean path, which keeps its behaviour.

Windows 11 / Node 22:

cd website
npx vitest run src/test/usePanelTabs.test.ts src/test/MarkdownPanel.test.tsx
   → 69 passed
npm run typecheck                                → clean
npx eslint <five changed files>                  → 0 errors; warning count at the repo's pinned ceiling

The full frontend suite (npm run check) exceeds this machine's local time budget; CI runs it authoritatively on this PR.

N/A — see the no-visual-delta marker above.

Fixes #1441

• [x] Single commit with a Conventional Commits title (fix: ...)
• [x] Existing tests pass and new tests added for new functionality
• [x] Self-review completed; code follows project style guidelines
• [x] Documentation updated (if applicable) — N/A: no documented behaviour changes; the baseline contract is documented on the type and in place
• [x] No secrets, credentials, or internal references in the diff

RealLaunchEngine._allocate_port exists only to work around a constraint that no longer exists. It was written because the instance tunnel forced local_port == remote_port and hard-failed when that port was busy, so a cloud crew registered on the default dashboard port could never be connected. #5189 removed that constraint — the hub now allocates its local forward port independently — and the helper's own docstring conceded it "could simply pass the registry default". The deletion was asked for three times in #5189's First Principles review and deferred there only on blast-radius grounds (fixes #5253).

Leaving it costs every future launch a fresh, pointless port allocation on both ends of one crew, keeps an instances-registry read on the cloud provisioning path for information the registry no longer needs, and forces every reader to reconstruct the dead local==remote rule to understand two lines of kwargs. The hedge "still mildly useful" is not a requirement; this makes the removal owned.

With the tunnel's local port independent, a crew no longer needs a unique remote port: EC2 hosts are isolated per crew, so sharing the stock remote port cannot collide across crews, and within one host there is exactly one gateway. Both ends therefore take their own defaults, which already agree:

• provision() calls ec2.deploy(...) with no dashboard_port override → the CloudFormation stack binds its DashboardPort default (5476, per cloud/templates/kirocrew-ec2.yaml).
• register() calls register_instance(...) with no remote_port → its signature default DEFAULT_REMOTE_DASHBOARD_PORT = 5476 (cloud/connect.py), the same number.
• _allocate_port, the memoised self._port, and the now-unused PortAllocator / InstancesRegistry imports are deleted.

Alternatives: keeping the helper "just in case" was the exact state being corrected; making the default explicit by passing constants at both call sites would duplicate two spellings of one fact instead of deleting them.

TestRealEngineGatewayPort in test/test_cloud_launch_job.py is rewritten to pin the new contract: after provision + register, ec2.deploy received no dashboard_port key and register_instance received no remote_port key — so any reintroduced allocation step turns the suite red. The three tests that existed to pin the old allocation behaviour (same port both ends, skip registry ports, survive an unreadable registry) are deleted along with the behaviour they described.

Full local run of the touched surface, Windows 11 / Python 3.12:

pytest test/test_cloud_launch_job.py test/test_cloud_cli.py test/test_cloud_wizard.py \
       test/test_cloud_login.py test/test_cloud_connect.py
→ 142 passed, 3 skipped

Plus flake8, isort, mypy clean on the changed module. Both changed files sit in .github/black-baseline.txt; their pre-existing formatting state is untouched and the diff adds no new black findings.

Fixes #5253

• [x] Single commit with a Conventional Commits title (refactor: ...)
• [x] Existing tests pass and new tests added for new functionality
• [x] Self-review completed; code follows project style guidelines
• [x] Documentation updated (if applicable) — N/A: no documented behaviour changes; the helper's docstring left with it
• [x] No secrets, credentials, or internal references in the diff

Code Review Sage's test suite is collected nowhere on Windows: tests/conftest.py sets collect_ignore_glob = ["*"] for the whole directory, so the platform the app is being brought up on has zero automated coverage of it (#4988). The gate predates the app's Windows support work and conflates the app-level refusal in sage_lib/discovery.py's gh_bin() (its review prompts still name python3 — untouched here, tracked separately) with three harness details the tests themselves can express.

The coverage hole sits exactly where recent risk was: a Windows-only silent failure in the review worker had to be found by hand because no Windows shard ran a single sage test. Every PR that touches this app lands blind on the platform it now claims to support.

Lift the collection gate and give each platform-dependent test the guard its failure actually needs, instead of one blanket skip:

• Symlink plants (20 of 21 failures, all OSError: [WinError 1314]). The suite already had the right pattern — SYMLINKS_OK, a probe for unprivileged symlink creation, defined privately in test_followup.py. That probe now lives in tests/fixtures.py, the suite's shared module, and every test that stages a planted link skips only where creating a symlink needs a privilege the host does not grant (Developer Mode / elevated runners run them again). The no-follow guards they pin keep running everywhere else.
• Owner-only mode bits. The report-output privacy test asserted 0600 through st_mode, which Windows never reports (the lockdown there is an ACL). That assertion is scoped to POSIX while file-presence and temp-file-cleanup checks still run on every platform; the two existing skipUnless(platform_compat.IS_POSIX) sites are unchanged.
• Collection gate. Removed; the conftest keeps only its SEL-muting fixture.

Alternatives weighed: keeping the blanket gate until #4979 lands (leaves the coverage hole open longer), or making the mode assertions ACL-aware instead of POSIX-gated (no cross-platform "verify owner-only DACL" helper exists yet; that would be a new production-side seam, out of scope for a test-enablement change).

Test-only change; the diff modifies how tests are gated, not what they assert:

• 17 planted-symlink test sites now carry @unittest.skipUnless(SYMLINKS_OK, ...) with the suite's established reason string.
• test_outputs_are_private_and_leave_no_temp_behind pins its 0600 assertion behind platform_compat.IS_POSIX and keeps presence/cleanup assertions unconditional.
• test_followup.py imports the shared probe instead of defining its own copy (same semantics, one owner).

Measured locally on Windows 11 (Python 3.12, the CI pin set: pytest 9.0.3 / xdist 3.5.0 / pytest-timeout 2.2.0):

Gates: flake8 clean, isort --check-only clean, mypy clean over the suite's 30 files, git diff --check clean. The five touched files sit in .github/black-baseline.txt; their formatting state is unchanged and the diff adds no new black findings. N/A for browser/UI checks — nothing user-visible moves.

Fixes #4988

• [x] Single commit with a Conventional Commits title (test: ...)
• [x] Existing tests pass and new tests added for new functionality
• [x] Self-review completed; code follows project style guidelines
• [x] Documentation updated (if applicable) — N/A: test-gating only, no documented behaviour changes
• [x] No secrets, credentials, or internal references in the diff

install.ps1 no longer crashes at line 367 on fresh Windows installs when the caller's PowerShell session has StrictMode enabled. $script:LastResolver was only assigned on the rare 8.3-short-path branch; every ordinary machine reached the resolved-path report with the variable unset → fatal InvalidOperation before any install stage ran.

Salvage of #93020 by @liuhao1024 (first-filed, primary credit; authorship preserved) + the 'skipped-long-path' diagnostics hunk from #93100 (Co-authored-by: @aniruddhaadak80). Closes #93017.

• scripts/install.ps1: initialize $script:LastResolver = 'none' before the report; early long-path return now records 'skipped-long-path' so the report distinguishes "skipped" from "never ran"
• tests/test_install_ps1_resolver_strictmode.py: 3 source-contract tests (init exists, init precedes resolver-run and report-read, early-return pinned)

Live repro on real PowerShell (portable 7.4.6), not simulated.

Auto-compaction no longer disables itself permanently on sessions that start too short to compress. The anti-thrash breaker counted structural no-ops (insufficient_messages, no_compressible_window, empty_post_handoff_window) as ineffective-compression strikes; two such no-ops latched the ≥2 breaker for the life of the session, so compaction never ran even after the session grew — context ballooned and every turn got more expensive.

Salvage of #93093 by @aniruddhaadak80 (authorship preserved). Closes #93022.

• agent/context_compressor.py: structural no-ops arm a transient in-memory 300s backoff (structural_backoff:<s> block reason) instead of durable strikes; cleared on /compress (force), completed compaction, and session reset. Genuine attempted-but-underperformed verdicts still strike.
• Tests: new test_context_compressor_structural_backoff.py + 4 existing anti-thrash test files aligned to the new contract

Preserves #40803's anti-refire guarantee (same 300s cadence as the existing recovery probe). Note: overlaps open #88388 (insufficient_messages prune-first) — that PR will need a small rebase of one hunk if salvaged later.

Gemini CLI loads a project's .env file into process.env at startup (trusted workspaces load all keys), and its internal git operations are executed with an environment derived from process.env. The git-environment hardening that does exist (getSafeGitEnv() in packages/core/src/utils/gitUtils.ts, sanitizeEnvironment() usage in packages/core/src/services/gitService.ts, and the equivalent logic in shellExecutionService.prepareExecution) only neutralizes GIT_CONFIG_* / GIT_CONFIG_PARAMETERS. It does not touch the other GIT_* variables that change which binaries and helpers git executes.

As a result, a malicious-but-trusted repository can ship a .env containing, for example:

GIT_EXEC_PATH=C:\Users\victim\AppData\Local\Temp\evil
GIT_SSH_COMMAND=calc.exe
GIT_PROXY_COMMAND=cmd /c calc.exe

and, merely by starting Gemini CLI inside that repository (folder-trusted, e.g. once via the trust prompt or when folder trust is disabled), trigger attacker-controlled code through completely ordinary, non-model, pre-approval git calls such as:

• checkpointing initialization — GitService.initialize() → spawnAsync('git', ['--version'], { env: getSafeGitEnv() }) and subsequent shadow-repo commits (git resolves subcommand binaries from GIT_EXEC_PATH; core.hooksPath is already neutralized, but these env vectors are not)
• extension install/update — cloneFromGit() uses simple-git with getSafeGitEnv(), where git.fetch/clone will invoke GIT_SSH_COMMAND/GIT_PROXY_COMMAND for non-https remotes
• memory/git integration paths using getAbsoluteGitDir()

The codebase demonstrates awareness of this exact risk class: DEFAULT_EXCLUDED_ENV_VARS blocks GEMINI_CLI_IDE_SERVER_STDIO_COMMAND/_ARGS from project env files precisely because they name executables — but no equivalent protection exists for git's executable/helper env vars.

Environment sanitization for internal git invocations should also strip or pin execution-affecting variables, e.g.:

• In getSafeGitEnv() (and the parallel logic in gitService.getShadowRepoEnv() / shellExecutionService.prepareExecution): delete GIT_EXEC_PATH, GIT_PROXY_COMMAND, GIT_SSH_COMMAND, GIT_SSH_VARIANT, GIT_ALTERNATE_OBJECT_DIRECTORIES, GIT_TEMPLATE_DIR, GIT_REPLACE_REF_BASE, GIT_CEILING_DIRECTORIES (as applicable), rather than only GIT_CONFIG_*.
• Alternatively/additionally, extend DEFAULT_EXCLUDED_ENV_VARS so project .env files cannot introduce these variables at all.

This restores the same isolation intent that already exists for git config (credential.helper, core.hooksPath, etc.) but which currently stops one layer short.

Source-level finding verified against upstream main at commit 5411f113c. All platforms; requires a trusted workspace whose .env sets the offending variables (untrusted workspaces are protected because loadEnvironment() whitelists only GEMINI_API_KEY, GOOGLE_API_KEY, GOOGLE_CLOUD_PROJECT, GOOGLE_CLOUD_LOCATION).

Not applicable.

Sources:

• packages/cli/src/config/settings.ts:693-726 — trusted workspace .env values are loaded into process.env unless in DEFAULT_EXCLUDED_ENV_VARS
• packages/cli/src/config/settings.ts:82-87 — DEFAULT_EXCLUDED_ENV_VARS contains only 4 entries; no GIT_*
• packages/core/src/utils/gitUtils.ts:9-45 — getSafeGitEnv() strips only GIT_CONFIG_*/GIT_CONFIG_PARAMETERS
• packages/core/src/services/gitService.ts:102-124 — shadow-repo env built from sanitized process.env; execution-affecting GIT_* survive
• packages/cli/src/config/extensions/github.ts:37 — extension clones run with getSafeGitEnv()
• Related but distinct: #28684 (env-var load-order race affecting settings resolution) — different defect.
• Duplicate check: searched issues/PRs for "GIT_EXEC_PATH", "GIT_SSH_COMMAND", "safe git env" — no existing report or fix found.

In the legacy ingestion path, when usage.total is explicitly provided as 0, the || operator treats it as falsy and overwrites it with a computed total derived from input + output. This silently corrupts the stored usage data.

worker/src/services/IngestionService/index.ts, lines 1924-1932:

const newTotalCount =
  ("usage" in obs.body ? obs.body.usage?.total : undefined) ||
  (Object.keys(
    "usageDetails" in obs.body ? (obs.body.usageDetails ?? {}) : {},
  ).length === 0
    ? newInputCount && newOutputCount
      ? newInputCount + newOutputCount
      : (newInputCount ?? newOutputCount)
    : undefined);

Send an ingestion event with:

{
  "type": "generation-create",
  "id": "...",
  "traceId": "...",
  "usage": { "input": 100, "output": 50, "total": 0 }
}

Expected: provided_usage_details.total = 0 (the caller explicitly stated 0 tokens) Actual: provided_usage_details.total = 150 (computed from input + output because 0 || ... is falsy)

JavaScript's || operator short-circuits on any falsy value, including 0, false, and "". Since usage.total can legitimately be 0 (e.g., cached responses where no tokens are counted, or when the caller intentionally reports 0), the || should be replaced with ?? (nullish coalescing) which only short-circuits on null or undefined.

• Data corruption: Stored provided_usage_details.total and usage_details.total will be wrong for any SDK that sends total: 0
• Dashboard inaccuracy: "Total Usage" metrics, cost calculations, and export data will be inflated
• Cascading: Cost details derived from usage details will also be incorrect since they depend on the stored total

Note: the newInputCount && newOutputCount expression on line 1929 has a similar issue -- if input: 0 is sent without output, the expression evaluates to 0 (falsy) and falls through to newInputCount ?? newOutputCount. However, this branch only executes when total is not provided AND usageDetails is empty, so it is a less likely scenario. The primary bug is the usage.total overwriting.

Replace || with ?? on line 1925:

const newTotalCount =
  ("usage" in obs.body ? obs.body.usage?.total : undefined) ??
  (Object.keys(
    "usageDetails" in obs.body ? (obs.body.usageDetails ?? {}) : {},
  ).length === 0
    ? newInputCount && newOutputCount
      ? newInputCount + newOutputCount
      : (newInputCount ?? newOutputCount)
    : undefined);

This preserves explicit 0 values while still falling through to the computed total when total is undefined or null.

• Whether this is the only place in the ingestion pipeline where || is used on numeric usage fields -- check if newInputCount or newOutputCount have the same truthiness issue elsewhere in the merge step
• Whether the newInputCount && newOutputCount fallback (line 1929) should also be fixed -- 0 && ... evaluates to 0 which is falsy, so it falls through to ??. This is technically correct for the fallback path but may warrant explicit != null checks for clarity
• Whether any SDKs currently send total: 0 -- if none do, this is a latent bug waiting to happen rather than an actively exploited one

@-mentions in comments are silently dropped when the mentioned user's display name contains square brackets, e.g. John Doe[ Platform Team ] (a common SSO/IdP display-name format). The mention UI inserts the raw name into the token:

@[John Doe[ Platform Team ]](user:cmr9klx3v0005434tzy5d86dq)

but MENTION_REGEX in web/src/features/comments/lib/mentionParser.ts captured display names with [^[\]]{1,100}, which can never match a bracketed name. Result: extractUniqueMentionedUserIds() returns nothing, validMentionedUserIds ends up empty, and no COMMENT_MENTION job is enqueued — no email, no log line, nothing. The worker's email-preview stripping (@\[([^\]]+)\]\(user:[^)]+\)) had the same bracket-hostile pattern.

Fixes #14836

The userId — not the display name — is the authoritative part of a mention, so both patterns now anchor a bounded lazy capture on the literal ](user: suffix instead of excluding brackets from the name:

@\[(.{1,100}?)\]\(user:([a-z0-9_-]{1,30})\)

• User-ID charset/length bounds are unchanged; invalid user IDs still never match.
• ReDoS safety is preserved: a single bounded lazy quantifier anchored on a literal suffix has no catastrophic-backtracking path. The existing timing-based tests (1000-char names, repeated brackets, pathological inputs) all pass.
• Worker preview building is extracted into an exported pure buildCommentPreview() (same file, no behavior change beyond the regex) so it can be unit-tested directly.

• Extended web/src/features/comments/lib/mentionParser.clienttest.ts: flipped the two cases that codified the old bracket-hostile behavior ("nested brackets" now match; "many repeated brackets" resolves to the mention's user), and added positive extract + sanitize coverage for names like Jane Doe[ Platform Team ].
  • pnpm --filter web run test-client src/features/comments/lib/mentionParser.clienttest.ts → Tests 57 passed (57)
• Added worker/src/__tests__/comment-mention-preview.test.ts covering regular names, bracketed names, truncation and plain text.
  • pnpm --filter worker run test src/__tests__/comment-mention-preview.test.ts → Tests 4 passed (4)
• Targeted ESLint clean on all touched files (web exit 0, worker exit 0); pnpm --filter web run typecheck → exit 0; prettier applied to every changed file.

• Mention extraction happens at comment creation, so comments saved before this fix don't retroactively notify anyone; new comments work immediately. Historical tokens remain in stored content either way.
• Rendering of mentions goes through the CommonMark link parser in MarkdownViewer; bracketed names already degraded to plain text there before this change and continue to do so. This PR restores notifications (the reported bug) without touching the renderer.
• One thing worth a second look: with the widened pattern, input like @[[[[[[[Alice](user:alice123) now resolves to the mention's user (previously ignored). I'd argue that is correct under "userId is authoritative" — the sanitizer rewrites it to the canonical name — but it is a deliberate behavior change called out by an updated test.

sequenceDiagram
  participant U as Comment author
  participant W as Web comment router
  participant P as Mention parser
  participant DB as Postgres
  participant Q as Notification queue
  participant N as Worker
  U->>W: Submit comment content
  W->>P: Extract and sanitize mentions
  P-->>W: Sanitized content and valid user IDs
  W->>DB: Persist sanitized comment
  W->>Q: Enqueue COMMENT_MENTION
  Q->>N: Process notification
  N->>DB: Re-fetch comment and membership
  N->>N: Build email preview
  N-->>U: Send mention email

### Issue 1
web/src/features/comments/lib/mentionParser.ts:22
**Mention matching crosses token boundaries**

When a malformed mention with an invalid user ID precedes a valid mention within 100 characters, the lazy display-name capture expands through the later token and `sanitizeMentions` replaces the entire merged span, silently deleting intervening user-authored text from the persisted comment.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "fix(comments): parse mentions whose disp..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Context used:

• Knowledge Base — Notifications, Automations, and Outbound Integrations