Skip to content

chore: make the never-mutated socket payload a let - #12153

Closed
austinywang wants to merge 1 commit into
mainfrom
chore/terminalcontroller-payload-let
Closed

austinywang wants to merge 1 commit into
mainfrom
chore/terminalcontroller-payload-let

Conversation

@austinywang

@austinywang austinywang commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Main's full CI currently fails the Swift warning budget in tests-build-and-lag with +1 Sources/TerminalController.swift: variable 'payload' was never mutated; consider changing to 'let' constant (see https://github.com/manaflow-ai/cmux/actions/runs/34057995765 on main and the same finding on the #12137 branch). The declaration at line 8779 builds a dictionary and returns it unchanged, so this makes it a let. No budget file change.

Verification

  • One-line change; full ci.yml dispatched on this branch so the Validate Swift warning budget step can confirm the count is back under budget (link in the comments).
  • No user-facing strings changed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01H8Eci7rC11iSuCw4F4DE2g


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Changes the payload variable in TerminalController.swift from var to let to fix the failing Swift warning budget check. The value is returned unchanged, so there's no behavior change.

Written for commit 8288d60. Summary will update on new commits.

Review in cubic


Note

Low Risk
Single declaration change with no logic or API behavior change; only silences an unused-mutation compiler warning.

Overview
Fixes a Swift warning budget failure on main by declaring the success-path response dictionary as let instead of var in the browser keyboard replay handler inside TerminalController.swift.

When native keyboard replay delivers, the handler still returns the same workspace/surface IDs and v2 refs; nothing in that path mutates the dictionary after it is built.

Reviewed by Cursor Bugbot for commit 8288d60. Bugbot is set up for automated code reviews on this repo. Configure here.

@vercel

vercel Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
cmux166 Ready Ready Preview Sep 8, 2026 2:21pm UTC
cmux41 Ready Ready Preview Sep 8, 2026 2:21pm UTC

@coderabbitai

coderabbitai Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 22 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 49b6243b-dd20-4739-934e-f580925f2053

📥 Commits

Reviewing files that changed from the base of the PR and between 76802d5 and 8288d60.

📒 Files selected for processing (1)
  • Sources/TerminalController.swift

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

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

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@austinywang

Copy link
Copy Markdown
Contributor Author

Full ci.yml dispatched on this branch: https://github.com/manaflow-ai/cmux/actions/runs/34220164195 (the Validate Swift warning budget step in tests-build-and-lag is the check that matters here).

@austinywang

Copy link
Copy Markdown
Contributor Author

Full CI on this branch stops at web-typecheck (scripts/check-devbox-image-reachable.ts(148,17): error TS2339: Property 'main' does not exist on type 'ImportMeta'), a break on current main that #12154 fixes; linux-preflight then skips the macOS jobs, so the Validate Swift warning budget step has not run yet here. The change is a one-line var → let on a value returned unchanged; I will re-dispatch once #12154 lands so the budget step can confirm the count.

The Swift warning budget gate (tests-build-and-lag) fails on main with
"variable 'payload' was never mutated" for this declaration, which is
returned unchanged. Use let so the budget check passes again without
touching the budget file.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H8Eci7rC11iSuCw4F4DE2g
@austinywang
austinywang force-pushed the chore/terminalcontroller-payload-let branch from fbd79d9 to 8288d60 Compare September 8, 2026 11:29
@austinywang

Copy link
Copy Markdown
Contributor Author

#12154 landed; rebased onto main and re-dispatched full CI: https://github.com/manaflow-ai/cmux/actions/runs/34220955654

@austinywang

Copy link
Copy Markdown
Contributor Author

After rebasing onto main (post #12154), full CI reached the warning-budget step: https://github.com/manaflow-ai/cmux/actions/runs/34220955654. The TerminalController.swift payload finding is gone, which is what this PR fixes. The step is still red because four newer warnings landed on main since the budget was last refreshed, none in files this PR touches:

  • Sources/SessionIndexTableController.swift: call to main actor-isolated instance method reconcilePresentation(in:) in a synchronous nonisolated context
  • Sources/SessionIndexTableController.swift: main actor-isolated property isApplyingRows can not be referenced from a Sendable closure
  • Sources/Surfaces/CmuxTuiSnapshotParser.swift: default will never be executed
  • Sources/Surfaces/SurfaceCatalogModel.swift: immutable value rowID was never used

swift-package-tests also failed once on SSHPTYAttachRetryScriptBuilderTests.signalInterruptsReconnectBackoffPromptly (a timing-based package test unrelated to this change). Those belong to their own follow-ups; this PR stays a one-line let.

austinywang added a commit that referenced this pull request Sep 8, 2026
`tests-build-and-lag` fails "Validate Swift warning budget" on every branch
that merges current main: #10564 deleted 20 lines from
.github/swift-warning-budget.tsv, including the entries for five warnings that
still exist (AppDelegate+PaneMemoryGuardrail x2, SessionIndexTableController
x2, CmuxTuiSnapshotParser, SurfaceCatalogModel), and the browser keyboard
replay `payload` dictionary is declared `var` but never mutated.

Restore exactly those five budget lines and make `payload` a `let` (the same
change as #12153) so the check passes against the current build output.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hgp819aHrzykKJnpSvrWk2
@austinywang

Copy link
Copy Markdown
Contributor Author

Heads-up: #12168 fixes the remaining four post-refresh warnings you listed (SessionIndexTableController isolation ×2, CmuxTuiSnapshotParser unreachable default, SurfaceCatalogModel unused rowID) plus the CmuxTerminal test compile break (#12161), and includes this PR's let payload line byte-identically so the two merge cleanly in either order. If #12168 lands first this PR becomes a no-op; if this lands first, #12168 rebases trivially.

austinywang added a commit that referenced this pull request Sep 8, 2026
…-budget warnings (#12159), un-normalized pbxproj, stale hook-test expectations (#12177) (#12168)

* ci: unbreak main's macOS lane (#12161, #12159)

The CmuxTerminal test target no longer compiled after #10564 added a `UUID`
parameter to FakeTerminalEngine.swift, which imported only GhosttyKit; add
`import Foundation`. The "Validate Swift warning budget" step also failed on
five warnings that landed after the budget refresh: wrap the
boundsDidChangeNotification observer body (queue: .main) in
MainActor.assumeIsolated in SessionIndexTableController, drop the unreachable
`default` from the exhaustive `switch resourceID.kind` in
CmuxTuiSnapshotParser, stop binding an unused `rowID` in SurfaceCatalogModel,
and make the never-mutated `payload` in TerminalController a `let` (identical
to #12153).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MPAVb9SSqAnuUnPEFQguE9

* ci: normalize project.pbxproj so the workflow guard passes

main's pbxproj carries the StackAccountAvatarViewTests entries (#12145) out of
normalized order, so scripts/check-pbxproj.sh fails "Validate pbxproj
objectVersion pin and normalization" in workflow-guard-tests on every full CI
run. Output of scripts/normalize-pbxproj.py, no content change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MPAVb9SSqAnuUnPEFQguE9

* fix: parenthesize confusable trailing closures in the pane memory guardrail

The full CI run on this branch had one bucket left over the Swift warning
budget: two "trailing closure in this context is confusable with the body of
the statement" warnings in postAggregateMemoryPressureWarning's guard
condition. Pass the closures as parenthesized arguments, matching the
existing call later in the file. No behavior change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MPAVb9SSqAnuUnPEFQguE9

* test: assert journal pane targeting instead of the removed prompt/pre-tool clear (#12177)

#11976 routes attention through the journal reconciler and removed the explicit
`clear_notifications --tab --panel` from the Claude prompt-submit and
pre-tool-use hook paths; the app clears attention from the emitted
agent_journal_append event instead. Two ClaudeHookLifecycleCleanupTests still
asserted the old command and failed on every full CI run. Assert the new
contract: the agent.turn.started / agent.state.changed event names the
resolved (moved) pane, sibling and fallback panes are untouched, and no
workspace-wide clear is sent. Verified by replaying both hooks against a
post-#11976 CLI with a port of the mock socket server.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MPAVb9SSqAnuUnPEFQguE9

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@austinywang

Copy link
Copy Markdown
Contributor Author

main now contains this exact let payload change via #12168 (3297e5e), which also cleared the other over-budget warnings; this PR can be closed as superseded.

aerickson pushed a commit to aerickson/cmux that referenced this pull request Sep 13, 2026
…2161), over-budget warnings (manaflow-ai#12159), un-normalized pbxproj, stale hook-test expectations (manaflow-ai#12177) (manaflow-ai#12168)

* ci: unbreak main's macOS lane (manaflow-ai#12161, manaflow-ai#12159)

The CmuxTerminal test target no longer compiled after manaflow-ai#10564 added a `UUID`
parameter to FakeTerminalEngine.swift, which imported only GhosttyKit; add
`import Foundation`. The "Validate Swift warning budget" step also failed on
five warnings that landed after the budget refresh: wrap the
boundsDidChangeNotification observer body (queue: .main) in
MainActor.assumeIsolated in SessionIndexTableController, drop the unreachable
`default` from the exhaustive `switch resourceID.kind` in
CmuxTuiSnapshotParser, stop binding an unused `rowID` in SurfaceCatalogModel,
and make the never-mutated `payload` in TerminalController a `let` (identical
to manaflow-ai#12153).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MPAVb9SSqAnuUnPEFQguE9

* ci: normalize project.pbxproj so the workflow guard passes

main's pbxproj carries the StackAccountAvatarViewTests entries (manaflow-ai#12145) out of
normalized order, so scripts/check-pbxproj.sh fails "Validate pbxproj
objectVersion pin and normalization" in workflow-guard-tests on every full CI
run. Output of scripts/normalize-pbxproj.py, no content change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MPAVb9SSqAnuUnPEFQguE9

* fix: parenthesize confusable trailing closures in the pane memory guardrail

The full CI run on this branch had one bucket left over the Swift warning
budget: two "trailing closure in this context is confusable with the body of
the statement" warnings in postAggregateMemoryPressureWarning's guard
condition. Pass the closures as parenthesized arguments, matching the
existing call later in the file. No behavior change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MPAVb9SSqAnuUnPEFQguE9

* test: assert journal pane targeting instead of the removed prompt/pre-tool clear (manaflow-ai#12177)

manaflow-ai#11976 routes attention through the journal reconciler and removed the explicit
`clear_notifications --tab --panel` from the Claude prompt-submit and
pre-tool-use hook paths; the app clears attention from the emitted
agent_journal_append event instead. Two ClaudeHookLifecycleCleanupTests still
asserted the old command and failed on every full CI run. Assert the new
contract: the agent.turn.started / agent.state.changed event names the
resolved (moved) pane, sibling and fallback panes are untouched, and no
workspace-wide clear is sent. Verified by replaying both hooks against a
post-manaflow-ai#11976 CLI with a port of the mock socket server.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MPAVb9SSqAnuUnPEFQguE9

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator

Closing as already on main: merging this branch into main at 8421357 produces main's own tree, so there's nothing left to land. The branch is kept; reopen if something here is still missing. Part of the backlog cleanup in manaflow-ai/cmuxterm-hq#563.

@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 24, 2026

This branch was successfully deployed

2 active deployments
Preview – cmux166 — 8288d605 Deployed Sep 8, 2026 by vercel[bot]
Preview – cmux41 — 8288d605 Deployed Sep 8, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants