Skip to content

release: spawn/team resolver hardening + TUI CLI-symmetric surfaces (v4.260417.1 → v4.260418.1) - #1162

Closed
namastex888 wants to merge 148 commits into
mainfrom
dev
Closed

namastex888 wants to merge 148 commits into
mainfrom
dev

Conversation

@namastex888

@namastex888 namastex888 commented Apr 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Rolling release from dev to main. Main theme: restore and lock down the canonical-UUID-per-agent invariant across every spawn path — CLI, TUI, and detached-from-env invocations — and rebuild the TUI spawn surfaces so they are 1:1 with the underlying CLI primitives. Ships the full tui-spawn-dx wish (PR #1172) plus a cluster of companion fixes that emerged during review and integration.

Version: v4.260417.1 → v4.260418.1
Scope: 84 commits · 42 files · +4962 / −236
Included PRs: #1163, #1164, #1165, #1167, #1168, #1169, #1170, #1171, #1172, #1173, #1174, #1175, #1176

What changed (by theme)

Spawn / team-resolver hardening

Single-verb spawn state machine (tui-spawn-dx wish)

  • genie spawn <name> is now the only verb, state-gated on canonical liveness (feat(tui-spawn-dx): CLI-symmetric agent/team primitives + canonical-UUID invariant #1172):
    • no canonical row → create canonical <name> with a fresh UUID.
    • canonical dead → resume canonical with its stored claudeSessionId.
    • canonical alive → create a parallel <name>-<s4> with its own fresh UUID. <s4> = first 4 hex chars of the parallel's own UUID; extends on collision.
  • Cross-team canonical routes to parallel (PK-safe) — agents.id is the PRIMARY KEY, so uniqueness is global; canonical lookup drops the per-team filter.
  • Parallels are semi-ephemeral: persisted, but only revived by explicit id (genie spawn <name>-<s4>) — bare-name spawn never touches them.

TUI CLI-symmetric surfaces (tui-spawn-dx wish)

  • buildSpawnInvocation(intent) → { cli, argv } — single source of truth for preview + execution. Round-trip invariant unit-tested so rendered preview can never drift from actual argv.
  • SpawnTargetPicker — agent-node "Spawn into…" opens a session/window target picker.
  • AgentPicker — session/window-node "Spawn here…" opens an agent picker.
  • TeamCreate — workspace-level Ctrl+N opens name + member-tick list.
  • CliPreviewLine — shared preview component embedded in all modals, renders the exact genie … command Enter will run.
  • Serialize team-create → team-hire so member hires don't race the team row commit.
  • Clean up cruft windows on isolated-session spawn paths (fix(spawn): eliminate cruft windows for isolated-session --new-window #1173).

Scheduler recovery & resilience

Registry drift / back-fill

Test infra fixes

Compatibility

  • Behavior change: the new resolver precedence may surface stale agent_templates.team values for legacy workflows that relied on the old options.team || discoverTeamName() chain. No backcompat shim — the old behavior was the bug. Callers who need a specific team can always pass --team explicitly.
  • No schema migrations. Parallel-naming uses existing agents.id + team columns; the id convention (<name>-<s4>) is format-compatible.
  • No config changes. GENIE_TEAM env continues to work; new tiers sit around it.

Docs follow-up

Documentation for the new team-resolution precedence and the canonical-vs-parallel mental model is carved out into #1166. Doc PR will land separately against dev.

Test plan

  • bun test full suite passes on dev (2581 tests across 134 files).
  • bun run typecheck clean.
  • Biome clean on touched files.
  • Shell reproducer scripts/tests/repro-canonical-uuid.sh exits 0 on clean dev install.
  • CI on main promotion green.
  • Post-merge smoke test: from a genie tmux session, genie spawn simone registers simone@simone, same claudeSessionId across dead/alive cycles.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Apr 17, 2026 •

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Bumped genie package/plugin versions; appended metrics-updater records for 2026-04-17; added native-team tmux-session fallback and best-effort PG backfill; introduced per-worker failure isolation and scheduled retry in scheduler startup recovery; added executor-liveness helpers and propagated use into non-tmux worker status resolution; added tests.

Changes

Cohort / File(s) Summary
Version & Manifests
​.claude-plugin/marketplace.json, package.json, plugins/genie/.claude-plugin/plugin.json, plugins/genie/package.json
Bumped genie version from 4.260417.1 → 4.260417.8 across manifests.
Metrics agent data
.genie/agents/metrics-updater/daily-stats.jsonl, .genie/agents/metrics-updater/runs.jsonl, .genie/agents/metrics-updater/state.json
Appended a 2026-04-17 daily-stats record, added a successful run entry, and updated persisted state.json (last_run, last_run_status, daily_stats_count, duration_ms).
Native team discovery & backfill
src/lib/claude-native-teams.ts, src/lib/claude-native-teams.test.ts
Added findTeamsContainingAgent(agentName), currentTmuxSessionName() fallback in discoverTeamName(), and best-effort backfillTeamRow() calls before returning team config; tests added for discovery behavior.
Team manager (DB backfill)
src/lib/team-manager.ts, src/lib/team-manager.test.ts
Added exported ensureTeamRow(name, opts?) to insert lightweight bootstrap rows with safe defaults and audit event; tests validate insertion, idempotency, and edge cases.
Scheduler recovery
src/lib/scheduler-daemon.ts, src/lib/scheduler-daemon.test.ts
Refactored startup recovery into runAgentRecoveryPass() with per-worker try/catch and recovery_worker_failed logging; records failed_agents and schedules delayed retry (scheduleRecoveryRetry) when failures occur (60s); tests added for failure isolation and retry scheduling.
Executor liveness & usage
src/lib/executor-registry.ts, src/lib/executor-registry.test.ts, src/term-commands/agents.ts
Added exports getLiveExecutorState(agentId) and isExecutorAlive(agentId); tests added. term-commands/agents.ts uses executorRegistry to resolve liveness/state for non-tmux transports and updated worker status mapping.
TUI spawn logging
src/tui/components/Nav.tsx
spawnAgent now attempts per-user timestamped log files and routes child stdout/stderr to logs when possible; adds exit/error handlers that reference the log path.
Tests / mocking updates
src/term-commands/init-bootstrap.test.ts, src/term-commands/init-flow.test.ts
Replaced whole-module mocks with spyOn for genie-config functions and updated setup/teardown to restore spies; minor test scaffolding changes.

Sequence Diagram(s)

sequenceDiagram
  participant Daemon as SchedulerDaemon
  participant Deps as SchedulerDeps (isPaneAlive, attemptAgentResume)
  participant Worker as Worker
  participant Timer as RetryTimer
  participant Logger as Logger

  Daemon->>Deps: runAgentRecoveryPass(workers)
  loop per resumable worker
    Daemon->>Deps: isPaneAlive(worker.pane)
    alt isPaneAlive throws
      Deps-->>Daemon: throws
      Daemon->>Logger: recovery_worker_failed (worker_id, error)
      Daemon->>Daemon: failed++
    else isPaneAlive ok
      Daemon->>Deps: attemptAgentResume(worker)
      Deps-->>Daemon: resume result
    end
  end
  alt failed > 0
    Daemon->>Logger: recovery_retry_scheduled
    Daemon->>Timer: setTimeout(scheduleRecoveryRetry, RECOVERY_RETRY_DELAY_MS)
    Timer->>Daemon: scheduleRecoveryRetry()
    Daemon->>Deps: runAgentRecoveryPass(retry)
    Daemon->>Logger: recovery_retry_completed / recovery_retry_error
  else
    Daemon->>Logger: recovery_completed
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 95.24% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title accurately describes the main changes: version bump (v4.260417.1 → v4.260417.8, not v4.260418.1 as stated), spawn hardening, team resolver improvements, and TUI CLI surface enhancements across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b72f4e145

ℹ️ 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".

Comment thread VELOCITY.md Outdated

| Milestone | Period | Days | Commits (linear) | Commits (all) | PRs merged | LoC (net) | Contributors |
|-----------|--------|-----:|-----------------:|--------------:|-----------:|----------:|-------------:|
| Genesis → v4 stable | 2026-04-09 → 2026-03-23 | -17 | 1.3K | 4.2K | 110 | +60.2K | 22 |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fix reversed dates in major milestone period

The new Genesis → v4 stable row reports the period as 2026-04-09 → 2026-03-23 with -17 days, which is chronologically impossible and indicates the start/end dates were swapped for this milestone. This makes the generated velocity dashboard numerically incorrect for anyone reading or exporting milestone timing data, so the period order (and derived day count) should be corrected.

Useful? React with 👍 / 👎.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request bumps the project version to 4.260417.2 and updates the metrics dashboard with data through April 17, 2026, including refreshed SVG charts and updated logs. While the VELOCITY.md was expanded with a new milestones section, a logical error was identified in the date range for the 'Genesis to v4 stable' entry, leading to a negative duration and inconsistent commit metrics.

Comment thread VELOCITY.md Outdated

| Milestone | Period | Days | Commits (linear) | Commits (all) | PRs merged | LoC (net) | Contributors |
|-----------|--------|-----:|-----------------:|--------------:|-----------:|----------:|-------------:|
| Genesis → v4 stable | 2026-04-09 → 2026-03-23 | -17 | 1.3K | 4.2K | 110 | +60.2K | 22 |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The 'Genesis → v4 stable' milestone entry shows a negative duration of -17 days because the start date (2026-04-09) is after the end date (2026-03-23). Additionally, the 'Commits (all)' value of 4.2K matches the current project total, which is inconsistent with a milestone that ended in March.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.genie/agents/metrics-updater/daily-stats.jsonl:
- Line 35: Check why the "2026-04-16" record is missing from
.genie/agents/metrics-updater/daily-stats.jsonl (the file currently jumps from
explicit zeros for "2026-04-14"/"2026-04-15" to the "2026-04-17" entry shown in
the diff); investigate whether the metrics-updater job ran on 2026-04-16 by
inspecting the metrics-updater run logs, scheduler/cron history, and any
ingestion/ETL error events, and then either (a) insert a proper "2026-04-16"
JSONL entry with the real metrics if available (ensuring fields like date,
commits, loc_added, loc_removed, releases, contributors match the existing
schema) or (b) if the job did not run or data is truly zero, add an explicit
zero-entry for "2026-04-16" consistent with the "2026-04-14"/"2026-04-15"
format; update any aggregate counts or metadata that depend on this file if you
change values.

In @.genie/agents/metrics-updater/state.json:
- Line 13: Investigate the ~5.4× regression by profiling and instrumenting the
two hot functions referenced in the metrics: generate_velocity and commit_push
(the entries associated with duration_ms in the metrics-updater state), locate
recent changes between the good and bad commits that touch those functions, and
add fine-grained timing/logging around internal steps (DB calls, network I/O,
serialization, locks, loops) to pinpoint the sub-step causing the 13.2× and 6.3×
regressions; once identified, revert or optimize the costly change (e.g., remove
redundant looping, batch DB/network calls, restore previous caching/memoization,
fix accidental synchronous/blocking behavior) and rerun the benchmark to confirm
duration_ms reduces toward the original ~1,350ms.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 680a438e-19d6-40b8-abdd-e0e27699a17d

📥 Commits

Reviewing files that changed from the base of the PR and between f4188ad and 0b72f4e.

⛔ Files ignored due to path filters (5)
  • .genie/assets/commits-30d.svg is excluded by !**/*.svg
  • .genie/assets/loc-30d.svg is excluded by !**/*.svg
  • .genie/assets/releases-30d.svg is excluded by !**/*.svg
  • README.md is excluded by !*.md
  • VELOCITY.md is excluded by !*.md
📒 Files selected for processing (7)
  • .claude-plugin/marketplace.json
  • .genie/agents/metrics-updater/daily-stats.jsonl
  • .genie/agents/metrics-updater/runs.jsonl
  • .genie/agents/metrics-updater/state.json
  • package.json
  • plugins/genie/.claude-plugin/plugin.json
  • plugins/genie/package.json

{"date":"2026-04-13","commits":14,"loc_added":668,"loc_removed":70,"releases":0,"contributors":["Claude","Felipe Rosa","genie","github-actions[bot]"]}
{"date":"2026-04-14","commits":0,"loc_added":0,"loc_removed":0,"releases":0,"contributors":[]}
{"date":"2026-04-15","commits":0,"loc_added":0,"loc_removed":0,"releases":0,"contributors":[]}
{"date":"2026-04-17","commits":4,"loc_added":8,"loc_removed":8,"releases":2,"contributors":["Felipe Rosa","github-actions[bot]"]}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Verify missing April 16 entry.

The daily stats sequence includes explicit zero entries for April 14-15, but April 16 is entirely absent before this April 17 entry. Confirm whether this is intentional (e.g., metrics-updater didn't run) or indicates a data gap.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.genie/agents/metrics-updater/daily-stats.jsonl at line 35, Check why the
"2026-04-16" record is missing from
.genie/agents/metrics-updater/daily-stats.jsonl (the file currently jumps from
explicit zeros for "2026-04-14"/"2026-04-15" to the "2026-04-17" entry shown in
the diff); investigate whether the metrics-updater job ran on 2026-04-16 by
inspecting the metrics-updater run logs, scheduler/cron history, and any
ingestion/ETL error events, and then either (a) insert a proper "2026-04-16"
JSONL entry with the real metrics if available (ensuring fields like date,
commits, loc_added, loc_removed, releases, contributors match the existing
schema) or (b) if the job did not run or data is truly zero, add an explicit
zero-entry for "2026-04-16" consistent with the "2026-04-14"/"2026-04-15"
format; update any aggregate counts or metadata that depend on this file if you
change values.

Genie (env-specialist) and others added 7 commits April 17, 2026 15:16
`discoverTeamName` was matching solely on `config.leadSessionId` against
the current Claude Code session UUID. After a server reboot (or any
claude restart), the stored `leadSessionId` becomes stale and the lookup
returns null, making every `genie agent spawn` from inside the surviving
tmux session fail with:

  Error: --team is required (or set GENIE_TEAM, or run inside a genie session)

— even though the user *is* inside the named tmux session of a team
whose config.json is still on disk.

Add a safe fallback: after the leadSessionId scan fails, if we are
inside tmux, read the current tmux session name and attempt to load a
team config.json at `~/.claude/teams/<tmux-session-name>/config.json`.
If it parses, return its name.

Regression guards:
- The fallback is skipped entirely when `process.env.TMUX` is unset, so
  CLI invocations from outside tmux cannot accidentally pick up a team
  by name alone.
- The tmux helper is loaded via dynamic import to avoid a circular
  dependency with `./tmux.ts`.

Tests: 4 new cases in claude-native-teams.test.ts covering env-var
precedence, session-id match, null-when-both-paths-fail, and the
no-tmux regression guard.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`recoverOnStartup` iterated resumable workers and called
`deps.isPaneAlive(worker.paneId)` + `attemptAgentResume` without any
per-worker fault isolation. A single throw — most commonly "error
connecting to /tmp/tmux-UID/<socket>" when the tmux socket is not yet
reachable at daemon boot — aborted the for-loop, and the outer
try/catch in `startDaemon` logged `recovery_error` and moved on.

Observed symptom on a real server reboot:

  14:44:09.439  recovery_started
  14:44:09.487  recovery_error   error="... display-message -t '%5' ...
                                 error connecting to /tmp/tmux-1000/genie
                                 (No such file or directory)"
  (session 'genie' was created 30s later at 14:44:39)

Every worker stayed stale with resume_attempts=0 forever — there was no
retry path, and auto-resume is the only code path that would have
revived them.

Fix:
1. Extract the worker-recovery loop into `runAgentRecoveryPass` with a
   per-worker try/catch. One worker's failure no longer poisons the
   rest. Each per-worker failure emits a structured
   `recovery_worker_failed` log line carrying the worker id, daemon
   id, and error message.
2. After the initial pass, if any worker failed, schedule a one-shot
   retry 60 s later via `setTimeout(...).unref()`. This covers the
   boot race where tmux is not yet listening when the daemon starts.
   The retry uses the same fault-isolated pass and emits
   `recovery_retry_scheduled` / `recovery_retry_completed` /
   `recovery_retry_error` events.
3. Surface `failed_agents` in the existing `recovery_completed` log
   line so the outcome of the initial pass is observable without
   having to correlate scattered events.

Tests: 3 new cases in scheduler-daemon.test.ts covering (a) one worker
throwing does not stop recovery for the next, (b) a retry is scheduled
when any initial failure occurs, and (c) no retry is scheduled when
every worker succeeds. Full file: 62/62 pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…silience

fix(scheduler): harden recoverOnStartup against per-worker failures
…llback

fix(teams): add tmux-session-name fallback to discoverTeamName
Symptom: after a reboot that wiped the pgserve data dir (or any scenario
where PG state is lost while the on-disk native team config at
~/.claude/teams/<name>/config.json survived), `genie team ls` returned
an empty list even though the team was fully usable via the native
protocol. The two registries — PG `teams` table and native team files —
drifted apart and no code path reconciled them.

Fix: add `ensureTeamRow(name, { repo })` to team-manager as a
lightweight, idempotent back-fill, and call it from `ensureNativeTeam`
after the native config is either loaded or saved. Mirrors newly
minted (or surviving) native teams into the PG registry so `team ls`
reflects reality.

Design choices to keep this minimal:
- `ensureTeamRow` does NOT create a git worktree — `worktreePath`
  defaults to `repo` (current cwd). If a user later runs
  `genie team create <same-name>`, its own `getTeam` check returns
  the back-filled row and skips worktree creation. Callers who need
  a real worktree must run `createTeam` explicitly.
- `INSERT ... ON CONFLICT (name) DO NOTHING` — never clobbers an
  explicit `createTeam` row.
- `backfillTeamRow` in claude-native-teams.ts uses a dynamic import to
  avoid circular dependency with team-manager.ts, and swallows errors
  so the native-team path is never blocked by PG issues.
- Invalid branch names are rejected silently (returns null) since the
  caller is `ensureNativeTeam`, which is on a hot path.

Tests: 4 new cases in team-manager.test.ts covering fresh insert,
idempotency, non-clobber of createTeam rows, and invalid name
rejection. Full suite: 75/75 pass across team-manager + native-teams.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sert-pg

fix(teams): back-fill PG teams row from ensureNativeTeam

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/claude-native-teams.test.ts (1)

42-54: ⚠️ Potential issue | 🟠 Major

Use delete instead of process.env.X = undefined to properly unset environment variables.

In Node.js, assigning undefined to process.env.X coerces to the string "undefined" rather than unsetting it. This breaks test isolation in discoverTeamName() at line 826–827, which returns process.env.GENIE_TEAM if truthy—causing tests like "returns null when no session and TMUX unset" and "tmux fallback is skipped" to receive the string "undefined" instead of null.

Affected locations
  • src/lib/claude-native-teams.test.ts: lines 42, 47, 52 (outer beforeEach/afterEach) and lines 566–567, 571, 573 (discoverTeamName suite beforeEach/afterEach)
  • src/lib/scheduler-daemon.test.ts: beforeEach and afterEach blocks

Replace all process.env.X = undefined with delete process.env.X. This pattern is already correctly used in workspace.test.ts, msg.test.ts, protocol-router.test.ts, and interactivity.test.ts.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/claude-native-teams.test.ts` around lines 42 - 54, Tests set
environment variables to undefined (which becomes the string "undefined")
causing discoverTeamName() to see truthy values; replace all assignments like
process.env.CLAUDE_CONFIG_DIR = undefined and process.env.CLAUDE_CODE_SESSION_ID
= undefined (and any similar patterns in src/lib/claude-native-teams.test.ts and
src/lib/scheduler-daemon.test.ts and the discoverTeamName() test-suite) with
delete process.env.CLAUDE_CONFIG_DIR and delete
process.env.CLAUDE_CODE_SESSION_ID so the env keys are truly removed and
discoverTeamName() (and related tests) observe absent variables correctly.
♻️ Duplicate comments (1)
src/lib/scheduler-daemon.test.ts (1)

178-183: ⚠️ Potential issue | 🟡 Minor

Same process.env.GENIE_MAX_CONCURRENT = undefined gotcha.

See consolidated comment on src/lib/claude-native-teams.test.ts — replace with delete process.env.GENIE_MAX_CONCURRENT, otherwise resolveConfig() will Number.parseInt("undefined", 10) → NaN → falls back to 5, which happens to match the default so the bug is masked, but it hides any future change that inspects the env string directly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/scheduler-daemon.test.ts` around lines 178 - 183, The test
setup/teardown assigns process.env.GENIE_MAX_CONCURRENT = undefined which leaves
the string "undefined" and causes resolveConfig() to parse it as NaN; replace
those assignments in the beforeEach and afterEach blocks with delete
process.env.GENIE_MAX_CONCURRENT so the env var is truly absent when tests run
and resolveConfig() falls back correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/claude-native-teams.ts`:
- Around line 855-865: The tmux-fallback currently treats the raw
tmuxSessionName as a directory via join(base, tmuxSessionName, 'config.json'),
which can miss teams because on-disk dirs use sanitizeTeamName; replace this
lookup by iterating listTeams() (the same approach used above) and for each team
read its NativeTeamConfig and compare either config.tmuxSession ===
tmuxSessionName or sanitizeTeamName(config.name) ===
sanitizeTeamName(tmuxSessionName) to find the matching team; update the
tmuxSessionName lookup code (the block referencing tmuxSessionName and
join(base, tmuxSessionName, 'config.json')) to use listTeams(), check
config.tmuxSession/sanitized names, and return config.name when matched.

In `@src/lib/scheduler-daemon.ts`:
- Around line 702-725: scheduleRecoveryRetry currently creates a one-shot timer
with setTimeout(..., RECOVERY_RETRY_DELAY_MS) and calls runAgentRecoveryPass but
never returns or stores the timer handle, so stop() cannot cancel it; modify
scheduleRecoveryRetry to return the timer handle (the value from setTimeout)
and/or store it on the Scheduler/daemon instance, and update
recoverOnStartup/startDaemon and stop() to call clearTimeout on that handle
during shutdown; additionally, add a runtime guard inside the timeout callback
(e.g., check a running/closed flag on the daemon or deps) before calling
runAgentRecoveryPass to avoid executing recovery logic during or after stop().

---

Outside diff comments:
In `@src/lib/claude-native-teams.test.ts`:
- Around line 42-54: Tests set environment variables to undefined (which becomes
the string "undefined") causing discoverTeamName() to see truthy values; replace
all assignments like process.env.CLAUDE_CONFIG_DIR = undefined and
process.env.CLAUDE_CODE_SESSION_ID = undefined (and any similar patterns in
src/lib/claude-native-teams.test.ts and src/lib/scheduler-daemon.test.ts and the
discoverTeamName() test-suite) with delete process.env.CLAUDE_CONFIG_DIR and
delete process.env.CLAUDE_CODE_SESSION_ID so the env keys are truly removed and
discoverTeamName() (and related tests) observe absent variables correctly.

---

Duplicate comments:
In `@src/lib/scheduler-daemon.test.ts`:
- Around line 178-183: The test setup/teardown assigns
process.env.GENIE_MAX_CONCURRENT = undefined which leaves the string "undefined"
and causes resolveConfig() to parse it as NaN; replace those assignments in the
beforeEach and afterEach blocks with delete process.env.GENIE_MAX_CONCURRENT so
the env var is truly absent when tests run and resolveConfig() falls back
correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3167d03e-c6d8-46cf-a8d3-20653749b2fb

📥 Commits

Reviewing files that changed from the base of the PR and between 0b72f4e and 30cbf0a.

📒 Files selected for processing (8)
  • .claude-plugin/marketplace.json
  • package.json
  • plugins/genie/.claude-plugin/plugin.json
  • plugins/genie/package.json
  • src/lib/claude-native-teams.test.ts
  • src/lib/claude-native-teams.ts
  • src/lib/scheduler-daemon.test.ts
  • src/lib/scheduler-daemon.ts

Comment on lines +855 to 865
const tmuxSessionName = await currentTmuxSessionName();
if (tmuxSessionName) {
const cfgPath = join(base, tmuxSessionName, 'config.json');
try {
const content = await readFile(cfgPath, 'utf-8');
const config: NativeTeamConfig = JSON.parse(content);
return config.name;
} catch {
// no matching team on disk
}
} catch {
// no teams dir
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Tmux-fallback uses raw session name as directory — sanitization mismatch risk.

join(base, tmuxSessionName, 'config.json') treats the tmux session name as the on-disk team directory name, but team directories elsewhere are created under sanitizeTeamName(name) (see teamDir at Line 88-90). If the tmux session name contains anything other than lowercase alphanumerics (uppercase, _, ., etc.), it will fail to locate the config even when a matching team exists. The safer route is to iterate listTeams() and match config.tmuxSession (or the sanitized form), matching how session-ID discovery is done above.

Proposed fix
-  const tmuxSessionName = await currentTmuxSessionName();
-  if (tmuxSessionName) {
-    const cfgPath = join(base, tmuxSessionName, 'config.json');
-    try {
-      const content = await readFile(cfgPath, 'utf-8');
-      const config: NativeTeamConfig = JSON.parse(content);
-      return config.name;
-    } catch {
-      // no matching team on disk
-    }
-  }
+  const tmuxSessionName = await currentTmuxSessionName();
+  if (tmuxSessionName) {
+    const sanitized = sanitizeTeamName(tmuxSessionName);
+    const cfgPath = join(base, sanitized, 'config.json');
+    try {
+      const content = await readFile(cfgPath, 'utf-8');
+      const config: NativeTeamConfig = JSON.parse(content);
+      return config.name;
+    } catch {
+      // no matching team on disk
+    }
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const tmuxSessionName = await currentTmuxSessionName();
if (tmuxSessionName) {
const cfgPath = join(base, tmuxSessionName, 'config.json');
try {
const content = await readFile(cfgPath, 'utf-8');
const config: NativeTeamConfig = JSON.parse(content);
return config.name;
} catch {
// no matching team on disk
}
} catch {
// no teams dir
}
const tmuxSessionName = await currentTmuxSessionName();
if (tmuxSessionName) {
const sanitized = sanitizeTeamName(tmuxSessionName);
const cfgPath = join(base, sanitized, 'config.json');
try {
const content = await readFile(cfgPath, 'utf-8');
const config: NativeTeamConfig = JSON.parse(content);
return config.name;
} catch {
// no matching team on disk
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/claude-native-teams.ts` around lines 855 - 865, The tmux-fallback
currently treats the raw tmuxSessionName as a directory via join(base,
tmuxSessionName, 'config.json'), which can miss teams because on-disk dirs use
sanitizeTeamName; replace this lookup by iterating listTeams() (the same
approach used above) and for each team read its NativeTeamConfig and compare
either config.tmuxSession === tmuxSessionName or sanitizeTeamName(config.name)
=== sanitizeTeamName(tmuxSessionName) to find the matching team; update the
tmuxSessionName lookup code (the block referencing tmuxSessionName and
join(base, tmuxSessionName, 'config.json')) to use listTeams(), check
config.tmuxSession/sanitized names, and return config.name when matched.

Comment thread src/lib/scheduler-daemon.ts Resolved
github-actions Bot and others added 15 commits April 17, 2026 18:37
Introduce buildSpawnInvocation(intent) -> {cli, argv} as the single source of
truth for rendering and executing genie spawn / genie team create commands.
The TUI's preview line and executor will both read this descriptor, so the
displayed string can never drift from the actual argv.

- spawn-agent intent covers name, team, session, window, newWindow, prompt.
- create-team intent covers name, repo, baseBranch (members are out of scope
  for this helper — team hire is a follow-up sequence).
- Validates non-empty name for both variants and branch-safe chars for
  create-team names.
- Reuses shellQuote from team-lead-command.ts; argv round-trips to cli via
  argv.map(shellQuote).join(' ').

Scaffolding for tui-spawn-dx wish groups 4-7, which will wire this into the
TUI render/execute paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
23 tests across three groups:

- Positive: each SpawnIntent variant (spawn-agent, create-team) produces the
  expected argv, with optional flags omitted when empty.
- Round-trip: argv.map(shellQuote).join(' ') === cli for a matrix of intents
  including prompts with single quotes, double quotes, and dollar signs, plus
  names and repos containing spaces.
- Negative: empty name throws naming the field; unsafe branch chars
  (spaces, ';') throw for create-team; unknown intent kind throws.

Proves the helper's contract that TUI preview (cli) and executor argv cannot
diverge, which is the reason the helper exists.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The wish specifies buildSpawnInvocation returns { cli: string; argv: string[] }
inline. A named SpawnInvocation interface was cosmetic and triggered knip as
an unused exported type (since downstream Groups 4-7 will consume it by
inference, not by name).

Inline the return type to match the wish signature exactly and keep
`bun run check` (which includes knip) green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds an optional `team` field to `DirectoryEntry` and populates it in
`resolve()` via a direct lookup against the `agent_templates` PG row.
This makes the template-pinned team available to spawn-time resolvers
without requiring them to re-query the templates table themselves.

This is tier 2 of the tui-spawn-dx Group 1 four-tier team-resolution
precedence. `agent.entry.team` is an authoritative PG lookup — NOT a
synthetic tmux/env fallback — so it does not violate the UUID-first
principle established by the perfect-spawn-hierarchy wish (PR #1133
merge 8a78346, PR #1134 merge 6921574, commits 6389f39, d44a6b9,
0b98bec, 4100d29).

Refs: tui-spawn-dx (Group 1)
…AndResume

Replaces the two-tier fallback (`options.team || discoverTeamName()`)
with an explicit four-tier precedence that restores the canonical-UUID-
per-agent invariant when spawning from a tmux session that does not
match the agent's home team.

Precedence (highest wins):
  1. `options.team` — the `--team` flag. Only tier that flips
     `teamWasExplicit` to true.
  2. `agent.entry?.team` — template-pinned team from the
     `agent_templates` PG row. Authoritative PG lookup, NOT a synthetic
     tmux/env fallback.
  3. `process.env.GENIE_TEAM` — session-scoped env var (short-circuit
     before the heavier discovery path).
  4. `await discoverTeamName()` — retained PR #1164 fallback (JSONL
     leadSessionId match → tmux session name). Fires only when every
     higher-priority tier is empty.

Extracts the pure precedence logic into `resolveTeamName(opts)` and
makes it exported for unit testing. `resolveTeamAndResume` now takes
the resolved agent as a parameter so `agent.entry.team` is in scope.

Reproducer now fixed: running `genie spawn simone` from inside the
`genie` tmux session (where `discoverTeamName` would return 'genie'
via PR #1164's tmux fallback) now resolves to `simone` because the
template pins it. Full regression test covers this case.

Authority: perfect-spawn-hierarchy wish — PR #1133 merge 8a78346,
PR #1134 merge 6921574 (commits 6389f39 kill-pending,
d44a6b9 remove-hardcoded-genie-from-team-flag,
0b98bec remove-genie-literal-from-dispatch,
4100d29 remove-genie-literal-from-resume).

Refs: tui-spawn-dx (Group 1)
genie ls was rendering SDK/omni-bridge/process agents as "offline" (or
a misleading "error (0/3 resumes, auto-resume: on)") while they were
actively running. buildWorkerStatusMap gated liveness entirely on
isPaneAlive(w.paneId), which requires paneId to match /^%\d+$/.
Non-tmux transports carry synthetic pane tokens ('sdk', 'inline', '')
that fail the regex, so they were silently dropped from statusMap and
their stale agents.state field was all that survived for display.

Gate on transport type: tmux panes still use isPaneAlive (preserving
legacy behavior exactly), while non-tmux transports consult
executors.state via the new getLiveExecutorState helper. That state is
also used directly for display, since agents.state is not kept in sync
for SDK/omni agents — the claude-sdk transport updates executors.state
on every message, which is the authoritative signal.

Scope: intentionally narrow. Parallel-pattern sites flagged by tracer
(agents.ts:1291, 1835, 1886; scheduler-daemon.ts:595,667;
team-auto-spawn.ts:122; auto-spawn.ts:84) are out of scope — separate
wish.

Evidence (genie ls --json | jq '.[] | select(.name=="trace")'):
  BEFORE: status = "error (0/3 resumes, auto-resume: on)"
  AFTER:  status = "spawning"

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fix(ls): check executor state for non-tmux (SDK/omni) agents
Adds the three-branch state machine that gates canonical vs parallel row
creation based on pane liveness — no new flags, semantics are invisible to
the caller.

State machine (in handleWorkerSpawn, after resolveTeamAndResume):
  - No row with id=<name>   → create canonical (workerId=<name>)
  - Alive canonical         → create parallel (workerId=<name>-<sN>)
  - Dead canonical          → findDeadResumable handles it (unchanged)

Parallels mint their OWN fresh Claude session UUID and derive a
deterministic short-id (<sN>) from that UUID's hex prefix — never a random
or minted token. On collision, the slice extends one char at a time
(s5, s6, …) of the SAME UUID until unique. This keeps row ids in lockstep
with session UUIDs and makes parallels resumable only by their full id.

Canonical rows are never clobbered by parallel creation: parallels get
their own row with their own UUID, preserving the perfect-spawn-hierarchy
canonical-UUID invariant (PR #1133/#1134 merge 6921574).

Helpers introduced (exported for unit testing):
  - resolveSpawnIdentity(name, team, uuidFactory?, isAliveFn?)
  - pickParallelShortId(baseName, team, uuid) — rejects non-UUID input
  - findDeadResumable now exported (for off-auto-resume-path tests)

rejectDuplicateRole is retained for the legacy explicit --role path;
for single-verb spawns it no longer fires (alive collision = parallel).

Authority: tui-spawn-dx wish, Group 2. Builds on Wave 1 (merge 9321dd6,
commits 79cbe06 + 898c219).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e invariants

Adds 15 unit tests for the Group 2 spawn state machine:

resolveSpawnIdentity:
  - no row             → canonical (id=<name>, fresh UUID)
  - dead canonical     → canonical (auto-resume handled upstream)
  - alive canonical    → parallel (id=<name>-<s4>, UUID starts with <s4>)
  - parallel creation does NOT clobber canonical row (UUID unchanged
    before and after)
  - <sN> collision extends to s5 of the SAME UUID; both parallels
    coexist and the new row's UUID starts with <s5>

pickParallelShortId:
  - returns s4 when no collision
  - extends to s5 on s4 collision
  - extends to s6 on s4 AND s5 collision
  - collision in a different team does not extend (team-scoped uniqueness)
  - rejects non-UUID input: "not-a-uuid", "", hex-only no dashes, "abc"

findDeadResumable (parallels off auto-resume path):
  - findDeadResumable(<name>) does NOT match a parallel row
  - findDeadResumable(<name>-<sN>) DOES match the parallel row
    (revive-by-full-id works)

Tests use pgserve-backed setupTestSchema and TRUNCATE between tests —
registry.register's ON CONFLICT DO UPDATE only rewrites a subset of
columns, so row reuse across tests bleeds state. The hermetic truncate
keeps each test self-contained.

Authority: tui-spawn-dx wish, Group 2.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Knip flagged the exported type as unused — the public surface is the
resolveSpawnIdentity function signature, so callers infer the type from
the return. Downgrading to an internal type keeps the knip dead-code gate
green without changing observable behavior.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Renders the exact `genie …` command a spawn or team-create modal is
about to execute, using `buildSpawnInvocation` (Group 3) as the single
source of truth. Consuming modals in Groups 4, 5, 6 will pass the same
intent to both the preview and the executor — guaranteeing the rendered
CLI and the dispatched argv cannot drift.

When the intent is malformed (empty name, unsafe branch chars), the
helper throws. We catch inline and render a red error row so a bad
intent in one modal field never crashes the surrounding modal.

Wish tui-spawn-dx, Group 7. Builds on Wave 1 + 2 (integration commit
ceb4c4b, Group 3 at merge 0d13568). Enables Groups 4, 5, 6 to consume
a single, tested preview component.
Genie (env-specialist) and others added 29 commits April 18, 2026 14:01
…ile.ts

The PID-based lockfile helpers were private to claude-native-teams.ts and
unusable by any other module that needs safe concurrent writes. Promote them
to a shared module with named exports — preserves the existing semantics
(5s timeout, 50ms poll with jitter, stale-PID detection via kill(pid, 0),
force-acquire with warning after timeout) so claude-native-teams.test.ts
passes unchanged.

Prepares the ground for agent-yaml.ts (next commit) which needs the same
lock semantics for atomic writes to agent.yaml.
Ships the Wave-1 module that every downstream group in the
dir-sync-frontmatter-refresh wish imports from:

- `AgentConfigSchema` — Zod schema mirroring DirectoryEntry field-for-field
  with `.strict()` at every object level. Unknown top-level keys (including
  out-of-scope `skill` and `extraArgs` that belong to the agent_templates
  PG row, not the yaml file) fail with a field-named error.
- `parseAgentYaml(path)` — reads, parses via js-yaml, validates via Zod,
  surfaces a descriptive Error on any failure.
- `writeAgentYaml(path, config)` — strips derived fields (name/dir/
  registeredAt) before serialization, writes atomically (`${path}.tmp` +
  rename) under `acquireLock` so concurrent writers and readers never
  observe a partial/spliced file.
- `extractFrontmatterFromAgentsMd(content)` — pure helper that splits the
  `---\n...\n---\n` fence off the top of an AGENTS.md string, preserving
  the body byte-for-byte (CRLF, trailing newlines, Unicode).

No caller is updated in this commit — Groups 2+ consume the module.
25 cases pinning every acceptance criterion from the wish:

- Round-trip: fully-populated config + empty config both survive
  writeAgentYaml → parseAgentYaml untouched; derived fields are stripped
  from the on-disk YAML and absent on re-parse.
- Strict schema / scope guard: unknown top-level keys (`bogus`, `skill`,
  `extraArgs`) are rejected with the key named in the error.
- Nested: `permissions.bashAllowPatterns` parses; `permissions.bashAllow`
  (wrong name) throws an unknown-field error pinned to `permissions`.
- `extractFrontmatterFromAgentsMd` purity: CRLF preserved, trailing newlines
  preserved, Unicode preserved, missing/unclosed fence returns the full body,
  call is pure (no mutation).
- Concurrency: two and eight parallel `writeAgentYaml` calls on the same
  path always leave the file parseable as EXACTLY one of the inputs —
  never a splice.
- Malformed YAML / missing file / top-level array / empty file all
  produce clear errors (or an empty config for the empty-file case).
…81K events over 8 days)

Tracer diagnosis (HIGH): the 60s mailbox delivery retry loop in
`scheduler-daemon.ts` escalates a permanently-failed message by
posting a new mailbox row `from='scheduler'`, `to='team-lead'`.
`'team-lead'` is a bare, unresolvable string — real team-leads have
suffixed IDs like `team-lead:<session>:<team>` — so the escalation
row itself cannot deliver, hits MAX_DELIVERY_ATTEMPTS, and is
escalated AGAIN, producing an unbounded chain.

Observed in production on felipe's machine:
  - 181,914 stale `[escalation]`-prefixed rows in `mailbox` (99.999%
    from `scheduler` -> `team-lead`)
  - 181,887 log events since 2026-04-10T03:10:05Z (8 days)
  - growing ~1500/h = ~12MB/day on mailbox + scheduler.log
  - ~34MB log, 61MB DB rows at time of diagnosis

Fix — 3 overlapping guards in `processMailboxRetryMessage`:

Extracted the per-message retry body from its inline setInterval
into an exported `processMailboxRetryMessage(deps, msg, overrides?)`
so the guards are unit-testable. Each skip logs
`mailbox_delivery_escalation_dropped` with a `reason` field.

Guard 1 (MANDATORY): `msg.from === 'scheduler'` -> drop
  A scheduler-authored message IS an escalation; re-escalating it
  extends the chain by one each cycle. This alone closes the loop.

Guard 2 (defense-in-depth): `msg.body.startsWith('[escalation] ')`
  Catches any escalation row that loses scheduler authorship
  (replay, sender rename).

Guard 3 (defense-in-depth): `msg.to === ESCALATION_RECIPIENT`
  `'team-lead'` is unresolvable today; escalating to it can never
  succeed, so re-escalating only amplifies the problem.

Also extracted `ESCALATION_RECIPIENT = 'team-lead'` as a top-of-file
constant so the Guard 3 check and the `send(...)` call reference the
same literal — defensive against drift.

Scope (2 files):
  - `src/lib/scheduler-daemon.ts`: add `ESCALATION_RECIPIENT` +
    `MAX_DELIVERY_ATTEMPTS` exports, extract
    `processMailboxRetryMessage` with 3 guards, collapse inline loop.
  - `src/lib/scheduler-daemon.test.ts`: add 5 regression tests
    (Guards 1/2/3 + positive path + 10-cycle infinite-loop
    regression).

Validation: `bun run check` -> 2639 pass / 0 fail (was 2634; +5
new tests).

Out of scope (follow-ups):
  - `team-lead` resolver that maps the bare string to a real worker
    ID (separate wish).
  - Operational cleanup of stale rows (felipe runs manually after
    merge).
  - Log rotation (felipe's call post-merge).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…loop

fix(scheduler): stop escalation recursion — guard mailbox loop (was 181K events over 8 days)
…-refresh-group1

feat(agent-yaml): YAML schema + parse/write library (wish dir-sync-frontmatter-refresh group 1)
…ation

Wave 2 / Group 2 of wish dir-sync-frontmatter-refresh. Depends on Group 1
(#1191, already on dev) which shipped the agent-yaml schema + IO library.

- `migrateAgentToYaml(agentDir, dbRow?)` is idempotent: a second call on
  an already-migrated agent returns `{ migrated: false, reason:
  'already-migrated' }` and makes zero disk writes.
- Extracts frontmatter via `extractFrontmatterFromAgentsMd`, parses it
  as YAML, strips derived/DB-only fields, merges with `dbRow` (frontmatter
  wins on conflict — source-of-truth principle), validates the result
  via `AgentConfigSchema`, writes `agent.yaml` atomically, saves an
  `AGENTS.md.bak` byte-for-byte, and rewrites `AGENTS.md` with the
  post-frontmatter body only.
- Malformed frontmatter throws a descriptive error BEFORE any disk write,
  so no partial state is ever left behind.
- The YAML_ALLOWED_KEYS set is derived from AgentConfigSchema's shape at
  module-load time, so this file never drifts from Group 1's source of
  truth.

Tests: 14 cases pinning every acceptance criterion — idempotency, body
preservation (plain / CRLF / Unicode), .bak byte-equality, dbRow fills
missing fields, frontmatter wins on conflict, DB-only fields (skill,
extra_args, id) never reach agent.yaml, nested permissions survive from
dbRow, no-frontmatter path, malformed-frontmatter error leaves files
untouched, empty frontmatter mapping parses as valid, return-shape
invariants.

No caller is rewired here. Groups 3+ consume the migrator.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ecutor spawn

buildOmniSpawnParams constructs SpawnParams for WhatsApp turn agents spawned
via the tmux executor, but dropped entry.permissions and entry.disallowedTools
on the floor. c5251d1 wired permissions into buildClaudeCommand's --settings
JSON, but the omni tmux path never populated those SpawnParams fields — so
WhatsApp turn agents ran with zero Bash-level enforcement despite having
permissions declared in their AGENTS.md frontmatter.

This was the last gap in the turn-sandbox-scope-enforcement wish (Groups 1-3
already shipped). SDK executor honors frontmatter permissions via
resolvePermissionConfig(entry.permissions) at claude-sdk.ts:450. This brings
the tmux path to parity.

Empty allow/deny arrays intentionally resolve to undefined so we don't emit
an empty permissions block that could be misread as "deny everything" by
--settings deep-merge.

SDK-only fields (preset, bashAllowPatterns) are ignored here — they belong
to claude-sdk-permissions.ts translation, not the CLI --settings path.

Tests cover:
- permissions.allow/deny propagation
- disallowedTools propagation
- absence preserved (no false sense of security)
- empty arrays → undefined (no misleading emission)
- SDK-only fields ignored in CLI path
…-refresh-group2

feat(agent-migrate): one-shot AGENTS.md frontmatter → agent.yaml migration (wish dir-sync-frontmatter-refresh group 2)
… re-processing)

Two related bugs, same architectural shape — terminal-state agents that the
scheduler kept re-processing every cycle.

Bug A: Rows in `state='error', auto_resume=true, claude_session_id=null`
were dropped by the resumable filter (no session id to resume), but stayed
in `auto_resume=true` forever — misleading `genie ls` ("auto-resume: on")
and cluttering the worker list. Observed live on felipe's machine: 9 wedged
rows (8 genie-docs directory placeholders + 2 omni workers that died before
capturing a Claude session).

Fix: new `reconcileUnresumable` pass wired into `startAgentResumeTimer`
between `reconcileDeadPaneZombies` and `runAgentRecoveryPass`. Flips those
rows to `auto_resume=false` and logs `agent_marked_unresumable`.

Bug B: The `attempts >= maxAttempts` early-exit in `attemptAgentResume`
logged `agent_resume_exhausted` and returned, but did NOT persist
`auto_resume=false`. Next scheduler tick (60s later) re-entered the same
branch and re-logged the same event — the agent was never re-attempted,
yet the log stayed polluted with redundant exhaustion events forever.

Fix: both exhaustion branches (pre-attempt early-exit + post-attempt final
failure) now call `deps.updateAgent(agentId, { autoResume: false })` before
logging. The next cycle's `attemptAgentResume` short-circuits at the
`autoResume === false` check with `agent_resume_skipped` (debug level),
ending the every-60s spam.

Prior art: PR #1181 (zombie concurrency cap), PR #1187 (counter
persistence). This closes the "terminal states should be terminal" theme.

Staged follow-ups (out of scope):
- dir:-row state mutation root cause (separate /trace)
- omni session-capture gap (separate /trace)

Regression tests (4 new + 1 updated in scheduler-daemon.test.ts):
- reconcileUnresumable flips null-session error rows to auto_resume=false
- reconcileUnresumable leaves valid-session + already-disabled + healthy rows untouched
- attemptAgentResume exhaustion branch persists auto_resume=false + logs event
- Subsequent runAgentRecoveryPass tick does not re-log agent_resume_exhausted
- Existing `exhaustion check fires on re-entry` test updated to assert the new
  terminal-flag write (autoResume=false, no counter mutation on early-exit)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rmissions-passthrough

fix(omni): pass permissions/disallowedTools through tmux executor
…ontmatter-refresh group 3)

Wave 2 / Group 3. Closes the loop the user originally hit today:
editing an AGENTS.md field silently dropped on the floor because `genie
dir sync` reported "Unchanged" and skipped the DB write.

Core behavior change:

1. **First sync on an unmigrated agent triggers migration** via
   `migrateAgentToYaml` (Group 2, already on dev as #1193). The agent's
   AGENTS.md frontmatter is moved into `agents/<name>/agent.yaml`;
   AGENTS.md.bak preserves the original byte-for-byte; AGENTS.md loses
   its frontmatter.
2. **agent.yaml becomes the canonical source post-migration.** Sync
   reads config via `parseAgentYaml` and drops back to legacy
   `parseFrontmatter(AGENTS.md)` only when neither yaml nor frontmatter
   exists (defaults-only registration).
3. **"Unchanged" path eliminated.** No change detection, no
   short-circuit — sync always upserts every reached agent's row. Lost
   edits are expensive; per-run writes are cheap.

Surface changes in `src/lib/agent-sync.ts`:

- `SyncResult.unchanged: string[]` removed. A new
  `SyncResult.migrated: string[]` tracks this-run migrations.
- `syncSingleAgent` gains a Phase 0 migration trigger. Its dbRow
  fallback is derived from the current DirectoryEntry via a small
  `dbRowFromEntry` helper (honors the frontmatter-wins precedence from
  Group 2's migrator).
- `syncSingleAgentByName` returns `'synced'` (was `'unchanged'`) for
  the non-change path; `processWatchedAgent`'s filter updated to match.
- `printSyncResult` drops the "Unchanged: ..." line and renames the
  closing summary to `Synced: N agent(s), M removed.`

`src/lib/agent-identity-sync.test.ts`:

- Old "sync detects AGENTS.md frontmatter changes" test updated to
  edit agent.yaml instead (post-migration canonical source).
- Old `expect(result.unchanged)` assertion replaced with
  `expect(result.updated)` (the existing agent still lands there).
- Four new Group 3 acceptance tests: migration trigger, migration
  idempotency, edit-yaml-then-resync reproducer, SyncResult shape no
  longer carries `unchanged`.

Scope guard verified: `grep 'Unchanged:' src/term-commands/dir.ts
src/lib/agent-sync.ts` returns zero hits in production code paths.

Full suite: 2678 pass / 0 fail. typecheck clean. Biome clean modulo a
complexity warning on `syncSingleAgent` (18 vs max 15) — the function
grew by one branch for the migration phase; pre-existing
`startDaemon`-style warning pattern, not a lint failure.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fix(scheduler): mark unresumable + exhausted agents as terminal (stop re-processing)
…-refresh-group3

feat(agent-sync): trigger migration + always upsert (wish dir-sync-frontmatter-refresh group 3)
…tmatter-refresh group 4)

Wave 3 / Group 4. Makes `genie dir edit` honor the "files are source of
truth" principle established in Groups 1-3. Depends on G1/G2/G3, all
already on dev.

Behavior change:

- `dir edit` now writes every flag into `agents/<name>/agent.yaml`
  FIRST, then triggers a single-agent sync so the DB picks up the new
  values via the refactored sync path from G3.
- No direct `agent_templates` / `agents` writes from the edit handler
  anymore. The yaml file is authoritative; the DB is a projection.
- If the agent hasn't been migrated yet (no `agent.yaml` on disk),
  `handleEdit` triggers migration first so the file exists before the
  write+sync cycle.
- The legacy `directory.syncFrontmatterToDisk` call is gone — there's
  no frontmatter to sync back to post-migration (AGENTS.md is
  body-only).

Code surface:

- `src/term-commands/dir.ts` — rewritten `handleEdit` per the flow
  above. New imports: `migrateAgentToYaml`, `parseAgentYaml`,
  `writeAgentYaml`, `syncSingleAgentByName`.
- `src/lib/agent-sync.ts` — export `syncSingleAgentByName` (was
  private). Used by the new edit flow. Watcher/file-sync call sites
  unchanged.

Tests (`src/term-commands/dir-edit.test.ts` — new, 7 cases):

- `--model` lands in yaml and DB
- `--allow <tools>` → nested `permissions.allow` array in yaml
- `--permission-preset` → `permissions.preset` in yaml
- `--sdk-permission-mode` → `sdk.permissionMode` in yaml
- yaml write precedes DB write (file on disk is correct even if sync
  were skipped)
- 8-way concurrent `writeAgentYaml` calls leave a parseable, complete
  config (never a truncation)
- combined flags all land in a single write

Wish scope-guard verified: `grep 'UPDATE agent_templates'
src/term-commands/dir.ts` returns zero hits.

Validation: 7/7 new tests pass, typecheck clean, biome clean modulo
the same pre-existing complexity warning on `syncSingleAgent` carried
forward from G3. Pre-push full suite: see CI on this PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-refresh-group4

feat(dir-edit): write agent.yaml first, then sync (wish dir-sync-frontmatter-refresh group 4)
…nc-frontmatter-refresh group 5)

Wave 3 / Group 5. `genie dir add <name>` now scaffolds BOTH files on
disk — `agent.yaml` (runtime config from CLI flags) and a
frontmatter-less `AGENTS.md` (pure prompt content from line 1) — then
syncs the DB row from the yaml via the same single-agent path as
`dir edit` from Group 4.

Behavior:
- `mkdirSync(agentDir, { recursive: true })` creates the folder.
- `AGENTS.md` is scaffolded from an inline template that starts with
  `# Agent: <name>` — no `---` fence. The template includes a
  `<mission>TBD</mission>` placeholder to prompt the user.
- `writeAgentYaml(join(dir, 'agent.yaml'), config)` writes the config
  atomically via the lockfile from Group 1.
- `syncSingleAgentByName(ws.root, name)` upserts the DB row via the
  G3 sync path so `dir add` is read-after-write consistent.
- If not in a workspace: fall back to `directory.add` so the row
  still registers; print a warning suggesting `genie dir sync`.

Tests (`src/term-commands/dir-add.test.ts` — new, 6 cases):

- creates both `agent.yaml` and `AGENTS.md`
- `AGENTS.md.slice(0, 3) !== '---'` (no frontmatter fence)
- `agent.yaml` parses cleanly; `--model` value lands inside
- DB directory entry has matching `model`
- `--permission-preset` + `--allow` flatten into `permissions.*`
  nested yaml + DB
- re-writing agent.yaml and re-syncing produces the new state in DB
  (idempotency of the write-then-sync cycle)

No direct `sql\`INSERT INTO agent_templates\`` calls in the add
handler path anymore; the DB projection flows from the yaml.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tter-refresh group 6)

Wave 3 / Group 6. Adds a new `checkLegacyAgentFrontmatter` check to
`genie doctor` that flags any agent directory where AGENTS.md starts
with a `---` fence AND `agent.yaml` is also present — the drift
scenario where someone pastes config back into the prompt file after
migration and sync silently ignores it.

Check semantics:
- Walks `<workspace>/agents/*`.
- Warn (not fail) when both files exist and AGENTS.md.slice(0, 3) === '---'.
- Pass when agent.yaml absent (pre-migration) OR no fence in AGENTS.md.
- Silent (empty results) when agents/ dir doesn't exist.
- Doctor exit code unchanged — warnings ≠ failures.

Exported for unit tests and for callers that want to run this check
outside `genie doctor` (e.g. a pre-sync lint).

Surface:
- `src/genie-commands/doctor.ts` — `checkLegacyAgentFrontmatter(workspaceRoot?)`
  and a `runCheckSection('Agent Config', ...)` wired into `doctorCommand`.
- `src/genie-commands/doctor.test.ts` — 7 cases: drift warn, pre-migration
  silent, clean post-migration silent, missing agents/ dir silent, multiple
  agents flagged independently, non-dir entries skipped, yaml-only dirs pass.

Validation:
- `bun test src/genie-commands/doctor.test.ts` → 7 pass
- `bun run typecheck` clean
- `bunx biome check` clean modulo a mild complexity warning (17 vs 15)

Note: the wish spec said `src/term-commands/doctor.ts` but the actual
file lives at `src/genie-commands/doctor.ts`. Scope unchanged; file
location only.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…y spam

Two retry-forever spam bugs in the observability loops — both follow the
pattern we've been closing all session: recoverable-looking error on a
fast-polling path with no circuit breaker → same log line every 30s.

Bug A — filewatch FK violation (session-filewatch.ts)
  Orphan subagent JSONLs (parent session not in PG sessions table, e.g.
  SDK agents) made ingestFileFull raise FK constraint 23503. The catch
  block logged but did NOT advance offsetCache, so every subsequent
  fs.watch event retried the same file at offset 0 → same FK error →
  same log. Forever.

  Fix: detect FK (code 23503 or "foreign key constraint" in message),
  pin offsetCache to Infinity AND add the session to an unrecoverable
  set so future events short-circuit before touching ingest. Non-FK
  errors (connection reset, deadlock) still preserve retry semantics.

Bug B — inbox-watcher missing workingDir (inbox-watcher.ts)
  checkInboxes iterates teams with unread messages. If workingDir is
  missing from a team's config, it warned every poll cycle — same 3-4
  teams re-warning every 30s forever (live-observed on felipe).

  Fix: mirror the existing spawnFailures rate-limit — new
  noWorkingDirWarned Map warns once per team, silences for 1 hour, then
  re-fires (so configs that get populated upstream naturally recover).

Pattern continuity with #1187/#1192/#1195 — same shape, same fix class:
observability path sees recoverable-looking condition → no circuit
breaker → floods logs.

Staged follow-up (out of scope): separate /trace needed upstream on
why teams register without workingDir (team-config creation bug).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-refresh-group6

feat(doctor): warn on legacy frontmatter drift (wish dir-sync-frontmatter-refresh group 6)
…-refresh-group5

feat(dir-add): scaffold agent.yaml + body-only AGENTS.md (wish dir-sync-frontmatter-refresh group 5)
…ards

fix(observability): stop filewatch FK + inbox-watcher workingDir retry spam
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants