Repository navigation
feat(agent-sync): genie update converges every detected coding agent - #2541
Conversation
…cs (agent-sync G3)
…saved-workflow input
|
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 |
![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 implements the agent-sync feature, which converges the genie skill set and the /council workflow stamp across all detected coding agents (Claude Code, Codex, and Hermes) using genie update as the canonical updater. It introduces the core sync engine, integrates triggers into the install and update commands, delegates the SessionStart hook to the CLI, and updates the doctor and uninstall commands to handle synced assets. The review feedback highlights three important improvement opportunities in src/lib/agent-sync.ts: normalizing relative paths to forward slashes in computeDirDigest to prevent platform-dependent digest mismatches, wrapping individual orphan processing in a try-catch block within removeManagedOrphans to ensure resilience against single-file failures, and using directory junctions on Windows in ensureHermesLink to avoid privilege and compatibility issues.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ): void { | ||
| for (const entry of readdirSync(current, { withFileTypes: true })) { | ||
| const abs = join(current, entry.name); | ||
| const rel = relative(root, abs); |
There was a problem hiding this comment.
On Windows, relative(root, abs) returns paths with backslashes (\\), whereas on POSIX systems it returns forward slashes (/). This makes the computed directory digest platform-dependent, which can lead to false-positive cache mismatches or 'stale' warnings when manifests are shared or verified across different operating systems. Normalizing the relative path to use forward slashes ensures platform-independent digests.
| const rel = relative(root, abs); | |
| const rel = relative(root, abs).replace(/\\/g, '/'); |
| function removeManagedOrphans( | ||
| ctx: RunContext, | ||
| agent: string, | ||
| targetParent: string, | ||
| sourceNames: Set<string>, | ||
| report: AgentReport, | ||
| ): void { | ||
| for (const entry of readdirSync(targetParent, { withFileTypes: true })) { | ||
| if (entry.name.endsWith(STAGING_SUFFIX) || entry.name.endsWith(PREV_SUFFIX)) continue; | ||
| const dir = join(targetParent, entry.name); | ||
| if (classifyEntry(dir, entry) !== 'dir' || sourceNames.has(entry.name)) continue; | ||
| const manifest = readManifest(dir); | ||
| if (manifest === null) continue; | ||
| if (computeDirDigest(dir) === manifest.digest) { | ||
| ctx.backupInto(agent, entry.name, dir); | ||
| rmSync(dir, { recursive: true, force: true }); | ||
| report.skills.push({ name: entry.name, action: 'removed' }); | ||
| } else { | ||
| report.skills.push({ name: entry.name, action: 'kept-modified-orphan' }); | ||
| report.advisories.push(`kept modified orphan ${entry.name} (${agent})`); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
If reading the manifest, computing the digest, or deleting/backing up an orphan directory fails (e.g., due to file locks, permission issues, or corrupt files), the entire removeManagedOrphans function will throw and abort. This would prevent other orphans from being cleaned up and abort subsequent sync steps for the agent. Wrapping the individual orphan processing inside the loop in a try-catch block ensures that a failure with one orphan doesn't disrupt the rest of the cleanup process.
function removeManagedOrphans(
ctx: RunContext,
agent: string,
targetParent: string,
sourceNames: Set<string>,
report: AgentReport,
): void {
for (const entry of readdirSync(targetParent, { withFileTypes: true })) {
if (entry.name.endsWith(STAGING_SUFFIX) || entry.name.endsWith(PREV_SUFFIX)) continue;
const dir = join(targetParent, entry.name);
if (classifyEntry(dir, entry) !== 'dir' || sourceNames.has(entry.name)) continue;
try {
const manifest = readManifest(dir);
if (manifest === null) continue;
if (computeDirDigest(dir) === manifest.digest) {
ctx.backupInto(agent, entry.name, dir);
rmSync(dir, { recursive: true, force: true });
report.skills.push({ name: entry.name, action: 'removed' });
} else {
report.skills.push({ name: entry.name, action: 'kept-modified-orphan' });
report.advisories.push(`kept modified orphan ${entry.name} (${agent})`);
}
} catch (err) {
report.advisories.push(`orphan removal ${entry.name} (${agent}) failed: ${err instanceof Error ? err.message : String(err)}`);
}
}
}| function ensureHermesLink( | ||
| ctx: RunContext, | ||
| linkPath: string, | ||
| hermesRoot: string, | ||
| backupName: string, | ||
| report: AgentReport, | ||
| ): LinkAction { | ||
| mkdirSync(dirname(linkPath), { recursive: true }); | ||
| const stat = lstatSafe(linkPath); | ||
| if (stat === null) { | ||
| symlinkSync(hermesRoot, linkPath); | ||
| report.extras.push({ kind: 'symlink', action: 'created', detail: `${linkPath} -> ${hermesRoot}` }); | ||
| return 'created'; | ||
| } | ||
| if (stat.isSymbolicLink()) return reconcileExistingSymlink(linkPath, hermesRoot, report); | ||
| const backup = ctx.backupInto('hermes', backupName, linkPath); | ||
| rmSync(linkPath, { recursive: true, force: true }); | ||
| symlinkSync(hermesRoot, linkPath); | ||
| report.extras.push({ kind: 'symlink', action: 'adopted', detail: `real dir backed up to ${backup}` }); | ||
| return 'adopted'; | ||
| } |
There was a problem hiding this comment.
On Windows, symlinkSync requires a type argument (such as 'dir' or 'junction') when creating a symlink to a directory. If not specified, it may default to a file symlink or fail. Furthermore, creating directory symlinks on Windows typically requires administrator privileges unless Developer Mode is enabled, whereas directory junctions ('junction') do not require special privileges. Since hermesRoot is a directory and we are resolving absolute paths, we should use 'junction' on Windows to ensure maximum compatibility and avoid privilege issues.
function ensureHermesLink(
ctx: RunContext,
linkPath: string,
hermesRoot: string,
backupName: string,
report: AgentReport,
): LinkAction {
mkdirSync(dirname(linkPath), { recursive: true });
const stat = lstatSafe(linkPath);
const symlinkType = process.platform === 'win32' ? 'junction' : undefined;
if (stat === null) {
symlinkSync(hermesRoot, linkPath, symlinkType);
report.extras.push({ kind: 'symlink', action: 'created', detail: `${linkPath} -> ${hermesRoot}` });
return 'created';
}
if (stat.isSymbolicLink()) return reconcileExistingSymlink(linkPath, hermesRoot, report);
const backup = ctx.backupInto('hermes', backupName, linkPath);
rmSync(linkPath, { recursive: true, force: true });
symlinkSync(hermesRoot, linkPath, symlinkType);
report.extras.push({ kind: 'symlink', action: 'adopted', detail: `real dir backed up to ${backup}` });
return 'adopted';
}![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: 46cc3fb7ec
ℹ️ 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".
| collectManagedSkillDirs(join(claudeDir, 'skills'), 'claude', out); | ||
| collectManagedSkillDirs(join(codexDir, 'skills', '.curated'), 'codex', out); | ||
| collectStampedCouncil(claudeDir, out); | ||
| collectHermesLink(hermesHome, genieHome, out); |
There was a problem hiding this comment.
When an active Hermes profile exists, the new sync path writes a managed symlink at ~/.hermes/profiles/<active>/plugins/genie (see ensureStickyProfileLink), but this collection only checks ~/.hermes/plugins/genie. Running genie uninstall then removes ~/.genie while leaving the profile-level symlink behind, so that profile retains a broken/stale Genie plugin entry. Include the profile links that resolve into genieHome in the uninstall asset scan.
Useful? React with 👍 / 👎.
| function isStampedCouncil(councilPath: string): boolean { | ||
| try { | ||
| const content = readFileSync(councilPath, 'utf8'); | ||
| return content.includes('const LENS_ROOT =') && /name:\s*'council'/.test(content); |
There was a problem hiding this comment.
With a user-managed ~/.claude/workflows/council.js that happens to use the same workflow name and a const LENS_ROOT = ... constant (for example a customized copy pointing outside Genie), this predicate treats it as shipped by Genie and removeAgentSyncAssets unlinks it. Since the file has no manifest and lives outside ~/.genie, verify that the stamped root is one of Genie's managed roots (or otherwise mark generated files) before deleting it.
Useful? React with 👍 / 👎.
…ng INDEX entries Full wish inventory (2026-07-26) found the INDEX.md ledger current but six WISH.md Status headers stale, two wishes entirely absent from INDEX, and one INDEX line (stable-release-security-gate) behind its own WISH header. Headers reconciled (each keeps its history, gains a dated note): - plugin-resource-shipping DRAFT -> EXECUTED (PR automagik-si#2540 merged 07-10) - codex-plugin-update-handoff EXECUTED/'merge gate OPEN' -> SHIPPED (PR automagik-si#2617 merged 07-22) - genie-ui IN_PROGRESS -> SUPERSEDED (direction moved to genie-ui-dash 07-21) - agent-sync-hardening BLOCKED -> SUPERSEDED (gates closed with pr-2545 07-24, F02 excepted) - agent-sync EXECUTED -> DONE (merged automagik-si#2541, superseded by current safety contract) - skills-fable5-revamp EXECUTED -> DONE (merged automagik-si#2518, 3 LOW remain) INDEX additions: v4-home-residue-doctor (DRAFT, now the natural vehicle for issue automagik-si#2450 v4-residue findings) and codex-plugin-dogfood-remediation (IN_PROGRESS 21/24 — the most active wish had no INDEX entry, invisible to the jar<->INDEX drift lint). INDEX line for stable-release-security-gate updated APPROVED -> IN_PROGRESS 4/6 to match its WISH header.
Summary
genie updateis now the single canonical updater: an internal agent-sync phase converges every DETECTED coding agent (Claude Code, Codex, Hermes) on every invocation — including the already-at-latest short-circuit. No new command or flag; the internal envGENIE_UPDATE_SYNC_ONLY=1is the only re-entry contract (post-swap re-exec of the fresh binary + the CC SessionStart hook trigger both use it).src/lib/agent-sync.ts): manifest-managed dirs (.genie-sync.json, dir-level digest), auto-adopt-with-backup to~/.genie/state-backups/, managed-orphan removal, staged-rename atomicity, 31 tmpdir-isolated tests. Adapters: claude (skills + council.js stamp), codex (~/.codex/skills/.curated/), hermes (~/.hermes/plugins/geniesymlink + enable).genie update(6h throttle) with a CLI-less stamp fallback viaresolveStampInputs(stable~/.genie/plugins/genieroot — kills the stale-cache downgrade ping-pong); the divergedscripts/smart-install.jsfork is deleted.genie doctorgains per-agent freshness;genie uninstallremoves only manifest/signature/target-verified assets; docs (plugin README, CLAUDE.md gotchas)./councilworkflow template coerces string args (the Workflow runtime stringifies saved-workflow input — found in live QA).Reviews
Wish
.genie/wishes/agent-sync/WISH.md— plan approved via /plan; per-group execution reviews SHIP (G1 after 1 fix loop killing a HIGH data-loss vector: user<name>.olddirs were destroyed by staging pre-clean, now collision-proof suffixes); final execution review SHIP.bun run check: 806 pass / 1 skip / 0 fail.Test plan
.genie/wishes/agent-sync/validate/)bun run checkincl. council-workflow lintgenie update×2 on the reference machine → verify~/.claude/skills+workflows/council.js,~/.codex/skills/.curated/,~/.hermes/plugins/genie; then a real/councilrun