Record that cloud_vm_sessions.attachment_count is cumulative

[teamleaderleo]

Problem

cloud_vm_sessions.attachment_count reads like the number of clients attached to a session right now. It is not. upsertVmSession inserts it at 1 and then only ever adds to it:

attachmentCount: sql`${cloudVmSessions.attachmentCount} + ${input.attachmentCount ?? 1}`,

Nothing decrements on detach, so the value never returns to zero. It is a lifetime attach counter.

The value is published verbatim as attachmentCount by GET /api/vm/[id]/sessions and decoded straight into VMCloudSession.attachmentCount on the app side. Nothing renders it today, so there is no user-visible bug to fix. The risk is the next reader: a Cloud sidebar or machine list that shows "attachments" would report a machine as having twelve people on it after one person attached twelve times, and that mistake would look correct in review because the field name says what the renderer assumed.

Change

Comments at the three places the value is defined, passed and published, saying it is cumulative and must not be presented as a live viewer count. No behavior change, no schema change, no migration.

Verification

Comments only, so there is nothing to execute that could change. Disclosing what I did not run: web/ has no installed dependencies in this checkout, so I did not run bun x tsc --noEmit or bun run lint:complexity locally; CI covers both, and a comment cannot affect either.

I also did not add a test locking the cumulative semantics. The nearest coverage is in web/tests/vm-workflows.test.ts, which asserts attachmentCount: 1 after a single attach and so would still pass if the + became a plain assignment. A lock test would need a second attach against the DB-backed harness, and there is no local Postgres here to get a red-then-green result from. Worth a follow-up by someone who can run that suite locally.

Changelog

none

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Documents that cloud_vm_sessions.attachment_count is a lifetime attach counter, not a live client count. upsertVmSession inserts it at 1 and only ever adds to it, and there is no detach writer in web/, so the value never returns to zero.

Adds comments at the four places the value is defined, passed, and published (including the Swift socket re-export), clarifying that the insert branch stores the value verbatim while the conflict branch adds to it, and warning readers not to render it as a live viewer count. No behavior change.

^{Written for commit 68f7b3e. Summary will update on new commits.}

Summary by CodeRabbit

• Documentation
  • Clarified that the session attachment count tracks cumulative attachments, not currently connected clients, and should be read alongside the last attachment time.
  • Documented that each session update adds to the running count, defaulting to one attachment.

[github-actions]

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

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 6156d0f4-3aee-439c-b2bb-05e8e86b6ee3

📥 Commits

Reviewing files that changed from the base of the PR and between 4a255fa and 68f7b3e.

📒 Files selected for processing (3)

• Packages/macOS/CmuxCloud/Sources/CmuxCloud/VMClient/VMClient.swift
• web/db/schema.ts
• web/services/vms/repository.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Comments clarify that attachmentCount is a cumulative session-lifetime count, not a count of currently connected clients. They document the count update behavior and identify lastAttachedAt as a recency field. Runtime behavior is unchanged.

Changes

Attachment count documentation

Layer / File(s)	Summary
Document attachment-count semantics web/db/schema.ts, web/services/vms/repository.ts, web/app/api/vm/[id]/sessions/route.ts, Packages/macOS/CmuxCloud/Sources/CmuxCloud/VMClient/VMClient.swift	Comments describe the cumulative count, how upsertVmSession adds to the count, and how lastAttachedAt indicates recency.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Change: Other

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to 68f7b

This documentation-only change does not indicate a user-facing regression or a remaining merge-blocking risk.

🚥 Pre-merge checks | ✅ 24 | ❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (24 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: documenting that cloud_vm_sessions.attachment_count is cumulative.
Description check	✅ Passed	The description explains the problem, documents the change, states verification limits, and includes a changelog entry. It uses Problem, Change, and Verification headings instead of the template headi…
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.
Cmux Cloud Persistent Session And Early Input	✅ Passed	PASS. The authoritative PR diff adds only comments and a documentation block in four files. It does not change Cloud terminal creation, transport allocation, readiness gates, input routing, snapshots,…
Cmux Swift Actor Isolation	✅ Passed	PASS: The only Swift change adds a documentation comment to the existing public struct VMCloudSession: Sendable. The diff does not change actor annotations, protocols, reference types, mutability, o…
Cmux Swift Blocking Runtime	✅ Passed	The Swift diff adds only a documentation comment to VMCloudSession.attachmentCount. It introduces no semaphore, blocking wait, sleep, delayed dispatch, polling, main-queue sync, or manual lock. The …
Cmux Browser Automation Off-Main	✅ Passed	PASS: The authoritative PR diff changes only comments in four files documenting cumulative attachment counts. All added lines are comments, and the patch contains no browser automation commands, WebKi…
Cmux Expensive Synchronous Load	✅ Passed	The only production Swift change is four documentation lines above VMCloudSession.attachmentCount in Packages/macOS/CmuxCloud/Sources/CmuxCloud/VMClient/VMClient.swift. The diff adds no load, file…
Cmux Cache Substitution Correctness	✅ Passed	PASS: The pull-request diff adds comments only: 4 Swift documentation lines, 1 TypeScript comment, 5 schema comments, and 5 repository type-documentation lines. It does not replace any authoritative r…
Cmux No Hacky Sleeps	✅ Passed	PASS. The PR adds only comments and documentation: 15 insertions and no deletions. The TypeScript changes add no sleeps, timers, polling, fixed delays, or wall-clock synchronization. The Swift change …
Cmux Algorithmic Complexity	✅ Passed	PASS. The authoritative PR diff changes four files with 15 insertions and no deletions. Every added line is a Swift or TypeScript comment/doc comment. The diff adds no loops, scans, sorting, filtering…
Cmux Swift Concurrency	✅ Passed	PASS. The only Swift change adds four documentation lines to VMCloudSession.attachmentCount. It adds no DispatchQueue, Combine, completion-handler API, or fire-and-forget Task pattern. The concu…
Cmux Swift @Concurrent	✅ Passed	PASS. The only Swift diff adds documentation to the synchronous VMCloudSession.attachmentCount property. It adds no async, nonisolated, @concurrent, actor isolation, or call-site changes. Ther…
Cmux Swift Package Boundaries	✅ Passed	PASS: The only Swift change adds a four-line documentation comment to the existing VMCloudSession.attachmentCount property in VMClient.swift. It introduces no domain logic, API behavior, or new ap…
Cmux Swiftpm Lockfiles	✅ Passed	PASS: The review-scoped diff changes only comments in one Swift source file and three TypeScript files. It does not change Package.swift, Package.resolved, .gitignore, workflow files, Xcode project pa…
Cmux Swift Logging	✅ Passed	The Swift diff only adds documentation above VMCloudSession.attachmentCount. It does not add or change print, debugPrint, dump, NSLog, file/stdout logging, Logger declarations, or diagnost…
Cmux User-Facing Error Privacy	✅ Passed	PASS: The PR adds only source comments and a Swift documentation comment. The API response mapping and runtime behavior are unchanged. No user-facing error, alert, command output, recovery copy, or er…
Cmux Full Internationalization	✅ Passed	PASS: The diff changes only developer-facing comments and Swift documentation comments in four source files. It adds no user-facing Swift text, localization keys, web UI/API copy, metadata, markdown, …
Cmux Swiftui State Layout	✅ Passed	PASS. The only Swift change adds documentation to VMCloudSession.attachmentCount. The changed declaration is an existing public struct VMCloudSession: Sendable, not a SwiftUI view or state owner. …
Cmux Architecture Rethink	✅ Passed	PASS. The Swift diff only adds a documentation comment to VMCloudSession.attachmentCount; it adds no sleeps, dispatch, polling, locks, observers, mutable state, duplicate wiring, or lifecycle owners…
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PASS: The only Swift change adds documentation to VMCloudSession.attachmentCount in VMClient.swift. It does not add or modify an NSWindow, NSPanel, NSWindowController, SwiftUI Window, or `…
Cmux Source Artifacts	✅ Passed	PASS. The diff changes only four existing hand-written source files: Swift, TypeScript route, schema, and repository declarations. All 15 added lines are documentation comments. No local output, gener…
Cmux No Test Or Debug Seam In Production Source	✅ Passed	PASS. The only changed Swift file is under Packages/macOS/CmuxCloud/Sources/ and the diff adds documentation comments only. It does not add a #if DEBUG or test-build extension, debug/test-named me…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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.}

[teamleaderleo]

Review subagent findings and how they were resolved, on head 68f7b3e6941.

Review: the central claim holds. The review traced every writer of the column and confirmed the two branches of one statement in web/services/vms/repository.ts are the only writers in the repo, that the single production caller (web/services/vms/workflows.ts:4063) always passes 1, and that no migration, backfill or script sets the column directly. It also confirmed nothing renders the value, and found two facts stronger than this PR assumed: nothing in the repo ever writes status = "closed" or a non-null closedAt, so the reopen path is unreachable today; and although rows are deleted on account deletion and by cascade from cloud_vms, a re-created row inserts at 1, not 0, so "never returns to zero" survives that too.

Fixed:

• web/services/vms/repository.ts: "Not a new value to store" was wrong for the insert branch, which stores the argument verbatim as the session's first count. The arithmetic agrees either way (0 + n = n), but a reader who checked the code would conclude the comment lied, which is the exact failure this PR exists to prevent. The note now describes both branches, and says to pass a positive count, since neither the number type nor the bare + in the SQL stops a negative argument from decrementing.
• web/db/schema.ts: "nothing decrements on detach" implied a detach handler exists that forgot to decrement. There is no session-detach writer in web/ at all, which is the actual reason the count only grows. Reworded.
• Packages/macOS/CmuxCloud/Sources/CmuxCloud/VMClient/VMClient.swift: the PR had left the highest-risk hop uncommented. Sources/Cloud/VMClientSocketCommands.swift:977 re-exports the value to socket clients as a bare attachment_count, which is where a consumer is most likely to read it as a live count. The note went on the VMCloudSession property that every Swift caller resolves to, so it covers the re-export and any future consumer. This is the one change that widens CI: it un-skips the macOS lane, which was skipped on the previous SHA.

Left:

• The test double at web/tests/vm-workflows.test.ts:7131 implements the newly documented contract as a store rather than an add (attachmentCount: session.attachmentCount ?? 1). The fake is stateless, so store and add are indistinguishable for a single attach, and changing it in a comments-only PR would be churn. It matters for the deferred two-attach test: against this fake that assertion passes at 1 whether the SQL adds or replaces, so the test would be green without testing anything. Whoever writes that test needs to fix the double first.
• No lock test added, for the reason already given in the PR body: a meaningful assertion needs a second attach against Postgres, and this checkout has neither docker nor psql. Disclosed rather than skipped quietly.

One naming hazard worth recording for the Cloud sidebar work: cmux-tui/crates/chatmux-relay/src/pty.rs:623 has an attachment_count() that is a live count. Same name, opposite meaning, different layer.

[github-actions]

Dogfood build of 68f7b3e6941dfd5c9a2dd79c710da9d0b11d777c

cmux DEV pr-15321-68f7b3e6.app

The link opens this exact commit in the cmux dev menu bar app. The build starts on each push and the page waits until it is ready; a newer push replaces it. It signs in against production, so Cloud or backend changes still need a tagged build with a development backend.

[teamleaderleo]

Merging on green under the standing rule for fix PRs (skip team review, dogfood, merge on green).

No fleet dogfood evidence on this one, and the reason is structural rather than a skipped step: #8029 turned off Vercel branch previews while keeping main deployments, so an unmerged change to a deployed worker or service has no preview URL an app build could talk to. A fleet build would exercise main, not this branch, so the clicks would prove nothing about the diff. The evidence here is the executed red/green plus the full check suite, recorded in the review comment above.

[github-actions]

Merge receipt for 68f7b3e694: every check was green at merge (32 verified; 21 skipped by policy). Full suite runs on main after merge.