test(ratchet): require src/main/provider-process now that it has landed

[brennanb2025]

	Files	Added	Deleted	Net
Test	1	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​4	$\color{#cf222e}{\Huge{\mathbf{−}}}$​3	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​1
Prod	1	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​3	$\color{#cf222e}{\Huge{\mathbf{−}}}$​3	0

ELI5

Main's test suite has one failing test since #24989 landed. The check that keeps Electron out of structured-chat code was told that src/main/provider-process/ might not exist yet, and its own test fails once the folder does exist, on purpose, so that someone flips the setting. #24989 added the folder, so this flips it.

What Changed

• config/scripts/check-runtime-electron-ratchet.mjs: src/main/provider-process is now a required lane. If it is ever renamed or removed, the check fails loudly instead of quietly checking nothing there. src/main/acp stays optional until it lands.
• config/scripts/check-runtime-electron-ratchet.test.mjs: the required-lanes fixture now includes a provider-process file (so the count is 6), and the comments now name only acp/ as optional.

There is no change for users. The folder's files were already being scanned; only the "may be missing" allowance is gone, so the recorded baseline doesn't change.

Why

#24988 (the check) and #24989 (the folder) were reviewed separately and merged back to back. The test lets only directories that have not landed yet be absent exists to make the PR that adds a folder also make it required, but that PR merged after the check, so neither PR's CI could see the conflict. It now fails test shard 5/5 on main and on every PR that merges main.

Linked Issue

None. This fixes main after #24988 and #24989.

Visual Proof

N/A: test and CI-script change only.

Testing

• I manually tested these changes locally
• Automated tests added/updated, or explained why not below

./node_modules/.bin/vitest run --config config/vitest.config.ts config/scripts/check-runtime-electron-ratchet.test.mjs: the previously failing test passes, along with the lane and fixture tests. Locally, the two tests that build the real tree fail only because this machine's installed packages are older than main (stream-json is missing); CI installs fresh packages. oxlint and oxfmt are clean on both files.

Review

Agent skill upstream boundary

• Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Cross-platform: the paths go through path.join, as before. No effect on SSH, mobile or the runtime.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[coderabbitai]

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: 1566ab4c-0733-4c73-8451-1e9a3596aeb9

📥 Commits

Reviewing files that changed from the base of the PR and between 48f072a and 805cc1e.

📒 Files selected for processing (2)

• config/scripts/check-runtime-electron-ratchet.mjs
• config/scripts/check-runtime-electron-ratchet.test.mjs

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

---

📝 Walkthrough

Walkthrough

The runtime ratchet now requires src/main/provider-process to exist. The acp directory remains optional. The test fixture adds provider-process-teardown.ts and expects six structured-chat entry points.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 805cc

The ratchet now requires provider-process while keeping acp optional, and the tests cover those behaviors. No material merge risk remains.

🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1

❌ Failed checks (1 warning, 1 inconclusive)

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 2 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description covers the change, reason, testing, visual proof, and checklist. It says there is no linked issue, but the template requires one for outside contributors and exempts maintainers. The a…	If the author is an outside contributor, add a link to the issue this PR addresses and complete the AI Disclosure section. If the author is a maintainer, confirm that status so the template’s exceptions can be applied.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the main change: making src/main/provider-process required in the Electron ratchet check.
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.

Full details: Description check

Explanation

The description covers the change, reason, testing, visual proof, and checklist. It says there is no linked issue, but the template requires one for outside contributors and exempts maintainers. The author’s status is not provided.

• 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[pullfrog]

✅ No new issues found.

Reviewed changes

• provider-process lane made required — STRUCTURED_CHAT_LANES drops the mayBeAbsent: true flag from src/main/provider-process, so a rename/removal now throws instead of silently checking nothing; src/main/acp keeps the allowance.
• Test fixture and comments updated — the required-lanes fixture gains provider-process-teardown.ts, the lane-count assertion moves 5 → 6, and the stale comments now name only acp/ as optional.

Verified src/main/provider-process/ exists at head with no electron imports, and the full test file (17 tests, including the real-tree baseline run) passes locally. The fails loudly when src/main/provider-process goes missing case is genuinely load-bearing: it drives the new lane through the toThrow path, so it would still be green before this PR only because the lane wasn't required yet.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}