Skip to content

fix(coil): scripts typecheck passes on effect rc.115 - #154

Merged
radroid merged 1 commit into
mainfrom
coil/fix-scripts-typecheck
Sep 20, 2026
Merged

radroid merged 1 commit into
mainfrom
coil/fix-scripts-typecheck

Conversation

@radroid

@radroid radroid commented Sep 20, 2026 •

Copy link
Copy Markdown
Owner

Main has been red since the sync merge (#153) landed: the scripts package fails typecheck, and because the release workflow gates on green CI, no build has shipped since 0.0.40-coil.138.

Two fork-owned files under scripts/coil were the cause:

  • verify-mac-signature.ts used Flag.string / Flag.boolean. Effect rc.115 renamed these to Flag.String / Flag.Boolean (upstream's apps/server/scripts/cli.ts already uses the new names).
  • sync-upstream.test.ts imports node:fs / node:child_process / node:path, and nodeBuiltinImport is an error in tsconfig.base.json. It now carries the same first-line @effect-diagnostics nodeBuiltinImport:off header upstream puts on its own scripts tests that shell out to real processes.

Verified locally at rc.115: tsc --noEmit in scripts exits 0 with no error TS lines, coil/sync-upstream.test.ts and coil/mac-signature.test.ts pass, lint and format are clean.

Written by Claude Fable 5.1 via Claude Code.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores

    • Updated internal command-line flag definitions without changing available options or behavior.
  • Tests

    • Adjusted diagnostics for an integration test; test behavior remains unchanged.

The sync merge (#153) bumped effect to rc.115, which renamed the unstable CLI
helpers Flag.string/Flag.boolean to Flag.String/Flag.Boolean. verify-mac-signature
still used the old names, so the scripts package failed typecheck on main and the
release was withheld. sync-upstream.test.ts also tripped the nodeBuiltinImport
diagnostic; it now carries the same per-file opt-out header upstream puts on its
own scripts tests that shell out to real processes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: radroid/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: fe23c7fa-41b4-4d7d-a98b-04a8c9dfb2d5

📥 Commits

Reviewing files that changed from the base of the PR and between 0e03812 and 3850427.

📒 Files selected for processing (2)
  • scripts/coil/sync-upstream.test.ts
  • scripts/coil/verify-mac-signature.ts

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


📝 Walkthrough

Walkthrough

The changes update CLI flag constructor names in the macOS signature verification command and add a diagnostic suppression directive to the upstream synchronization integration test. Runtime behavior remains unchanged.

Changes

CLI flag API update

Layer / File(s) Summary
Update verification command flags
scripts/coil/verify-mac-signature.ts
The command uses Flag.String and Flag.Boolean instead of the lowercase constructors. Flag names, descriptions, optionality, and behavior remain unchanged.

Integration test diagnostic

Layer / File(s) Summary
Suppress integration test diagnostic
scripts/coil/sync-upstream.test.ts
The test file disables the nodeBuiltinImport warning. Test runtime behavior remains unchanged.

Priority: ⬇️ Low

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

Change: Bug fix

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: restoring scripts typechecking with Effect rc.115.
Description check ✅ Passed The description clearly explains the changes, the cause, the scope, and local verification results. It does not use the template headings or checklist, but it provides the required information and rem…
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 2…
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
🧪 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.

@radroid
radroid merged commit fecbf70 into main Sep 20, 2026
2 checks passed
@radroid
radroid deleted the coil/fix-scripts-typecheck branch September 20, 2026 11:52
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