Skip to content

refactor(opencode): inject services in light instance routes - #1319

Merged
Astro-Han merged 1 commit into
devfrom
codex/i936-light-route-effect
Jun 14, 2026
Merged

refactor(opencode): inject services in light instance routes#1319
Astro-Han merged 1 commit into
devfrom
codex/i936-light-route-effect

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Jun 14, 2026

Copy link
Copy Markdown
Owner

Related to #936

@Astro-Han Astro-Han added P2 Medium priority upstream Tracked upstream or vendor behavior harness Model harness, prompts, tool descriptions, and session mechanics task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work tech-debt Supplemental cleanup, maintainability, architecture, test, or quality debt context labels Jun 14, 2026
@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0d34b07e-41ae-4ba9-9004-7e23d88fcb2f

📥 Commits

Reviewing files that changed from the base of the PR and between fbd6765 and 3fbe1c4.

📒 Files selected for processing (6)
  • packages/opencode/specs/effect-migration.md
  • packages/opencode/src/server/instance/global.ts
  • packages/opencode/src/server/instance/index.ts
  • packages/opencode/src/server/instance/permission.ts
  • packages/opencode/src/server/instance/session.ts
  • packages/opencode/test/server/vcs-routes.test.ts

📝 Walkthrough

Walkthrough

Four server/instance route handler modules (permission.ts, session.ts, index.ts, global.ts) are refactored from Service.use(...) service access to Effect.gen with yield*-style injection running through AppRuntime.runPromise. The VCS apply test is updated to use a real git-initialized end-to-end patch instead of a mock. A migration log entry is added.

Changes

Instance Route Handler Effect.gen Migration

Layer / File(s) Summary
Service.use → Effect.gen in all four instance handlers
packages/opencode/src/server/instance/permission.ts, packages/opencode/src/server/instance/session.ts, packages/opencode/src/server/instance/global.ts, packages/opencode/src/server/instance/index.ts
Six route handler bodies (POST /__e2e/ask, GET / in permission; GET /status, POST /__e2e/update-todos, GET /:sessionID/todo in session; POST /upgrade in global; GET /vcs/diff/raw, POST /vcs/apply in index) are rewired to obtain services via yield* Service inside Effect.gen blocks run through AppRuntime.runPromise, replacing Service.use(...). All response shapes, error mappings, and side effects are preserved.
VCS apply end-to-end test and migration log
packages/opencode/test/server/vcs-routes.test.ts, packages/opencode/specs/effect-migration.md
The escaped-JSON apply test switches from a mocked Vcs.apply to a real git-initialized temp directory test that applies an escaped-backslash patch, asserts byte-length constraints against Vcs.MAX_APPLY_PATCH_BYTES, and verifies escaped.txt file contents. A migration log bullet is added for the instance handler changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • Astro-Han/pawwork#1032: Earlier effectification of permission.ts route handlers using the same AppRuntime.runPromise(Effect.gen(...)) / yield* pattern this PR continues.
  • Astro-Han/pawwork#578: Introduced the POST /__e2e/ask handler wiring in permission.ts that this PR replaces with Effect.gen/yield*.
  • Astro-Han/pawwork#956: Added the GET /vcs/diff/raw and POST /vcs/apply routes in index.ts whose internals this PR migrates to the Effect.gen pattern.

Poem

🐇 Hop, hop, yield and flow,
No more .use() down below!
Effect.gen takes the stage,
Services yield from every cage.
The bunny cheers each migrated line —
The runtime runs, the tests align! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is severely incomplete, providing only 'Related to #936' without covering required template sections such as Summary, Why, Human Review Status, Review Focus, Risk Notes, How To Verify, and Checklist items. Complete the PR description by filling out all template sections: add a detailed summary of changes, explain the goal/problem, set Human Review Status, describe review focus, identify any risks, provide verification steps, and tick applicable checklist items.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'refactor(opencode): inject services in light instance routes' clearly summarizes the main change across all modified files in the changeset.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/i936-light-route-effect

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


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 and usage tips.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested priority: P2 (includes non-doc, non-test paths outside the low-risk bucket).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request migrates several light instance route handlers (including global, VCS, permission, and session routes) to use the unified AppRuntime.runPromise(Effect.gen(...)) service injection pattern instead of the legacy .use(...) pattern. Additionally, it updates the VCS routes test to perform a real integration test with a git-enabled temporary directory instead of mocking Vcs.apply. I have no feedback to provide as there are no review comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@Astro-Han
Astro-Han merged commit 5e6bfa7 into dev Jun 14, 2026
37 of 38 checks passed
@Astro-Han
Astro-Han deleted the codex/i936-light-route-effect branch June 14, 2026 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

harness Model harness, prompts, tool descriptions, and session mechanics P2 Medium priority task Narrow execution, audit, spike, migration, tracking, or upstream follow-up work tech-debt Supplemental cleanup, maintainability, architecture, test, or quality debt context upstream Tracked upstream or vendor behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant