review-bot-rules: no test/debug seam in production Swift Sources

.coderabbit.yaml

@@ -19,6 +19,9 @@ reviews:
     - path: "**/*.swift"
       instructions: |
         Apply the cmux custom Swift lint rules in `.github/review-bot-rules/` during review. Treat those files as the source of truth. Focus on production Swift changes and ignore test-only scaffolding unless it makes production behavior worse.
+    - path: "**/Sources/**/*.swift"
+      instructions: |
+        Apply `.github/review-bot-rules/no-test-debug-seam-in-production-source.md` during review. For Swift files under a production `Sources/` path (not under `Tests/`), flag added test-only or debug-only seams: `#if DEBUG` (or other test-build-guarded) extensions/members that expose internal state for tests or a debugger with no production caller, members named like `debug…`/`…ForTesting`/`…ForTests`/`testOnly…`/`…TestHook`/`…TestSeam`/`_test…`, or visibility widened plus a wrapper accessor added "so the test can call it". Prefer reaching internal state from the test target via `@testable import` after widening `private` to `internal`; isolate genuinely debug-only facilities in a dedicated debug file or folder. Pass for `#if DEBUG` blocks gating real product behavior and existing seams not worsened.
     - path: "**/Package.swift"
       instructions: |
         Apply the cmux custom Swift lint rules in `.github/review-bot-rules/`, especially the concurrency modernization, actor isolation, blocking runtime, file/package boundary, and architectural rethink rules.
@@ -132,3 +135,7 @@ reviews:
         mode: error
         instructions: |
           For every changed path, fail when the diff violates `.github/review-bot-rules/source-control-artifacts.md`: local tool output, generated logs, screenshots, recordings, temp folders, dependency checkouts, caches, build output, DerivedData, package-manager downloads, or broad scratch directories enter source control without a deliberate product, docs, fixture, build, release, or test-system reason. Pass for intentional source files, configs, localization catalogs, review rules, durable docs assets, required fixtures, and artifact removals or ignore-only cleanup.
+      - name: "cmux no test or debug seam in production source"
+        mode: error
+        instructions: |
+          For Swift files under a production `Sources/` path (matching `**/Sources/**` and not under `**/Tests/**`), fail when the diff violates `.github/review-bot-rules/no-test-debug-seam-in-production-source.md`: a `#if DEBUG` (or other test-build-guarded) extension/member that exposes internal state only for tests or a debugger with no production caller, a member named like `debug…`/`…ForTesting`/`…ForTests`/`testOnly…`/`…TestHook`/`…TestSeam`/`_test…`, or visibility widened together with a wrapper accessor added so a test can call it. Require moving test observation into the test target via `@testable import` after widening `private` to `internal`, or isolating a genuinely debug-only facility in a dedicated debug file or folder. The canonical fix is https://github.com/manaflow-ai/cmux/pull/6452. Pass for `#if DEBUG` blocks that gate real product behavior, scaffolding inside `Tests/` or a test-support module, and existing seams not worsened by the PR.

.github/review-bot-rules/README.md

@@ -11,6 +11,7 @@ Current rules:
 - `algorithmic-complexity.md`
 - `cache-substitution-correctness.md`
 - `full-internationalization.md`
+- `no-test-debug-seam-in-production-source.md`
 - `runtime-no-hacky-sleeps.md`
 - `source-control-artifacts.md`
 - `swiftpm-package-resolved.md`

.github/review-bot-rules/no-test-debug-seam-in-production-source.md

@@ -0,0 +1,27 @@
+# No Test or Debug Seam in Production Source
+
+Scope: production Swift source under `**/Sources/**` that is NOT under `**/Tests/**`. This is the production-source sibling of `test-determinism.md` (which governs test files) and complements `swift-logging.md` (which governs diagnostic output).
+
+A test-only or debug-only seam does not belong inline in production source. If a unit test needs to observe internal state, the test target reaches that state through `@testable import` after widening the declaration from `private` to `internal`. Production source should not grow accessors, hooks, or `#if DEBUG` extensions that exist solely to let tests or a debugger peek at otherwise-private state. The compiled-out `#if DEBUG` guard does not make this acceptable: it still adds a test-shaped surface to the shipping file, encodes the test's needs into production code, and invites more of the same.
+
+The canonical case is https://github.com/manaflow-ai/cmux/pull/6452: a `#if DEBUG` `debugQueuedRequestCount()` accessor was added to the production `MobileCoreRPCSession`/`MobileCoreRPCClient` sources so a test could read the actor's private writer-queue state. The correct fix removed the production seam entirely, widened the queue state `private` -> `internal`, and observed it from `CmuxMobileRPCTests` via `@testable import CmuxMobileRPC`.
+
+## Fail
+
+Flag a file under a production source path (`**/Sources/**`, not `**/Tests/**`) when the PR adds any of these:
+
+- A `#if DEBUG` (or `#if canImport(XCTest)` / `#if TESTING` / similar test-build guard) extension or member that exposes internal/private state for a test or debugger to read, with no production caller.
+- A function or property whose name signals a test/debug seam: `debug…`, `…ForTesting`, `…ForTests`, `testOnly…`, `…TestHook`, `…TestSeam`, `_test…`, or an accessor that exists only to surface otherwise-private state.
+- Widening visibility of production state to `public`/`internal` *and* adding a wrapper accessor in production source "so the test can call it" — when the test target could read the state directly via `@testable import` after a `private` -> `internal` widen.
+
+## Pass
+
+- The same observability achieved from the test target: state widened `private` -> `internal` (not `public` unless the public API genuinely needs it) and read through `@testable import`, with no test/debug accessor left in production source.
+- A genuinely debug-only facility that is unavoidable in production code (e.g. a Debug-menu action, an in-app debug overlay, or a developer diagnostic command that a real user path invokes) isolated in a dedicated debug file or folder, not inlined into the main production type.
+- Test scaffolding that lives in the test target (`**/Tests/**`), in a test helper module, or in a `Mocks`/`Testing` support target.
+- Existing test/debug seams the PR merely touches incidentally without adding new ones.
+- A `#if DEBUG` block that gates real product behavior (a developer-only feature, assertion, or logging), not a test-observability accessor.
+
+## Report
+
+When this rule fails, name the exact production source file and the test/debug member, state that a test-observability or debug seam was added to shipping source, and prescribe the smallest source-of-truth fix: move test scaffolding into the test target and reach internal state via `@testable import` (widening `private` -> `internal` as needed); if the facility is genuinely debug-only and unavoidable, isolate it in a dedicated debug file or folder. Cite https://github.com/manaflow-ai/cmux/pull/6452 as the reference fix.

.greptile/config.json

@@ -78,6 +78,12 @@
       "scope": ["**/Package.swift", "**/Package.resolved", "**/.gitignore", "cmux.xcodeproj/**", ".github/workflows/**"],
       "severity": "high"
     },
+    {
+      "id": "cmux-no-test-debug-seam-in-production-source",
+      "rule": "Flag Swift files under a production Sources path (matching **/Sources/** and not under **/Tests/**) that add a test-only or debug-only seam: a #if DEBUG (or other test-build-guarded) extension/member exposing internal state only for tests or a debugger with no production caller, a member named like debug…/…ForTesting/…ForTests/testOnly…/…TestHook/…TestSeam/_test…, or visibility widened together with a wrapper accessor added so a test can call it. Prefer observing internal state from the test target via @testable import after widening private to internal, or isolating a genuinely debug-only facility in a dedicated debug file or folder (canonical fix: cmux PR 6452). Pass for #if DEBUG blocks that gate real product behavior, scaffolding inside Tests/ or a test-support module, and existing seams not worsened by the PR.",
+      "scope": ["**/Sources/**/*.swift"],
+      "severity": "high"
+    },
     {
       "id": "cmux-source-control-artifacts",
       "rule": "Flag local tool output, generated logs, screenshots, recordings, temp folders, dependency checkouts, caches, build output, DerivedData, package-manager downloads, and broad scratch directories that enter source control without a deliberate product, docs, fixture, build, release, or test-system reason. Pass for intentional source files, configs, localization catalogs, review rules, durable docs assets, required fixtures, generated files that are already part of the repo's source-of-truth model, and PRs that only remove or ignore existing accidental artifacts.",

.greptile/files.json

@@ -55,6 +55,11 @@
       "description": "Source-of-truth cmux lint rule for SwiftUI state and layout review.",
       "scope": ["**/*.swift"]
     },
+    {
+      "path": ".github/review-bot-rules/no-test-debug-seam-in-production-source.md",
+      "description": "Source-of-truth cmux review rule against adding test-only or debug-only seams to production Swift Sources.",
+      "scope": ["**/Sources/**/*.swift"]
+    },
     {
       "path": ".github/review-bot-rules/source-control-artifacts.md",
       "description": "Source-of-truth cmux review rule for local/generated artifacts that should not enter source control.",

.greptile/rules.md

@@ -22,6 +22,7 @@ Review production Swift and runtime changes for:
 - Substituting a cached value for a fresh authoritative read in persistence/history/undo paths without handling cold and stale caches.
 - Local/generated artifacts, dependency checkouts, caches, logs, screenshots, temp folders, and scratch directories that accidentally enter source control.
 - SwiftPM dependency changes that ignore or omit cmux-owned `Package.resolved` lockfiles.
+- Test-only or debug-only seams added to production Swift `Sources/` that should live in the test target or a dedicated debug folder.
 
 ## Runtime No Hacky Sleeps
 
@@ -59,6 +60,16 @@ For every changed path, flag local tool output, generated logs, screenshots, rec
 
 Pass for intentional source files, configs, localization catalogs, review rules, durable docs assets, required fixtures, generated files that are already part of the repo's source-of-truth model, and PRs that only remove or ignore existing accidental artifacts.
 
+## No Test or Debug Seam in Production Source
+
+For Swift files under a production `Sources/` path (matching `**/Sources/**` and not under `**/Tests/**`), flag added test-only or debug-only seams.
+
+Fail a `#if DEBUG` (or other test-build-guarded) extension or member that exposes internal/private state for tests or a debugger with no production caller, a member named like `debug…`/`…ForTesting`/`…ForTests`/`testOnly…`/`…TestHook`/`…TestSeam`/`_test…`, or visibility widened together with a wrapper accessor added so a test can call it. The compiled-out `#if DEBUG` guard does not make a test-observability accessor acceptable in shipping source.
+
+Prefer observing internal state from the test target via `@testable import` after widening `private` to `internal`, or isolating a genuinely debug-only facility in a dedicated debug file or folder. The canonical fix is cmux PR https://github.com/manaflow-ai/cmux/pull/6452, which removed the `#if DEBUG debugQueuedRequestCount()` accessor, widened the queue state to `internal`, and read it from the test target.
+
+Pass for `#if DEBUG` blocks that gate real product behavior, scaffolding inside `Tests/` or a test-support module, and existing seams the PR does not introduce or worsen.
+
 ## SwiftPM Package.resolved
 
 For SwiftPM package, Xcode project, `.gitignore`, workflow, and dependency changes, flag cmux-owned package `.gitignore` files that ignore `Package.resolved`, external dependency resolution changes that omit the relevant package-local `Package.resolved` diff, or Xcode project package-reference changes that omit the root Xcode `Package.resolved` diff.