Refactor: use compiled perry binary for container session discovery#13
Refactor: use compiled perry binary for container session discovery#13
Conversation
8a41375 to
5f9410f
Compare
gricha
left a comment
There was a problem hiding this comment.
Code Review
Good refactoring that eliminates shell script duplication in favor of shared TypeScript. A few items to address:
1. DESIGN.md doesn't match implementation
The documentation describes a packages/opencode-sessions and packages/perry-worker structure with npm-based installation:
// perry/internal/src/lib/devtools.ts
const ensurePerryWorker = async () => {
const installResult = await runCommand("npm", ["install", "-g", "@gricha/perry-worker"]);But the actual implementation:
- Uses
src/sessions/agents/opencode-storage.ts(not a separate package) - Copies a compiled binary directly (not npm install)
Please update DESIGN.md to reflect what was actually built.
2. Silent failure when binary doesn't exist
In src/workspace/manager.ts:400-410:
try {
await fs.access(workerBinaryPath);
} catch {
return; // Silently continues
}If the binary doesn't exist, sync completes successfully but session discovery will fail later with a confusing error. Consider logging a warning here.
3. Binary name collision risk
const destPath = '/usr/local/bin/perry';This overwrites any existing perry in the container. Consider using /usr/local/bin/perry-worker to avoid potential conflicts and make the purpose clearer.
Core approach is solid. These are minor cleanups.
5f9410f to
c7a48ba
Compare
gricha
left a comment
There was a problem hiding this comment.
Re-review: LGTM
Previous concerns addressed:
✅ DESIGN.md - Now matches actual implementation
✅ Warning on missing binary - Added console.warn during sync
One minor note: bun.lock still has entries for @gricha/opencode-sessions and @gricha/perry-worker packages that don't appear to be used in the final implementation. Consider cleaning those up if they're leftover from a different approach, but not blocking.
Ready to merge.
- Add perry worker subcommand with sessions list/messages commands - Compile perry to standalone binary during build (bun build --compile) - Copy perry binary to containers during sync (/usr/local/bin/perry) - Remove shell script approach (perry-session-reader.sh) - Share opencode-storage.ts between host and worker modes - Update AGENTS.md with manual testing instructions for port 7391 - Fix OpenCode sessions not showing up in web UI This replaces the npm-based perry-worker package with a compiled binary that gets synced to containers, eliminating the need to publish a separate package and ensuring the container always has the latest code.
c7a48ba to
b0d0621
Compare
Summary
perry workersubcommand withsessions list/messagescommandsbun build --compile)/usr/local/bin/perry)perry-session-reader.sh)opencode-storage.tsbetween host and worker modesProblem
OpenCode sessions weren't showing up in the web UI because the container didn't have the
perry-session-reader.shscript - it was added after the Docker image was built. Rather than baking more scripts into the image (which requires rebuilding), we needed a better pattern.Solution
Instead of separate tools baked into the Docker image, we now:
bun run buildusingbun build --compileperry syncto/usr/local/bin/perryperry worker sessions list/messagesto discover sessions inside containersThis means:
perry syncChanges
package.jsonbuild:workerscript, remove workspace configsrc/index.tsperry worker sessions list/messagessubcommandsrc/sessions/agents/opencode-storage.tssrc/sessions/agents/opencode.tsperry workerinstead ofperry-workersrc/workspace/manager.tscopyPerryWorker()to sync binary to containerssrc/agent/router.tsopencode-storage.tsfor host sessionsperry/Dockerfileperry-session-reader.shreferenceperry/scripts/perry-session-reader.shAGENTS.mdDESIGN.mdTesting
All automated tests pass (
bun run validate).