feat: retain Luna as interaction-only worker - #106
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 279cc2d201
ℹ️ 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".
| Within one role thread append concise deltas; a new thread receives the latest self-contained artifact instead of the full prior conversation. | ||
| ${workflowRecoveryContract} | ||
| ${workflowEscalationContract} | ||
| ${workflowLunaInteractionContract} |
There was a problem hiding this comment.
Register the previous skill template for migration
Adding this contract changes the managed model-router/SKILL.md, but the exact template from the parent commit is absent from LEGACY_TEMPLATES.skill. Unlike agent TOMLs, skill contents are not added dynamically by installedProfile (lib/cli.js:194-212), so the migration branch in lib/router-core.js:675-684 classifies an otherwise untouched tracked skill as “managed content is not a recognized package template” and preserves it. Consequently, existing installations never receive this central Luna interaction contract; add the immediately previous skill template (and similarly the changed planning skill) to the recognized migration set.
Useful? React with 👍 / 👎.
| For each workflow, derive one exact PLAN_ARTIFACT_PATH under the selected safe CODEX_ROOT as \`model-router/workflows/<workflow_id>/PLAN.md\`. Pass the unchanged workflow_id and path to every role; stop the stage if either is missing, relative, changed, or cross-workflow. Reviewers return plan content as self-contained role artifacts; only the active writable executor persists or removes the exact workflow directory. On PASS, request cleanup after required evidence is preserved; persist and report cleanup-failed rather than silently ignoring it. | ||
|
|
||
| For nontrivial code changes, the primary loads or initializes task-scoped workflow state, performs only stage-required actions, persists each transition atomically, and returns on PASS or a blocked verdict. INITIAL uses Terra planning/review with enabled Luna reading/execution for every primary, including Primary Sol. Non-PASS verdicts follow the monotonic Sol replan, Terra-attempt, and full-takeover stages; blocked verdicts never attempt execution or advance counters. | ||
| For nontrivial code changes, the primary loads or initializes task-scoped workflow state, performs only stage-required actions, persists each transition atomically, and returns on PASS or a blocked verdict. INITIAL and SOL_REPLAN_WITH_LUNA use Luna as ACTIVE_EXECUTOR. After a failed Sol replan, SOL_PLAN_REVIEW_WITH_TERRA and SOL_FULL_TAKEOVER demote the same Luna role to INTERACTION_ONLY instead of removing it; Terra or Sol owns planning/implementation while Luna runs only delegated bounded actions. Non-PASS verdicts follow the monotonic Sol replan, Terra-attempt, and full-takeover stages; blocked verdicts never attempt execution or advance counters. |
There was a problem hiding this comment.
Keep the unknown-primary topology within two children
When the primary identity is unavailable, the fallback on line 791 creates Luna and Terra and later adds Sol, so after this new Stage 3 rule it needs Luna for interaction, Terra for execution, and Sol for review—all as children. That exceeds maxChildren: 2 at line 362, and the recovery contract explicitly turns an oversized valid-role set into EVIDENCE_GAP without mutation, preventing every escalated unknown-primary workflow from continuing. Define which role is detached or handled inline in this fallback instead of retaining all three.
Useful? React with 👍 / 👎.
Summary
Why
A failed Sol replan previously removed Luna from the topology. That lost a reusable bounded worker and forced higher-reasoning roles to spend tokens on tests, builds, logs, and artifact inspection. This keeps Luna available for authorized interaction without restoring implementation or decision authority.
Validation
pm test — 96 passed
pm run check — passed
pm run test:package — passed
Closes #104