chore: back-merge main hotfixes into dev (option 2 — accept dev for conflicts) - #1703
namastex888 wants to merge 2 commits into
Conversation
Brings 6 main-only PRs (#1689, #1690, #1693, #1698, #1699) into dev. Strategy: -X ours — for files modified on both branches, keep dev's version (dev is 481 commits past the merge-base ca0a1d8 and those evolved files supersede main's older edits). New files from main come in cleanly. Files from main expected to land: - src/genie-commands/recover-orphans.ts (new, from #1699) - src/genie-commands/__tests__/recover-orphans.test.ts (new) - src/db/migrations/054_fix_subagent_team_inheritance.sql (new, from #1689) - src/db/migrations/055_default_auto_resume_true.sql (new, from #1690) - src/__tests__/agent-team-inheritance.test.ts (new, from #1689) - src/lib/protocol-router.test.ts (new, from #1689/#1690) - src/lib/provider-adapters.test.ts (new, from #1690)
…d it The -X ours strategy resolved the imports-block conflict in genie.ts to dev's version, which dropped main's import line for recoverOrphansCommand (added by PR #1699). The command registration at line 213 survived intact, so the build broke with TS2304 'Cannot find name recoverOrphansCommand'. Restoring the import + its RecoverOrphansOptions type, biome organize-imports applied.
|
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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Superseded — dev now has |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf20fbf517
ℹ️ 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".
| WHERE child.id LIKE parent.id || '/%' | ||
| AND parent.id NOT LIKE '%/%' |
There was a problem hiding this comment.
This migration assumes agent_templates.id is a text name (child.id LIKE parent.id || '/%' and the built-in DELETE ... WHERE id IN (...)), but this repo already has migration 061 that retypes agent_templates.id to UUID and introduces name as the human key. On any environment that already applied 061, this pending migration will fail during runMigrations with UUID/text operator or cast errors, blocking subsequent migrations and startup; the backfill/delete logic needs to use name (or explicit safe casts) instead of id.
Useful? React with 👍 / 👎.
| if (!isBuiltinAgent(templateId)) { | ||
| // Hierarchical names (`parent/child`) inherit the parent's team. Without |
There was a problem hiding this comment.
Skipping saveTemplate for built-ins removes the only spawn template source that ensureWorkerAlive/findSpawnTemplate uses when a worker is dead or suspended, so built-in agents can no longer be auto-respawned after pane loss and message delivery falls through to failure. This is especially impactful here because the same commit's migration deletes existing built-in template rows, making the regression immediate on upgraded installs unless there is an alternate template source.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a series of improvements to agent session management and team inheritance, most notably a new recover-orphans command for re-linking orphaned session files and a migration to enable auto_resume by default. It also addresses issues where built-in agents could inherit incorrect team pins and retires the --name flag to ensure reliable session resumption. Review feedback highlighted a recurring date typo in documentation comments and recommended renaming a misleading variable in the orphan recovery implementation.
| test('fresh DB: agents.auto_resume column default is false after migration 044', async () => { | ||
| test('fresh DB: agents.auto_resume column default is true after migration 055', async () => { | ||
| // Migration 055 (PR #1693) flipped the column default from `false` to `true`. | ||
| // Felipe directive 2026-05-07: default-off was a bug shape — 52/53 agents |
There was a problem hiding this comment.
The date 2026-05-07 appears to be a typo as it is in the future. It should likely be 2024-05-07. This typo is present in several other files in this PR as well (e.g., 055_default_auto_resume_true.sql, agent-directory.ts, executor-registry.ts, provider-adapters.ts, etc.). It would be good to correct it everywhere for consistency and to avoid confusion.
// Felipe directive 2024-05-07: default-off was a bug shape — 52/53 agents| const fd = readFileSync(jsonlPath, { encoding: 'utf-8', flag: 'r' }); | ||
| head = fd.slice(0, 16384); |
There was a problem hiding this comment.
The variable name fd is a bit misleading here. Typically, fd stands for 'file descriptor', which is an integer. However, in this context, it holds the entire file content as a string. Renaming it to something like fileContent or content would improve clarity and maintainability.
const fileContent = readFileSync(jsonlPath, { encoding: 'utf-8', flag: 'r' });
head = fileContent.slice(0, 16384);2 participants
Why
Main and dev diverged from common ancestor `ca0a1d8100`:
Six main-only PRs never reached dev:
Strategy
Standard `git merge --no-ff` with `-X ours`:
This matches user intent: "accept dev for everything except genuinely new content from those 6 PRs."
What lands
20 files / +1479 / −73 — mostly the new content, plus modest edits to wire it in:
New files (725 LOC of genuinely new content):
Modified files (main edits wiring in the new functionality):
Notes
One follow-up commit (`fix(merge): restore recoverOrphansCommand import`) was needed: the `-X ours` resolution of the imports block in `genie.ts` kept dev's version, which dropped main's import line — but the command registration line survived, breaking the build (`TS2304: Cannot find name recoverOrphansCommand`). Re-added the import + biome organize-imports.
Test plan
What this is NOT
This is not a wholesale main → dev merge. With plain `git merge`, dev would resurrect 7 stale files and lose conflicts in 219 modified files to main's older versions. `-X ours` prevents that.