[gdamore]

Summary

• mark terminfo screens finished before shutdown begins so existing post-Fini() guards become effective
• prevent the remaining public terminal-mutating methods from changing state or emitting output after Fini()
• add regression coverage for both the original shutdown flag bug and the broader post-shutdown no-op behavior

Validation

• go test ./...
• go test -race ./...

Fixes #1091

Summary by CodeRabbit

• Bug Fixes

  • Operations after screen shutdown are now reliably blocked from changing state or emitting output, including cursor/display, mouse, paste, focus reporting, resize, suspend/resume, beep, window title/clipboard, notifications, and style updates — preventing post-shutdown side effects.

• Tests

  • Added tests verifying shutdown prevents further terminal mutations and that prior terminal output and internal state remain unchanged after finalization.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7022e286-3390-41b0-ac8a-a9b4193c2394

📥 Commits

Reviewing files that changed from the base of the PR and between 03d76ef and 92b900f.

📒 Files selected for processing (2)

• tscreen.go
• tscreen_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• tscreen_test.go

---

📝 Walkthrough

Walkthrough

tScreen.finish() now sets the fini flag under the screen lock before shutdown. Public methods across cursor/display, input modes, sizing, suspend/resume, beep, and terminal controls now check t.fini under lock and return early to avoid terminal I/O or state mutations after finalization. Tests exercise and assert these post-finalization guards.

Changes

Post-finalization Shutdown Semantics

Layer / File(s)	Summary
Finalization flag initialization tscreen.go	tScreen.finish() sets t.fini = true under lock before closing quit and running finalization.
Guarding public methods against post-finalization calls tscreen.go	Cursor operations (ShowCursor, SetCursor), mouse/paste/focus toggles (EnableMouse, DisableMouse, EnablePaste, DisablePaste, EnableFocus, DisableFocus), sizing (SetSize), user feedback (Beep), and terminal control (SetTitle, SetClipboard, GetClipboard, ShowNotification) now acquire the lock and return early when t.fini is set, preventing I/O/state changes after Fini.
Suspend/Resume and engage/disengage refactor tscreen.go	Suspend()/Resume() lock first and use new locked helpers; disengage() was split into a locked start step and an unlocked disengageFinish() to complete terminal restoration after background loops stop.
Tests for post-finalization behavior tscreen_test.go	TestFiniPreventsSetStyleMutation and TestFiniPreventsFurtherTerminalMutation verify that after Fini(), style mutations and a broad set of terminal operations produce no internal state changes or terminal output.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• gdamore/tcell#1026: Modifies tScreen shutdown sequencing and Fini() behavior; overlaps with finalization flow changes.
• gdamore/tcell#1109: Also adjusts SetSize locking behavior; overlaps with resizing/locking fixes.
• gdamore/tcell#909: Changes engage() wiring and terminal lifecycle logic; related to engage/disengage refactor.

Poem

🐰 I set the fini flag with care,
Locks hush methods that would still dare,
No cursor blink or title shout,
The terminal sleeps, all outputs out—
Tests nod: the screen stays fair.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and accurately summarizes the main change: preventing terminal mutations after the screen is finalized.
Linked Issues check	✅ Passed	The PR fully addresses all objectives from #1091: sets t.fini=true under lock, audits and guards terminal-mutating methods, and adds regression tests.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to #1091 requirements—setting fini flag, adding guards to terminal-mutating methods, and adding regression tests.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch gdamore/check-issue-1091-report

---

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 and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 91.17647% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.05%. Comparing base (d7fff26) to head (92b900f).

Files with missing lines	Patch %	Lines
tscreen.go	91.17%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1110      +/-   ##
==========================================
+ Coverage   88.48%   89.05%   +0.57%     
==========================================
  Files          47       47              
  Lines        5530     5593      +63     
==========================================
+ Hits         4893     4981      +88     
+ Misses        481      462      -19     
+ Partials      156      150       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@tscreen.go`:
- Around line 1288-1293: The code has a TOCTOU: Resume() and Beep() read t.fini
under t.Lock()/Unlock() and then perform terminal mutations after unlocking,
allowing a concurrent Fini() to flip state; fix by performing the
check-and-action while holding the same mutex (i.e., keep t.Lock() held while
verifying t.fini and doing the terminal mutation) or re-check t.fini after
reacquiring the lock before mutating state/output; update the implementations of
Resume, Beep (and the analogous block at the other location) to ensure the
terminal mutation is executed only while the mutex protects t.fini to prevent
races with Fini().

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5f26fb0a-77ca-4370-a603-d40525b50460

📥 Commits

Reviewing files that changed from the base of the PR and between b91ae6d and aba0772.

📒 Files selected for processing (2)

• tscreen.go
• tscreen_test.go