fix: Issues-tab status writeback (409/scope) + terminal reconciler silent-death modes#226
Conversation
The quiet-time screen reconciler (the convergence backstop for renderer/ broker divergence) had three ways to be disabled silently, exactly while corruption was on screen: - A persistent PTY/grid dims mismatch — the very state that creates stacked-frame corruption — skipped every check with no log, forever. Now escalates after 2 consecutive quiet checks via onPersistentDimsMismatch: invalidate the size-sync ack, refit, and re-assert the rendered grid as the PTY size (30s rate limit). - A stranded optimistic-echo prediction held the quiet gate shut permanently: the engine has no time-based expiry, so a keystroke the TUI swallows (or typed into a hung agent) wedges hasPredictions true. Now rolled back after 10s of output silence; only optimistic glyphs are erased, confirmed bytes untouched. - A rejecting snapshot IPC was an unhandled rejection per 4s cycle; the check loop now catches and rate-limit-logs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d-only /linear/states
Changing a Linear issue status from the Issues tab always failed with a
409 revision conflict: writeRemoteFile hardcoded baseRevision '0'
("file must not exist"), which is right for Slack writeback creates but
wrong for PATCH writebacks onto existing canonical files. Now reads the
file's current revision first ('0' only when the read 404s) and retries
exactly once if a concurrent sync bumps the revision mid-write.
The Issues tab also lists /linear/states for the status dropdown, which
the mount-scope check rejected on every load (integrations configure
mounts like /linear/issues; the states subtree is reference data, not a
configurable mount). Add a read-only carve-out mirroring the Slack DM
one: list/read allowed when a Linear integration is visible, writes
still rejected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR harddens the terminal reconciliation system against persistent dimension mismatches and stranded predictions, while introducing Linear integration remote-filesystem support with optimistic-concurrency write semantics and revision conflict recovery. ChangesLinear Integration Remote File Access & Optimistic-Concurrency Writes
Terminal Screen Convergence Hardening
Documentation & Test Synchronization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances terminal screen convergence and remote file integration handling. Key updates include implementing a persistent dimension mismatch escalation mechanism that forces a size resync, rolling back stranded optimistic-echo predictions after a timeout, and ensuring that errors during terminal reconciliation checks do not halt the polling loop. Additionally, the IntegrationsManager is updated to support optimistic concurrency by reading the current file revision before writing (with a single retry on conflict) and allowing read-only access to the /linear/states reference subtree when a Linear integration is active. The reviewer feedback suggests using optional chaining and nullish coalescing when reading the existing file revision to prevent potential runtime errors if the response is empty.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const existing = await client.readFile(handle.workspaceId, path) | ||
| baseRevision = existing.revision |
There was a problem hiding this comment.
To ensure robust defensive programming, consider using optional chaining and a nullish coalescing operator when accessing existing.revision. This prevents potential runtime TypeError crashes if the file read response is unexpectedly empty or null.
| const existing = await client.readFile(handle.workspaceId, path) | |
| baseRevision = existing.revision | |
| const existing = await client.readFile(handle.workspaceId, path) | |
| baseRevision = existing?.revision ?? '0' |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@node_modules`:
- Line 1: Remove the node_modules symbolic link from the commit and ensure
node_modules is ignored: delete the symlink entry (the tracked file named
"node_modules") from the index/commit (e.g., git rm --cached node_modules or
remove it in your staging UI) and commit that removal; then add "node_modules/"
to .gitignore (or confirm it's present) so it won't be re-added; verify this
symlink was unintentional and that dependencies are managed via
package.json/package-lock.json instead of committing node_modules.
In `@src/renderer/src/lib/terminal-reconciler.ts`:
- Around line 150-166: The code preserves dimsMismatchStreak across an early
return when plain.rows/cols differ from viewport, which lets a prior divergence
carry over and trigger a repair later; inside the mismatch branch (the block
that checks plain.rows !== viewport.rows || plain.cols !== viewport.cols) reset
dimsMismatchStreak to 0 before returning so the confirmation streak is cleared
whenever dimensions are incomparable, leaving the rest of the logic (the kick
logic using RECONCILE_DIMS_MISMATCH_CHECKS, lastDimsKickAt,
deps.onPersistentDimsMismatch, plain, viewport, now()) unchanged.
In `@src/renderer/src/lib/terminal-runtime-registry.ts`:
- Around line 379-389: The rollback timing currently uses sinceOutput =
Date.now() - lastOutputAt which incorrectly triggers rollback when lastOutputAt
is stale; change the logic to measure prediction age instead: compute the age
from a timestamp on the predictiveEcho (e.g., predictiveEcho.predictionCreatedAt
/ predictiveEcho.lastPredictionAt) such that const sincePrediction = Date.now()
- predictiveEcho.predictionCreatedAt and use sincePrediction <
STRANDED_PREDICTION_ROLLBACK_MS to gate the early return; if predictiveEcho
lacks a creation timestamp, add one when predictions are created and reference
that in the rollback check before calling predictiveEcho.rollback() and
predictiveEcho.hasPredictions.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c5f24ccd-30fa-4740-8979-90cd59b9eac7
📒 Files selected for processing (8)
AGENTS.mdnode_modulessrc/main/integration-remote-paths.tssrc/main/integrations.test.tssrc/main/integrations.tssrc/renderer/src/lib/terminal-reconciler.test.tssrc/renderer/src/lib/terminal-reconciler.tssrc/renderer/src/lib/terminal-runtime-registry.ts
| const sinceOutput = Date.now() - lastOutputAt | ||
| if (predictiveEcho?.hasPredictions) { | ||
| if (sinceOutput < STRANDED_PREDICTION_ROLLBACK_MS) return false | ||
| // See STRANDED_PREDICTION_ROLLBACK_MS: the confirming echo is never | ||
| // coming; without this escape the quiet gate never reopens. | ||
| console.warn( | ||
| '[terminal] rolling back stranded optimistic-echo predictions (no confirming output)' | ||
| ) | ||
| predictiveEcho.rollback() | ||
| if (predictiveEcho.hasPredictions) return false | ||
| } |
There was a problem hiding this comment.
Stranded-prediction timeout is anchored to last server output, not prediction age.
At Line 379, sinceOutput is used to decide rollback timing. If lastOutputAt is old (or still 0), predictions can be rolled back on the first reconcile check instead of after a true 10s stranded period.
Suggested fix
let activitySerial = 0
let lastOutputAt = 0
+ let predictionsOutstandingSince = 0
let attachSeeded = false
@@
isQuiet: () => {
if (disposed || !attachSeeded || currentToken === null || !opened) return false
if (document.visibilityState !== 'visible') return false
- const sinceOutput = Date.now() - lastOutputAt
+ const nowTs = Date.now()
+ const sinceOutput = nowTs - lastOutputAt
if (predictiveEcho?.hasPredictions) {
- if (sinceOutput < STRANDED_PREDICTION_ROLLBACK_MS) return false
+ if (predictionsOutstandingSince === 0) predictionsOutstandingSince = nowTs
+ if (nowTs - predictionsOutstandingSince < STRANDED_PREDICTION_ROLLBACK_MS) return false
console.warn(
'[terminal] rolling back stranded optimistic-echo predictions (no confirming output)'
)
predictiveEcho.rollback()
if (predictiveEcho.hasPredictions) return false
}
+ predictionsOutstandingSince = 0
return sinceOutput >= RECONCILE_QUIET_MS
},Based on learnings: “Stranded predictions must roll back … after STRANDED_PREDICTION_ROLLBACK_MS of output silence.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sinceOutput = Date.now() - lastOutputAt | |
| if (predictiveEcho?.hasPredictions) { | |
| if (sinceOutput < STRANDED_PREDICTION_ROLLBACK_MS) return false | |
| // See STRANDED_PREDICTION_ROLLBACK_MS: the confirming echo is never | |
| // coming; without this escape the quiet gate never reopens. | |
| console.warn( | |
| '[terminal] rolling back stranded optimistic-echo predictions (no confirming output)' | |
| ) | |
| predictiveEcho.rollback() | |
| if (predictiveEcho.hasPredictions) return false | |
| } | |
| let activitySerial = 0 | |
| let lastOutputAt = 0 | |
| let predictionsOutstandingSince = 0 | |
| let attachSeeded = false | |
| // ... other code ... | |
| isQuiet: () => { | |
| if (disposed || !attachSeeded || currentToken === null || !opened) return false | |
| if (document.visibilityState !== 'visible') return false | |
| const nowTs = Date.now() | |
| const sinceOutput = nowTs - lastOutputAt | |
| if (predictiveEcho?.hasPredictions) { | |
| if (predictionsOutstandingSince === 0) predictionsOutstandingSince = nowTs | |
| if (nowTs - predictionsOutstandingSince < STRANDED_PREDICTION_ROLLBACK_MS) return false | |
| // See STRANDED_PREDICTION_ROLLBACK_MS: the confirming echo is never | |
| // coming; without this escape the quiet gate never reopens. | |
| console.warn( | |
| '[terminal] rolling back stranded optimistic-echo predictions (no confirming output)' | |
| ) | |
| predictiveEcho.rollback() | |
| if (predictiveEcho.hasPredictions) return false | |
| } | |
| predictionsOutstandingSince = 0 | |
| return sinceOutput >= RECONCILE_QUIET_MS | |
| } |
🤖 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 `@src/renderer/src/lib/terminal-runtime-registry.ts` around lines 379 - 389,
The rollback timing currently uses sinceOutput = Date.now() - lastOutputAt which
incorrectly triggers rollback when lastOutputAt is stale; change the logic to
measure prediction age instead: compute the age from a timestamp on the
predictiveEcho (e.g., predictiveEcho.predictionCreatedAt /
predictiveEcho.lastPredictionAt) such that const sincePrediction = Date.now() -
predictiveEcho.predictionCreatedAt and use sincePrediction <
STRANDED_PREDICTION_ROLLBACK_MS to gate the early return; if predictiveEcho
lacks a creation timestamp, add one when predictions are created and reference
that in the rollback check before calling predictiveEcho.rollback() and
predictiveEcho.hasPredictions.
Source: Learnings
|
Fixed one validated issue: the PR’s read-before-write path used the writer Relayfile handle, but that handle only requested write scope. It now requests read + write scope in src/main/integrations.ts, with the integration test updated in src/main/integrations.test.ts. Addressed comments
Advisory Notes
Local validation run:
Redraw timed out once when I ran it concurrently with fidelity; rerunning it alone, matching CI order, passed. |
|
Fixed one validated review issue in the PR scope: a dimensions-mismatch skip now resets the reconciler’s content mismatch confirmation streak, so a repair still requires fresh consecutive valid confirmations after dimensions agree again. Fix is in terminal-reconciler.ts, with regression coverage in terminal-reconciler.test.ts. Addressed Comments
Advisory Notes
Local validation passed:
Lint still reports existing warnings only, with zero errors. I did not use GitHub status/mergeability checks, so I’m not marking this READY. |
…iming flake The pacer drains via setTimeout(0); on Linux CI the assertion raced it. Matches the pattern used in the adjacent revision-dedup test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
khaliqgant
deleted the
fix/issues-writeback-revision-and-reconciler-gates
branch
1 participant
Summary
Two fix sets from today's debugging, verified independently of the in-flight cloud-auth work (built and tested from a clean
mainworktree).1. Issues tab: changing a Linear issue status always failed
revision_conflict:writeRemoteFilehardcodedbaseRevision: '0'("file must not exist") — correct for Slack writeback creates, wrong for the Issues tab's PATCH writeback onto the existing canonical issue file. It now reads the file's current revision first (keeping'0'only when the read 404s) and retries exactly once on a conflict; a second consecutive conflict propagates. The 429workspace_busyerrors were fallout from retrying the doomed write.list-remote-dirscope rejection on every Issues load: the status dropdown loads/linear/states, but Linear integrations configure mount scopes like/linear/issues— the states subtree is reference data, not a configurable mount, so the scope check rejected it and the UI silently fell back to issue-derived states. Added a read-only carve-out (isLinearStatesListablePath, mirroring the Slack-DM one): list/read allowed whenever a Linear integration is visible in the project; writes remain rejected.2. Terminal reconciler: three silent-death modes closed
The quiet-time convergence backstop (broker snapshot reconciler) could be disabled silently, exactly while corruption was visible:
AGENTS.md "Terminal Screen Convergence" documents the new invariants.
Test plan
vitest: integrations (52), integration-mounts (36), terminal-reconciler (incl. 4 new gate tests), terminal-runtime-registry.dom, echo-router, pty-size-sync — all pass in the clean worktreenpm run typecheck(web + node) clean{stateId}payload until Linear's webhook re-materializes the record); no[cloud-auth-diag] listRemoteDirectory REJECTfor/linear/statesin the main-process log🤖 Generated with Claude Code