Skip to content

Fix Claude clear-session app-host fixture expectation - #13381

Closed
teamleaderleo wants to merge 8 commits into
mainfrom
test/app-host-claude-clear-ack-8565
Closed

teamleaderleo wants to merge 8 commits into
mainfrom
test/app-host-claude-clear-ack-8565

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fix one deterministic current-main app-host fixture failure from #8565.

testClaudeClearSessionStartMarksWorkspaceRunning still expects the pre-contract OK\n acknowledgment. The current CLI returns the canonical structured empty object {}\n, while the rest of the test continues to verify the pane-scoped notification clear and Running status update.

Red evidence

The same assertion failed independently in:

  • run 35530676667, app-host shard 1: CLINotifyProcessIntegrationRegressionTests.swift:335
  • run 35567420859, app-host shard 6: CLINotifyProcessIntegrationRegressionTests.swift:335

Current main 3cc2087b0f75d8204e0b5a92476cedc53a891c32 still contains the stale OK\n expectation.

Scope

Test fixture only; no product behavior changes. This isolates one repair that is also present in the broader open #13263 fixture bundle, so it can land and validate independently.


Summary by cubic

Fixes a deterministic app-host fixture failure from #8565 where testClaudeClearSessionStartMarksWorkspaceRunning expected the old OK\n acknowledgment. The CLI now returns the structured empty object {}\n, matching the machine-readable JSON contract documented in the test. Test fixture only; no product behavior changes.

Written for commit a47c782. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Tests
    • Updated integration test expectations to support the structured JSON acknowledgment returned by Claude session-start hooks.
    • Documented coverage confirming that clearing a session affects only the current pane and marks the workspace as running.
    • No user-facing behavior changes are introduced; this update keeps regression coverage aligned with the current hook response format.

@github-actions

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

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: 5ff58a0e-3d18-42f4-9416-b38045649e9f

📥 Commits

Reviewing files that changed from the base of the PR and between 61ae744 and a47c782.

📒 Files selected for processing (1)
  • cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift

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


📝 Walkthrough

Walkthrough

The integration regression test now expects the Claude session-start hook to print {}\n instead of OK\n. A comment documents the structured acknowledgement and related session behavior.

Changes

Claude hook contract

Layer / File(s) Summary
Session-start response assertion
cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift
The test documents the clear-session behavior and changes the expected Claude session-start output from OK\n to {}\n.

Priority: ⬇️ Low

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

Change: Other

Merge Risk: ⚪ Minimal · up to a47c7

This updates the Claude session-start regression test to expect the current {} acknowledgment without changing product behavior. The change is ready to merge.

🚥 Pre-merge checks | ✅ 23 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description explains the problem, expected behavior, evidence, and scope. It does not include the required Testing section or Checklist, and it does not state whether the Demo Video section is not… Add a Testing section with the tests and verification performed. Add the repository Checklist and mark each item accurately. State that a Demo Video is not applicable because this is a test-fixture-only change, if appropriate. Include the R…
✅ Passed checks (23 passed)
Check name Status Explanation
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 changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift: it updates a Claude hook test expectation from OK\\n to {}\\n and adds a comment. It does n…
Cmux Swift Actor Isolation ✅ Passed PASS: The authoritative PR diff changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. It adds a test documentation comment and changes the test expectation from "OK\\n" to `"{}\n…
Cmux Swift Blocking Runtime ✅ Passed PASS — The review-scoped diff changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. It updates a test assertion from "OK\\n" to "{}\\n" and adds comments. The diff introduces no…
Cmux Browser Automation Off-Main ✅ Passed PASS: The pull request changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift, outside the policy scope of TerminalController.swift and ControlCommandExecutionPolicy.swift. The …
Cmux Expensive Synchronous Load ✅ Passed The pull request changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. The diff adds test documentation and changes an expected CLI string from "OK\\n" to "{}\\n". It adds no pr…
Cmux Cache Substitution Correctness ✅ Passed PASS: The pull request changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. The diff adds a test comment and changes the expected hook output from OK\\n to {}\\n. It does not c…
Cmux No Hacky Sleeps ✅ Passed PASS. The reviewed range changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift, a Swift test fixture. It changes the expected acknowledgement from "OK\\n" to "{}\\n" and adds com…
Cmux Algorithmic Complexity ✅ Passed The pull request changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. It updates a test assertion from "OK\\n" to "{}\\n" and adds a test comment. The complexity policy explici…
Cmux Swift Concurrency ✅ Passed The diff changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. It adds documentation, a comment, and changes an XCTAssertEqual expected output from "OK\\n" to "{}\\n". The add…
Cmux Swift @Concurrent ✅ Passed PASS: The PR changes only a synchronous XCTest method's documentation, comment, and expected output string. The diff adds no async, nonisolated, @concurrent, actor-isolation, or async call-site …
Cmux Swift Package Boundaries ✅ Passed PASS. The authoritative diff changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. It updates a test expectation from OK\\n to {}\\n and adds test documentation. The change is t…
Cmux Swiftpm Lockfiles ✅ Passed PASS: The pull request changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. The patch updates a test expectation from "OK\\n" to "{}\\n" and adds comments. It does not change `…
Cmux Swift Logging ✅ Passed PASS. The PR changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift, a test fixture. The diff adds a doc comment, a test comment, and changes the expected stdout value from "OK\\n"…
Cmux User-Facing Error Privacy ✅ Passed PASS. The authoritative diff changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. It updates a test assertion from OK\\n to {}\\n and adds developer-only test comments. The gov…
Cmux Full Internationalization ✅ Passed PASS: The review-scoped diff changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. It updates a test expectation from OK\\n to the structured protocol acknowledgement {}\\n and …
Cmux Swiftui State Layout ✅ Passed PASS. The PR changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. The diff adds test documentation, changes an expected string from OK\\n to {}\\n, and adds a comment. It intro…
Cmux Architecture Rethink ✅ Passed PASS: The authoritative diff changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. It updates one XCTest expectation from "OK\\n" to "{}\\n" and adds comments. It introduces no …
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The authoritative diff changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift. It updates a test expectation from "OK\\n" to "{}\\n" and adds a test comment. It adds no `NSW…
Cmux Source Artifacts ✅ Passed PASS. The PR changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift, a hand-written Swift test source file. The diff adds test documentation, a contract comment, and changes the exp…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS. The authoritative diff changes only cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift, which is test code and is not a Swift file under a production Sources/ path. The changes add a…
Title check ✅ Passed The title clearly identifies the main change: updating the Claude clear-session app-host fixture expectation.
Full details: Description check

Explanation

The description explains the problem, expected behavior, evidence, and scope. It does not include the required Testing section or Checklist, and it does not state whether the Demo Video section is not applicable.

Resolution

Add a Testing section with the tests and verification performed. Add the repository Checklist and mark each item accurately. State that a Demo Video is not applicable because this is a test-fixture-only change, if appropriate. Include the Review Trigger block if required by repository process.

  • 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

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

@greptile-apps

greptile-apps Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge because it only aligns a stale test expectation with the existing CLI response contract.

Summary

Updates the Claude clear-session integration test to expect the CLI’s current machine-readable empty JSON response instead of the obsolete plain-text acknowledgment.

  • Changes only test fixture expectations.
  • Preserves assertions for pane-scoped notification clearing and workspace Running status.
  • Adds a clarifying comment describing the response contract.

Reviews (2) · Last reviewed commit: "test: document Claude session-start JSON..."

@teamleaderleo teamleaderleo added the full-ci EXPENSIVE: full macOS tests/builds; overrides selective PR routing. Not needed for normal checks. label Sep 21, 2026 — with ChatGPT Codex Connector

Copy link
Copy Markdown
Collaborator Author

Warning cleanup at exact head d764d18ddfef87d3e299e6300c48e1a9e577cd85.

Added an API-style doc comment to the single touched regression method so CodeRabbit's 0% docstring-coverage warning has a concrete fix without changing product behavior or the test assertion.

Current exact-head evidence:

  • Fast static checks: success
  • workflow guard tests/source lints: success
  • Swift package tests: success
  • CodeRabbit status: success
  • macOS compile admission: still running

This remains a test-fixture-only PR; no runtime behavior changed.

@cursor

cursor Bot commented Sep 22, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

Copy link
Copy Markdown
Collaborator Author

@greptileai review

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Superseded: this one-line change is included in #13263, and it conflicts with #13586's /clear behavior.

auto-merge was automatically disabled September 22, 2026 13:32

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full-ci EXPENSIVE: full macOS tests/builds; overrides selective PR routing. Not needed for normal checks.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant