feat: add Pi ready marker and session metadata#129
feat: add Pi ready marker and session metadata#129RogerNavelsaker wants to merge 6 commits intojayminwest:mainfrom
Conversation
jayminwest
left a comment
There was a problem hiding this comment.
Review
Good DRY refactor centralizing session env vars, and the Pi ready marker is a clear improvement over fragile TUI heuristic detection. A few issues to address:
Must fix
-
Supervisor
sessionKindis wrong (src/commands/supervisor.ts) — The supervisor getssessionKind: "coordinator"but it's a per-project supervisor, not a coordinator. TheOverstorySessionKindunion includes"standalone" | "orchestrator" | "coordinator" | "monitor" | "worker"but omits"supervisor". Either add"supervisor"to the union or use a more semantically accurate kind. This propagates an incorrectOVERSTORY_SESSION_KIND=coordinatorenv var to any consumer. -
No tests for
buildOverstorySessionEnv()(src/agents/session-env.ts) — This is a new pure function with conditional spread logic. It should have a colocatedsession-env.test.tscovering: (a) all required fields are set, (b) optionaltaskId/profileare omitted when undefined and included when defined, (c)baseEnvmerge order (Overstory keys should override baseEnv on collision). Trivially testable pure function — should be straightforward. -
Types belong in
src/types.tsper project convention — CLAUDE.md states "All shared types and interfaces go insrc/types.ts". TheOverstorySessionKindtype andOverstorySessionEnvOptsinterface are cross-cutting (used by coordinator, monitor, supervisor, sling) and should be moved there.
Non-blocking suggestions
- Document that
OVERSTORY_SESSION_KIND,OVERSTORY_CAPABILITY, andOVERSTORY_PROJECT_ROOTare part of the external contract with the os-eco Pi extension (no consumers in the overstory codebase yet). - The
PI_READY_MARKER_PREFIXuses a Unicode checkmark (\u2713) — consider a comment noting the exact character for searchability. sapling.ts:48comment "Mirrors the constant in pi-guards.ts" — the cross-reference was removed but no replacement context was added.
91fd04d to
3b2cf64
Compare
|
Rebased onto current |
|
Hi @jayminwest, as I’ve been working on this, I realized it might be cleaner to keep the os-eco related tools together. I originally created the pi-os-eco extension as a way to contribute while keeping the Overstory code separate from the direct pi-coding-agent. If you're interested, I’m happy to transfer the pi-os-eco repository to you. This would allow you to publish the extension to NPM under your own organization and keep the entire ecosystem under one roof. Let me know if that’s something you’d like to do. |
… vars - Add optional extensionSource field to PiRuntimeConfig so the extension URL is not hardcoded to a personal repo — callers can override via config - Document OVERSTORY_* agent session env vars in CLAUDE.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
✓ os-eco agent=... runtime=pimarker emitted by the companion extensionTesting