Skip to content

test(claude): fix queued-send cancellation CI expectations - #23675

Merged
nwparker merged 1 commit into
mainfrom
nwparker/fix-ci-09-28
Sep 28, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/fix-ci-09-28

Conversation

@nwparker

@nwparker nwparker commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​6 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3
Prod 0 0 0 0

ELI5

Unrelated pull requests were failing five Claude cancellation tests after recent changes merged. Update the tests to check the complete cancellation result so CI can pass again.

What Changed

Add the expected rejection: { kind: 'cancelled' } to the queued-send cancellation assertions, including the real-Claude check. Exact equality and the checks that only withdrawn messages settle remain intact. Application behavior is unchanged.

Why

The cancellation result intentionally gained a typed reason in #23116, but #23553 introduced expectations without it. Checking that field preserves the contract; dropping it from production or weakening the assertions would hide regressions.

Linked Issue

CI failures: https://github.com/stablyai/orca/actions/runs/36469406876 and https://github.com/stablyai/orca/actions/runs/36468659066.

Visual Proof

N/A — test expectations only.

Testing

  • Reproduced the five failing tests locally before the change.
  • All 31 targeted unit tests pass locally on macOS; full typecheck and changed-code quality checks pass.
  • The real-Claude cancellation check passes. Another local real-CLI test fails on project command catalog discovery; the complete CI suite will verify the fix.

Review

Self-reviewed: this adds an expected field without relaxing existing assertions.

Agent skill upstream boundary

  • Not applicable.

Notes

No platform, SSH, folder-workspace, or wire behavior changes.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • N/A with reason for visual proof
  • Self-reviewed for correctness, security, and performance
  • Cross-platform and SSH/remote impact considered
  • Full validation in CI

@coderabbitai

coderabbitai Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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 UI

Review profile: CHILL

Plan: Advanced

Run ID: 5012fe58-09dd-4014-b135-9aa8d0eaf2bd

📥 Commits

Reviewing files that changed from the base of the PR and between 3eac4d9 and 81748e5.

📒 Files selected for processing (3)
  • src/main/claude/claude-structured-control-actions.test.ts
  • src/main/claude/claude-structured-queued-stop.test.ts
  • src/main/claude/claude-structured-real-cli.test.ts

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


📝 Walkthrough

Walkthrough

Three cancellation test expectations now include rejection: { kind: 'cancelled' } alongside the existing rejection state or cancellation reason.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 81748

The application behavior is unchanged, and the tests now expect the structured cancellation result the producer emits. No merge-blocking risk is evident; normal CI checks remain appropriate.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: updating Claude queued-send cancellation test expectations.
Description check ✅ Passed The description is mostly complete. It explains the change, reason, testing, scope, and validation status. The Linked Issue section lists CI run links instead of an issue reference, and full CI valida…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
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.
✨ Finishing Touches
📝 Generate docstrings
  • 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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • claude-structured-control-actions.test.ts — adds rejection: { kind: 'cancelled' } to the settled.mock.calls exact-equality assertion so it matches the full settlement production emits.
  • claude-structured-queued-stop.test.ts — adds the field to the shared WITHDRAWN constant that four tests compare against with toEqual.
  • claude-structured-real-cli.test.ts — adds the field to the toContainEqual assertion on the real-CLI settlement.

The additions match production exactly: settleCancelledClaudeDispatchWaiters builds each settlement through claudeDispatchRejection(agentSessionFailureFact('cancelled')), which returns both the legacy marker reason and the typed rejection fact { kind: 'cancelled' }. The assertions remain exact-equality and are tightened, not relaxed. I ran the three changed suites (31/31 pass) plus the other suites touching this contract (194/194 pass), and the remaining cancellation assertions either use toMatchObject or already carry the field.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@nwparker
nwparker merged commit fe7e13d into main Sep 28, 2026
36 checks passed
Jinwoo-H added a commit that referenced this pull request Sep 28, 2026
…#23676)

* fix(mobile): truncate oversize markdown reads instead of failing them

The desktop bridge refused markdown over a private 512 KiB cap with
file_too_large, which reached the phone as a generic runtime_error that
the reader discarded, so a 632 KB file showed "Couldn't load markdown".
Reads now return a UTF-8-boundary prefix under one shared 2 MiB budget,
marked truncated with the full byteLength and read-only. The phone shows
the truncation like file tabs do and maps refusal codes to real copy, so
an older desktop's refusal reads "File too large for mobile preview".

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): pin that a truncated markdown read is never editable

The read budget sits above the edit budget, so every truncated document is
already read-only as file_too_large. Pin that ordering so a future budget
change cannot make a prefix editable, and name the constant as the markdown
preview budget, separate from the file preview's own.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): refuse saves from a truncated markdown read

A phone holding a truncated prefix must not write it back; the 256 KiB
save guard refuses it as file_too_large before any version check. The
shared budget test shrinks to the ordering it pins and names the bridge
test as the behavioural pin.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* refactor(mobile): tighten markdown truncation types and measure once

The read truncation measures the document once. The disk fallback drops
its truncated-only read-only text, which the status line never showed,
and both truncation fields are optional there. A markdown doc's flag is
only ever true, and the schema comment names the hook, not line numbers.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* docs(mobile): drop a stale disk-fallback comment

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* chore: retrigger CI after #23675 landed on main

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
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.

1 participant