[refactor] Group RunPlan by concern instead of one flat bag - #5702
Conversation
dosubot
Bot
added
size:XL
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
c2ab544 to
469f8e5
Compare
Railway Preview Environment
Updated at 2026-08-03T19:07:42.858Z |
`RunPlan` had grown to thirty fields on one flat interface, and every consumer took the whole thing regardless of how little it read. Raised in review of #5670. The fields are grouped into `credentials`, `workspace`, `tools`, and `prompt`. The five identity fields (`harness`, `acpAgent`, `sandboxId`, `isPi`, `isDaytona`) stay at the top level because almost every consumer branches on them, and `sandboxPermission` stays there too since the declared security boundary is its own concern. The payoff is not the tidier interface. It is that a consumer can now say what it actually touches. `prepareWorkspace` used to take a nine-key `Pick` of the flat plan; it now takes three named slices and the parameter type reads as a description of the function. Several other consumers narrowed the same way, which means a future change to, say, credential delivery has a compiler-checked list of what depends on it. The one rename is `plan.prompt` to `plan.prompt.text`, forced by the group taking the name. Nothing else was renamed, no logic changed, and no `any` cast or `@ts-expect-error` was added. Runner tsc clean, 99 files / 1535 tests, the same counts as before the change.
469f8e5 to
a6879d8
Compare
1 participant
Context
RunPlanhad grown to thirty fields on one flat interface, and every consumer took the whole thing regardless of how little of it they read. Raised in review of #5670: #5670 (comment)This PR is stacked on
feat/daytona-secrets-v2(#5670) and its base is that branch, so the diff here is only the refactor.Changes
The fields are grouped by concern into
credentials,workspace,tools, andprompt, each exported as its own interface. Five identity fields stay at the top level because almost every consumer branches on them, andsandboxPermissionstays there too since the declared security boundary is its own concern.The real payoff is that consumers can now say what they touch.
prepareWorkspaceused to declare a nine-keyPickof the flat plan:Before:
After:
The parameter type now reads as a description of the function rather than an alphabet soup of field names.
pi-assets.ts,codex-assets.ts,attachments.ts, anddaytona.tsnarrowed the same way. That means a future change to credential delivery has a compiler-checked list of what depends on it, which is the thing the flat bag made impossible.The one rename is
plan.prompttoplan.prompt.text, forced by the group taking the name.Tests / notes
tsc --noEmitis clean and the unit suite reports 99 files / 1535 tests, the same counts as before the change.anycast,@ts-ignore, or@ts-expect-errorwas added; verified by grepping the diff.run-plan.tsfirst for the new shape, then skim the rest as mechanical. The interesting files are the ones where a signature narrowed, since that is where the change earns its keep.This is a pure refactor, so there is nothing to QA manually. If the runner suite is green and the type checker is clean, behavior is unchanged.