Skip to content

[CI] Keep the structured-session compatibility manifest within the file limit - #26342

Merged
nwparker merged 2 commits into
mainfrom
nwparker/structured-surface-manifest-line-limit
Oct 7, 2026
Merged

nwparker merged 2 commits into
mainfrom
nwparker/structured-surface-manifest-line-limit

Conversation

@nwparker

@nwparker nwparker commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

A newly added cross-version method pushed the test manifest over the repository's file-length limit, blocking PR checks before tests could start. Put its existing method table in a separate file so checks can proceed.

What Changed

Move the existing constants and ordered method/reply table into structured-agent-session-surface-calls.ts. The original manifest imports the constants it uses and reexports every existing public name. Its operation counter, parameter builders, fingerprints and method coverage keep their original bodies and values. The new data module has no imports, avoiding a circular dependency.

Why

Keep the 300-line limit enforced while giving the growing table its own concrete home. Removing method coverage or raising the limit would hide the problem; changing test expectations would change the compatibility contract.

Linked Issue

Observed blocker: PR26256 static checks, on synthetic merge 66d855c2 with incoming main 995ef11ce77a69d324b6032495ccf5d4143cb910. The import PR does not touch this manifest.

Visual Proof

N/A — test metadata only; no visual or interaction change.

Testing

  • Locally verified formatting, targeted lint, changed-code checks, pnpm tc:node, and a focused no-emit check of both fixtures using the existing E2E TypeScript configuration on macOS with background launch policy.
  • Existing compatibility tests retained. No new mirror test: source audits verify every original declaration and literal, and CI exercised the existing callers.

Before/after measurements of the blocking gate:

Measurement Before After
Manifest effective code lines 304 (limit 300; CI rejects) At most 190 physical lines (local and CI lint pass)
Largest resulting file, physical lines 359 190
Targeted lint failures 1 in the preserved CI log 0 locally and in CI

The after line count is a conservative physical-line bound, rather than an invented effective-line measurement. This is a correctness repair, with no test-speed claim. Fresh corrected-head CI passes static analysis, all ten unit shards, relay integration and cross-version wire compatibility. Optional jobs skipped by the repository policy are recorded as skipped.

Root and independent source audits verify all declaration bodies byte for byte, all 152 string/numeric literals, all 16 public exports, local imports required by the remaining functions, and the complete ordered method table. Existing caller paths are retained. Full cross-version execution belongs in CI; no native app or integration suite was launched on the user's desktop.

Review

Independent refactor review found no changed functions, constants, wire names, counter ownership, export surface or caller loss. Root and independent source reviews cover corrected head dcdf11e3d99c2628fc8f91afd33e542cc515b159. All current-head checks pass or are intentionally skipped. The one actionable bot thread is fixed and resolved; the bot review itself was on the first head, not the corrected one.

Agent skill upstream boundary

  • Not applicable.

Notes

No production RPC registration, remote protocol, platform behavior or workspace ownership changes. Existing macOS/Linux/Windows and SSH compatibility coverage remains in place.

Checklist

  • Small and focused; before/after, mechanism and alternatives explained.
  • N/A visual proof explained.
  • Self-reviewed and independently audited for behavior preservation.
  • Cross-platform and SSH impact considered.
  • Corrected-head CI and independent source review complete; actionable bot findings resolved.

The first head f75c8f2 omitted the local THREAD import while reexporting it. The review bot correctly flagged it: declaration/literal equality alone did not verify local binding scope. The corrected head imports it locally. A new source check covers all six moved constants referenced by the retained code; no unresolved local reference remains. The focused E2E no-emit check passes. The initial source audits and CI epoch are retained; fresh corrected-head CI passes before merge.

Post-merge source audit: squash 7abd74e8187d39cbdc8ff127969c5cd384a2a20b has actual parent c785c986c20241dba27cff35c452ebaef6b8930b, after five concurrent upstream merges. Both changed fixtures are byte-identical to corrected shipping head dcdf11e3d99c2628fc8f91afd33e542cc515b159. The complete squash tree differs from the pre-merge CI tree; that CI is not claimed to cover the extra upstream changes. Concurrent commits: c785c98 feat(native-chat): teach chat agents to show inline visuals in their own folder (#26099); 2460068 feat(mobile): show a native chat's subagents like desktop (#26125); 16cd1e0 docs: remove stale internal reference documentation (#26328); b57bb7c fix(native-chat): show host outages in the notice card (#26161); 2803be7 [STA-6940] Deliver chat, workspace composer, and feedback file drops at their elements (3/6) (#25781).

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: fc6fb7e9-a2e7-4f2b-a34a-6b0e4c8746a1
📥 Commits

Reviewing files that changed from the base of the PR and between f75c8f2 and dcdf11e.

📒 Files selected for processing (1)
  • tests/e2e/cross-version-wire/structured-agent-session-surface-manifest.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/e2e/cross-version-wire/structured-agent-session-surface-manifest.ts

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


📝 Walkthrough

Walkthrough

The session fixtures and structured RPC method table move into structured-agent-session-surface-calls.ts. The manifest module imports and re-exports these values. The table records expected host methods and results, with no result field for two subscription methods.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to dcdf1

The extraction preserves the cross-version call contract and meets the file limit. No identified issue blocks merging after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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.
Title check ✅ Passed The title clearly states the primary change: keeping the structured-session compatibility manifest within the repository file-length limit.
Description check ✅ Passed The description is detailed and covers the change, rationale, testing, visual proof, review, compatibility impact, and checklist. The Linked Issue section does not provide an actual issue link or issu…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4a401de6-764c-4453-abdd-0842417a1ecf
📥 Commits

Reviewing files that changed from the base of the PR and between 995ef11 and f75c8f2.

📒 Files selected for processing (2)
  • tests/e2e/cross-version-wire/structured-agent-session-surface-calls.ts
  • tests/e2e/cross-version-wire/structured-agent-session-surface-manifest.ts

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

@nwparker
nwparker merged commit 7abd74e into main Oct 7, 2026
35 checks passed
@nwparker nwparker changed the title Keep the structured-session compatibility manifest within the file limit [CI] Keep the structured-session compatibility manifest within the file limit Oct 9, 2026
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