Issue 122: define first-class runner harness interfaces - #124
Conversation
|
Visit the preview URL for this PR (updated for commit b28dc28): https://pi-agents-cloud--pr124-122-runner-harnesses-lhhllgee.web.app (expires Tue, 07 Jul 2026 21:26:39 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: ad3a3866d5dcd316f5eca68b23f0ce7295063677 |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 858fb49026
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const skillService = createWorkspaceSkillService({config, syncUp}); | ||
| const subagentService = createWorkspaceSubagentService({config, syncUp}); |
There was a problem hiding this comment.
Defer harness-only services for shell runners
For Shell and SSH sessions, server.js still constructs createPiService unconditionally, but this now eagerly constructs workspace skill/subagent services whose harness helpers throw when the active harness does not support those features. With HARNESS_ID=shell (the default image) the runner process fails during startup before the terminal can serve requests, so shell/SSH sessions cannot start; these services need to be lazy or tolerate unsupported harnesses.
Useful? React with 👍 / 👎.
| function agentAuthDoc(uid, db) { | ||
| return db.collection("users").doc(uid).collection("private").doc("agentAuth"); | ||
| } |
There was a problem hiding this comment.
Read the legacy piAuth document during auth migration
For users who saved Pi credentials before this change, the data is still stored in users/{uid}/private/piAuth; switching every auth read/write to agentAuth without a fallback or copy makes /api/auth and the legacy /api/pi-auth alias return no saved entries, and runners have no remote credentials to materialize until users re-enter them. Please migrate or read the old document during rollout.
Useful? React with 👍 / 👎.
| const data = snap.exists ? snap.data() : {}; | ||
| if (!Object.prototype.hasOwnProperty.call(data, "authSelection")) return null; | ||
| return normalizeAuthSelection(data.authSelection); |
There was a problem hiding this comment.
Honor legacy piAuthSelection when materializing auth
Existing session documents store their per-session credential choices as piAuthSelection, but the new runner only checks for authSelection and returns null otherwise. When an existing Pi session is restarted under this runner, buildMaterializedAuth falls back to the full provider set instead of the previously selected entries, so saved session auth choices are ignored until the user manually saves them again.
Useful? React with 👍 / 👎.
|
codex fails to launch |
Summary
harnessIdon sessionsTesting
QA
Closes #122