Repository navigation
chore: rolling promotion dev -> main - #1041
Merged
Merged
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBumps the genie package/plugin version; records a successful metrics-updater run; relaxes branch-guard to require explicit Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/runs.jsonl:
- Line 19: Normalize run status when a run contains errors/fallback or zero API
calls: if the JSON record's errors array is non-empty or fallback is true or
api_calls == 0 then set status to "error" (or a distinct "partial_failure")
instead of "success"; update the code that writes/normalizes runs (the logic
that sets the "status" field) to inspect errors, fallback, and api_calls and
derive status accordingly, ensuring downstream monitors see failures; update any
tests or validation that assert status values to reflect this normalization for
records like the one containing errors/errors + fallback.
In @.genie/agents/metrics-updater/state.json:
- Around line 2-5: The state JSON uses last_run and last_run_status but
run-metrics.sh writes last_run_at and never updates last_run_status; update
run-metrics.sh to consistently use one timestamp key (prefer last_run_at to
match existing writer usage) and ensure it also writes/updates a status field
(rename or replace last_run_status with last_run_status or last_run_state
consistently) on every run—modify the code paths in run-metrics.sh that write
the state (the section around the timestamp write at line ~208) to write the
chosen timestamp key (last_run_at) and set the status key to the current result
(e.g., "success" or "failure") every time so the state.json no longer
accumulates stale/dead keys.
🪄 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: 90b09a5d-e22f-4f60-a77d-f0246b498eb9
⛔ Files ignored due to path filters (1)
README.mdis excluded by!*.md
📒 Files selected for processing (2)
.genie/agents/metrics-updater/runs.jsonl.genie/agents/metrics-updater/state.json
| {"timestamp":"2026-04-01T00:00:00Z","duration_ms":8000,"api_calls":0,"tools_generated":0,"errors":["gh CLI unavailable","GitHub MCP tools not loaded in session"],"status":"failed","fallback":false,"metrics":null} | ||
| {"timestamp":"2026-04-02T00:00:00.000Z","duration_ms":15000,"api_calls":0,"tools_generated":0,"errors":["gh CLI not available","GitHub MCP tools not present in session"],"status":"failed","fallback":false,"metrics":null} | ||
| {"timestamp":"2026-04-03T00:00:00.000Z","duration_ms":3200,"api_calls":0,"tools_generated":0,"errors":["gh CLI not available","GitHub MCP tools not present in session"],"status":"failed","fallback":false,"metrics":null} | ||
| {"timestamp":"2026-04-04T00:00:00.000Z","duration_ms":30000,"api_calls":0,"tools_generated":0,"errors":["gh CLI not available","GitHub MCP tools not present in session"],"status":"success","fallback":true,"metrics":{"releases_24h":2,"merged_prs_7d":35,"avg_merge_time_h":0.3,"ship_rate_pct":85}} |
There was a problem hiding this comment.
status: "success" conflicts with this run’s error state.
Line 19 records errors plus fallback:true and zero API calls, but marks status as success. Any status-only monitor will treat this as healthy and miss the tooling outage.
Proposed normalization
-{"timestamp":"2026-04-04T00:00:00.000Z","duration_ms":30000,"api_calls":0,"tools_generated":0,"errors":["gh CLI not available","GitHub MCP tools not present in session"],"status":"success","fallback":true,"metrics":{"releases_24h":2,"merged_prs_7d":35,"avg_merge_time_h":0.3,"ship_rate_pct":85}}
+{"timestamp":"2026-04-04T00:00:00.000Z","duration_ms":30000,"api_calls":0,"tools_generated":0,"errors":["gh CLI not available","GitHub MCP tools not present in session"],"status":"failed","fallback":true,"metrics":{"releases_24h":2,"merged_prs_7d":35,"avg_merge_time_h":0.3,"ship_rate_pct":85}}📝 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
| {"timestamp":"2026-04-04T00:00:00.000Z","duration_ms":30000,"api_calls":0,"tools_generated":0,"errors":["gh CLI not available","GitHub MCP tools not present in session"],"status":"success","fallback":true,"metrics":{"releases_24h":2,"merged_prs_7d":35,"avg_merge_time_h":0.3,"ship_rate_pct":85}} | |
| {"timestamp":"2026-04-04T00:00:00.000Z","duration_ms":30000,"api_calls":0,"tools_generated":0,"errors":["gh CLI not available","GitHub MCP tools not present in session"],"status":"failed","fallback":true,"metrics":{"releases_24h":2,"merged_prs_7d":35,"avg_merge_time_h":0.3,"ship_rate_pct":85}} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.genie/agents/metrics-updater/runs.jsonl at line 19, Normalize run status
when a run contains errors/fallback or zero API calls: if the JSON record's
errors array is non-empty or fallback is true or api_calls == 0 then set status
to "error" (or a distinct "partial_failure") instead of "success"; update the
code that writes/normalizes runs (the logic that sets the "status" field) to
inspect errors, fallback, and api_calls and derive status accordingly, ensuring
downstream monitors see failures; update any tests or validation that assert
status values to reflect this normalization for records like the one containing
errors/errors + fallback.
Comment on lines
+2
to
+5
| "last_run": "2026-04-04T00:00:00.000Z", | ||
| "last_run_status": "success", | ||
| "last_metrics": { | ||
| "releases_24h": 0, | ||
| "releases_24h": 2, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Writer paths =="
rg -n -C2 "last_run_at|last_run_status|last_run" .genie/agents/metrics-updater/tools/run-metrics.sh
echo
echo "== Readers/usages in .genie =="
rg -n -C2 "\"last_run_at\"|\"last_run_status\"|\"last_run\"" .genieRepository: automagik-dev/genie
Length of output: 719
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== State file reader locations =="
rg -r -n "state.json" .genie --max-count=20
echo
echo "== Python code reading/accessing state dict =="
rg -n "state\[" .genie/agents/metrics-updater/tools/ -A 2 -B 1
echo
echo "== Any references to last_run or last_run_at in Python code =="
rg -n "last_run" .genie/agents/metrics-updater/ --type py -A 2 -B 1Repository: automagik-dev/genie
Length of output: 4514
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Contents of metrics-updater agent directory =="
ls -la .genie/agents/metrics-updater/
echo
echo "== Content of actual state file (-n) =="
cat .genie/agents/metrics-updater/-n
echo
echo "== Check if state.json exists =="
if [ -f .genie/agents/metrics-updater/state.json ]; then
cat .genie/agents/metrics-updater/state.json
else
echo "state.json does NOT exist"
fi
echo
echo "== Search for code that reads/uses last_run_status =="
rg -n "last_run_status" .genie --type py --type shRepository: automagik-dev/genie
Length of output: 580
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full content of state.json =="
cat .genie/agents/metrics-updater/state.json
echo
echo "== Examine run-metrics.sh write logic more carefully =="
sed -n '200,220p' .genie/agents/metrics-updater/tools/run-metrics.shRepository: automagik-dev/genie
Length of output: 1012
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Code that reads/accesses last_run, last_run_at, or last_run_status =="
rg -n "last_run|last_run_at|last_run_status" .genie/agents/metrics-updater/tools/ --type py --type sh -A 3 -B 1
echo
echo "== Any health checks or status reporting that might use last_run_status =="
rg -n "status" .genie/agents/metrics-updater/AGENT.md -A 2 -B 1Repository: automagik-dev/genie
Length of output: 1410
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for external code reading state.json fields =="
rg -n "last_run_status|last_run_at|last_run" . --type py --type sh --type json -g "!\.genie" | head -20
echo
echo "== Check if there are any monitoring or health check scripts =="
fd -e sh -e py | xargs grep -l "health\|status\|metrics-updater" 2>/dev/null | head -10
echo
echo "== Verify the actual schema inconsistency by checking what keys exist after next write =="
echo "After run-metrics.sh completes, state.json will have:"
echo " - last_metrics (updated)"
echo " - last_run_at (updated/added)"
echo " - run_count (updated)"
echo " - last_run (STALE - never touched)"
echo " - last_run_status (STALE - never touched)"Repository: automagik-dev/genie
Length of output: 119
Align state.json keys with the writer's actual field names.
The state file contains last_run and last_run_status, but run-metrics.sh writes last_run_at (line 208) and never updates last_run_status. After each run, the state file accumulates dead keys (last_run untouched) and perpetually stale last_run_status. Standardize on one timestamp field name and ensure status is actively maintained on each update.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.genie/agents/metrics-updater/state.json around lines 2 - 5, The state JSON
uses last_run and last_run_status but run-metrics.sh writes last_run_at and
never updates last_run_status; update run-metrics.sh to consistently use one
timestamp key (prefer last_run_at to match existing writer usage) and ensure it
also writes/updates a status field (rename or replace last_run_status with
last_run_status or last_run_state consistently) on every run—modify the code
paths in run-metrics.sh that write the state (the section around the timestamp
write at line ~208) to write the chosen timestamp key (last_run_at) and set the
status key to the current result (e.g., "success" or "failure") every time so
the state.json no longer accumulates stale/dead keys.
Removed the deny rule that blocked PR creation targeting production branches. Agents CAN propose PRs to any branch — PRs are proposals for human review. The protection is on merging, not proposing. Kept: require explicit --base flag, block merge commands, block direct push.
…lease-prs fix: allow agents to create release PRs to any branch
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/__tests__/branch-guard.test.ts (1)
96-106: 🧹 Nitpick | 🔵 TrivialMissing test coverage for
--base=valuesyntax.The
ghCLI accepts both--base mainand--base=main. If you fix the regex issue in the handler, add test cases to ensure both syntaxes are handled:Suggested additions
// PR targeting main/master (creating PRs is proposing, not merging) 'gh pr create --base main --title "test"', 'gh pr create --base master --title "test"', 'gh pr create -B main --title "test"', 'gh pr create -B master --title "test"', + 'gh pr create --base=main --title "test"', + 'gh pr create --base=dev --title "test"',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/__tests__/branch-guard.test.ts` around lines 96 - 106, The tests in src/hooks/__tests__/branch-guard.test.ts only cover space-separated flag syntax and miss the equals form; add test cases mirroring the existing ones but using --base=dev, --base=main, --base=master and -B=main/-B=master forms (e.g., 'gh pr create --base=dev', 'gh pr create -B=main') to ensure both syntaxes are accepted, and update the branch-guard hook's flag-parsing regex (the handler logic that parses the 'gh pr create' command in the branch-guard hook) to accept both '--base value' and '--base=value' (and equivalent -B=) forms so the tests pass.
🤖 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/hooks/handlers/branch-guard.ts`:
- Around line 29-36: The test predicate in the branch guard (the object with
test: (cmd) => /gh\s+pr\s+create\b/.test(cmd) && !/(--base|-B)\s+\S/.test(cmd))
fails to catch --base=value syntax; update the regex used in that test so it
accepts either whitespace or an equals sign after --base (and similarly handle
-B), e.g. change the negative check to look for (--base|-B) followed by either =
or whitespace before the value (so --base=main and --base main both match)
within the same test function.
---
Outside diff comments:
In `@src/hooks/__tests__/branch-guard.test.ts`:
- Around line 96-106: The tests in src/hooks/__tests__/branch-guard.test.ts only
cover space-separated flag syntax and miss the equals form; add test cases
mirroring the existing ones but using --base=dev, --base=main, --base=master and
-B=main/-B=master forms (e.g., 'gh pr create --base=dev', 'gh pr create
-B=main') to ensure both syntaxes are accepted, and update the branch-guard
hook's flag-parsing regex (the handler logic that parses the 'gh pr create'
command in the branch-guard hook) to accept both '--base value' and
'--base=value' (and equivalent -B=) forms so the tests pass.
🪄 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: 35d32d55-a78f-429b-b32a-4f49a1a6e0ff
📒 Files selected for processing (2)
src/hooks/__tests__/branch-guard.test.tssrc/hooks/handlers/branch-guard.ts
Comment on lines
+29
to
36
| // Agents CAN create PRs targeting any branch (including main/master). | ||
| // PRs are proposals for human review — the protection is on MERGING, not proposing. | ||
| { | ||
| // gh pr create --base main, gh pr create -B master | ||
| test: (cmd) => /gh\s+pr\s+create\b/.test(cmd) && /(--base|-B)\s+(main|master)\b/.test(cmd), | ||
| reason: 'BLOCKED: PRs must target dev, not main/master. Use: gh pr create --base dev', | ||
| }, | ||
| { | ||
| // gh pr create without --base (defaults to main) | ||
| // gh pr create without --base (defaults to main — require explicit intent) | ||
| test: (cmd) => /gh\s+pr\s+create\b/.test(cmd) && !/(--base|-B)\s+\S/.test(cmd), | ||
| reason: | ||
| 'BLOCKED: gh pr create requires explicit --base flag. Without it, GitHub defaults to main. Use: gh pr create --base dev', | ||
| 'BLOCKED: gh pr create requires explicit --base flag. Use: gh pr create --base dev (or --base main for releases)', | ||
| }, |
There was a problem hiding this comment.
Regex misses --base=value syntax.
The pattern /(--base|-B)\s+\S/ requires whitespace after --base, but gh CLI also accepts --base=main. A user running gh pr create --base=main --title "x" would be blocked incorrectly.
Proposed fix
{
- // gh pr create without --base (defaults to main — require explicit intent)
- test: (cmd) => /gh\s+pr\s+create\b/.test(cmd) && !/(--base|-B)\s+\S/.test(cmd),
+ // gh pr create without --base (defaults to main — require explicit intent)
+ test: (cmd) => /gh\s+pr\s+create\b/.test(cmd) && !/(--base[=\s]|-B\s)\S/.test(cmd),
reason:
'BLOCKED: gh pr create requires explicit --base flag. Use: gh pr create --base dev (or --base main for releases)',
},📝 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
| // Agents CAN create PRs targeting any branch (including main/master). | |
| // PRs are proposals for human review — the protection is on MERGING, not proposing. | |
| { | |
| // gh pr create --base main, gh pr create -B master | |
| test: (cmd) => /gh\s+pr\s+create\b/.test(cmd) && /(--base|-B)\s+(main|master)\b/.test(cmd), | |
| reason: 'BLOCKED: PRs must target dev, not main/master. Use: gh pr create --base dev', | |
| }, | |
| { | |
| // gh pr create without --base (defaults to main) | |
| // gh pr create without --base (defaults to main — require explicit intent) | |
| test: (cmd) => /gh\s+pr\s+create\b/.test(cmd) && !/(--base|-B)\s+\S/.test(cmd), | |
| reason: | |
| 'BLOCKED: gh pr create requires explicit --base flag. Without it, GitHub defaults to main. Use: gh pr create --base dev', | |
| 'BLOCKED: gh pr create requires explicit --base flag. Use: gh pr create --base dev (or --base main for releases)', | |
| }, | |
| // Agents CAN create PRs targeting any branch (including main/master). | |
| // PRs are proposals for human review — the protection is on MERGING, not proposing. | |
| { | |
| // gh pr create without --base (defaults to main — require explicit intent) | |
| test: (cmd) => /gh\s+pr\s+create\b/.test(cmd) && !/(--base[=\s]|-B\s)\S/.test(cmd), | |
| reason: | |
| 'BLOCKED: gh pr create requires explicit --base flag. Use: gh pr create --base dev (or --base main for releases)', | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/handlers/branch-guard.ts` around lines 29 - 36, The test predicate
in the branch guard (the object with test: (cmd) =>
/gh\s+pr\s+create\b/.test(cmd) && !/(--base|-B)\s+\S/.test(cmd)) fails to catch
--base=value syntax; update the regex used in that test so it accepts either
whitespace or an equals sign after --base (and similarly handle -B), e.g. change
the negative check to look for (--base|-B) followed by either = or whitespace
before the value (so --base=main and --base main both match) within the same
test function.
Four boolean fields in translateSdkConfig() used truthiness checks that silently dropped explicit `false` values. Changed to `!= null` guards matching the pattern already used by adjacent fields. - enableFileCheckpointing - includePartialMessages - promptSuggestions - agentProgressSummaries
- Match 'claude-sdk' in isClaude checks for session ID generation and native team registration in protocol-router-spawn - Route claude-sdk executor transport to 'process' instead of 'tmux' - Add 'app-pty' to spawnParamsSchema Zod enum to match ProviderName type
…cleanup, concurrency guard - Log warning + send buffer-full reply when MAX_BUFFER_PER_CHAT exceeded (was silent drop) - Shut down all active executor sessions in stop() before clearing (was resource leak) - Count spawning entries in concurrency limit to prevent oversubscription - Re-queue buffered messages on spawn failure instead of losing them Closes #1035
The function is only used within the module — no need to export it. Fixes dead-code check (knip) warning.
Reject non-numeric strings at parse time for --sdk-max-turns, --sdk-max-budget, and --sdk-thinking budgetTokens. Commander's bare Number parser silently passes NaN through; replaced with parseNumericFlag() in genie.ts and spawn.ts, and toSafeNumber() in dir.ts. NaN never reaches the database or SDK config.
After git clone --shared, worktrees now get: - A symlink to the parent repo's node_modules (so builds work immediately) - Execution of .genie/init.sh if present (post-clone hook convention) Both are best-effort — failures don't block team creation.
) Three new hook handlers expanding the handler library: - brain-inject: queries genie-brain on first tool call per session for relevant knowledge context (no-op when brain is not installed) - audit-context: injects recent git history for files being written/edited - freshness: warns when reading files recently modified by another agent All handlers are informational (permissionDecision: allow) and best-effort.
…k-executor-guide Replace 3 U+FFFD replacement characters with ▼ in the data flow diagram. Move inline comments out of shell line-continuations to make examples copy-pasteable. Closes #1040
…roup 1 dependencies - Re-export sanitizeWindowName (needed by test file) - Add isPaneProcessRunning to tmux.ts (used by claude-code executor liveness check) - Include claude-code.test.ts for sanitizeWindowName collision tests
Document the auto-resume system: spawn-time resume, daemon-level resume, session matching criteria, dead session detection, resume budget/cooldown, and the --no-auto-resume flag.
Document the AgentFrontmatterSchema from src/lib/frontmatter.ts: - All 9 fields with types, defaults, and descriptions - Warn-not-reject validation behavior - 5 complete examples (minimal, role, council, SDK, empty) - Field details for promptMode, provider, sdk, and tools - API usage for parseFrontmatter and schema access
`genie brain --help` now shows install, uninstall, update, and version subcommands with descriptions. Replaces manual args[0] checks with commander's native subcommand routing while preserving pass-through to the enterprise brain module for its own commands. Closes #1002
…ed serve model Create comprehensive architecture documentation covering: - Serve/daemon unification (daemon is now alias for serve --headless) - Service topology: pgserve, scheduler, event-router, inbox-watcher, session-capture, omni-bridge, service-registry - Agent lifecycle: spawn → register → work → report → shutdown - Data flow: CLI → PG → hooks → events → agents - Hook system as event bus - Directory structure and state file locations Update CO-ORCHESTRATION-GUIDE.md to reference genie serve instead of deprecated genie daemon commands, update architecture diagram, and link to ARCHITECTURE.md for internals. Closes #1003
Scan all registerXxxCommands() functions across src/term-commands/ and document every command with name, description, options (types and defaults), subcommands, and usage examples. Organized by category: core aliases, agent namespace, task management, team management, board/pipeline, project, events, sessions, database, scheduling, daemon, infrastructure, brain, omni, import/export, dispatch, QA, tags, types, releases, templates, metrics, notifications, hooks, directory management, utility commands, and executor debug.
docs: fix corrupted chars and shell examples in sdk-executor-guide
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/provider-adapters.ts`:
- Line 118: The zod enum for provider
(z.enum(['claude','codex','claude-sdk','app-pty'])) allows "app-pty" but
buildLaunchCommand's switch (function buildLaunchCommand) only handles 'claude',
'codex', and 'claude-sdk', causing a mismatch and an incomplete error message;
fix by adding an explicit 'app-pty' branch in buildLaunchCommand that either
implements its launch behavior or throws a clear, descriptive error (e.g.,
"provider 'app-pty' is not supported by buildLaunchCommand") and update the
existing default/error message to list all valid handled providers, or
alternatively remove 'app-pty' from the z.enum if that provider should not be
accepted by buildLaunchCommand.
🪄 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: f4dc6b1a-7166-4cb8-a55d-79f32a84fb44
📒 Files selected for processing (14)
docs/AGENT-FRONTMATTER.mddocs/ARCHITECTURE.mddocs/CLI-REFERENCE.mddocs/CO-ORCHESTRATION-GUIDE.mddocs/SPAWN-AUTO-RESUME.mddocs/sdk-executor-guide.mdsrc/genie.tssrc/lib/protocol-router-spawn.tssrc/lib/provider-adapters.tssrc/lib/providers/claude-sdk.tssrc/services/executors/claude-sdk.tssrc/term-commands/agent/spawn.tssrc/term-commands/brain.tssrc/term-commands/dir.ts
|
|
||
| const spawnParamsSchema = z.object({ | ||
| provider: z.enum(['claude', 'codex', 'claude-sdk']), | ||
| provider: z.enum(['claude', 'codex', 'claude-sdk', 'app-pty']), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if buildLaunchCommand is ever called with app-pty provider
rg -n "buildLaunchCommand" --type=ts -C 3 | head -80
echo "---"
# Check how app-pty spawns are routed
rg -n "app-pty" --type=ts -C 2Repository: automagik-dev/genie
Length of output: 7609
🏁 Script executed:
# Read the buildLaunchCommand implementation
sed -n '436,460p' src/lib/provider-adapters.ts
# Check protocol-router-spawn.ts usage of buildLaunchCommand
rg -n "buildLaunchCommand" src/lib/protocol-router-spawn.ts -A 10 -B 2
# Check how app-pty flows in agents.ts
rg -n "app-pty" src/term-commands/agents.ts -B 5 -A 5Repository: automagik-dev/genie
Length of output: 2099
🏁 Script executed:
# Check what provider values are passed to buildLaunchCommand in agents.ts
rg -n "buildSpawnParams|buildLaunchCommand" src/term-commands/agents.ts -B 5 -A 2 | head -100
# Also check if app-pty is passed as provider anywhere in agents.ts
rg -n "provider.*app-pty|app-pty.*provider" src/term-commands/agents.tsRepository: automagik-dev/genie
Length of output: 1565
🏁 Script executed:
# Find the buildSpawnParams function implementation in agents.ts
sed -n '1281,1350p' src/term-commands/agents.ts
# Also check buildFullResumeParams which is called at line 1807
rg -n "buildFullResumeParams" src/term-commands/agents.ts -A 20 | head -60Repository: automagik-dev/genie
Length of output: 4338
🏁 Script executed:
# Check buildResumeParams to see what provider it uses
rg -n "buildResumeParams" src/term-commands/agents.ts -A 15 | head -40
# Check if app-pty can reach buildLaunchCommand via genie-app path
grep -r "createExecutor" packages/genie-app/src-backend/pty.ts -B 5 -A 5
# Verify if app-pty flows to protocol-router-spawn
rg -n "spawnWorker|protocol-router" src/term-commands/agents.ts | head -20Repository: automagik-dev/genie
Length of output: 2519
Zod schema accepts app-pty but buildLaunchCommand doesn't handle it.
Line 118 allows app-pty through validation, but the switch statement (lines 439–454) only handles 'claude', 'codex', and 'claude-sdk'. The error message on line 452 also omits app-pty from the list of valid providers.
The genie-app backend bypasses buildLaunchCommand and calls the provider registry directly, so it never hits this case. However, this type mismatch is confusing—either add app-pty handling to the switch (even if just to throw a clear error) or remove it from the Zod enum if it's not meant for this function.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/provider-adapters.ts` at line 118, The zod enum for provider
(z.enum(['claude','codex','claude-sdk','app-pty'])) allows "app-pty" but
buildLaunchCommand's switch (function buildLaunchCommand) only handles 'claude',
'codex', and 'claude-sdk', causing a mismatch and an incomplete error message;
fix by adding an explicit 'app-pty' branch in buildLaunchCommand that either
implements its launch behavior or throws a clear, descriptive error (e.g.,
"provider 'app-pty' is not supported by buildLaunchCommand") and update the
existing default/error message to list all valid handled providers, or
alternatively remove 'app-pty' from the z.enum if that provider should not be
accepted by buildLaunchCommand.
added 3 commits
April 4, 2026 20:06
…ompt Closes #1015 When spawning a council/team leader via `genie team create --wish`, fetch any broadcast messages sent to the team conversation in the last 5 minutes and append them to the initial kickoff prompt. This lets the leader auto-discover the broadcast topic without a manual `genie send` after spawn. - Add `getRecentBroadcasts(teamName, minutesAgo=5)` — best-effort DB fetch with graceful fallback when PG is unavailable - Update `plugins/genie/agents/council/SOUL.md` to instruct the council moderator to check for "Recent team broadcasts for context" in its prompt - No change in behavior when no broadcasts exist — prompt unchanged
…xity Biome reported cognitive complexity 21 (max 15) on the freshness handler. Extract buildCommitWarning and checkUncommittedChanges helpers and flip the early-return guard so the main function stays under the limit.
feat: worktree DX — node_modules symlink, broadcast discovery, new hook handlers
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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/hooks/handlers/__tests__/freshness.test.ts`:
- Around line 100-108: The test for freshness is too permissive because it
allows result to be undefined, so regressions in the warning path can pass;
update the test to assert that result is defined for the "recently committed by
another agent" scenario (e.g., add expect(result).toBeDefined() or split into
two explicit cases), then continue to assert result.hookSpecificOutput,
hookSpecificOutput.additionalContext contains '[freshness]', and
hookSpecificOutput.permissionDecision === 'allow' so the warning/allow path is
enforced; reference the freshness function and the properties
hookSpecificOutput, additionalContext, and permissionDecision when updating the
assertions.
In `@src/hooks/handlers/audit-context.ts`:
- Around line 20-25: The git log invocation uses execSync with a template string
(execSync(`git log ... ${JSON.stringify(filePath)}`)) which allows shell
injection; replace it with execFileSync and pass arguments as an argv array
(e.g., execFileSync('git', ['log','--oneline','-n', String(MAX_COMMITS),'--',
filePath], {encoding:'utf-8', timeout:5000, cwd, stdio:['pipe','pipe','pipe']})
so the filePath is not shell-interpolated; update the call in audit-context (the
execSync that assigns to log and references MAX_COMMITS, filePath, cwd) and make
the identical change in freshness.ts for both occurrences (the execSync at the
top and the other call around line 71).
In `@src/hooks/handlers/brain-inject.ts`:
- Around line 31-33: The isBrainAvailable() filesystem probe (which checks
existsSync(join(BRAIN_DIR, 'package.json'))) can falsely report the brain
package missing in workspace/monorepo setups; remove the redundant availability
check and the early return that uses it so the code relies solely on the dynamic
import('@automagik/genie-brain') wrapped in its try/catch. Specifically, delete
the isBrainAvailable function and remove the conditional that returns early (the
check around isBrainAvailable() that prevents enrichment), allowing the existing
try/catch import logic to determine availability and handle errors.
- Around line 26-28: The sessionKey function currently uses process.pid as a
fallback which collapses distinct requests in the same process; update
sessionKey (the function taking HookPayload/session_id) to produce a
deterministic per-session key when session_id is absent by incorporating
additional unique context from the payload (for example payload.cwd or working
directory, payload.request_id if present) or by generating a stable unique
identifier (UUID/timestamp+random) per incoming payload so different sessions in
the same process don’t share the same key; change only sessionKey to combine
process.pid with payload-specific data or a generated UUID to ensure uniqueness.
In `@src/hooks/handlers/freshness.ts`:
- Line 111: The code currently uses substring matching
(commitInfo.author.includes(currentAgent)) which can false-match; change this to
an exact identity check by comparing normalized full author names (e.g.,
commitInfo.author.trim().toLowerCase() === currentAgent.trim().toLowerCase())
or, if commitInfo.author can contain multiple names/emails, split it into an
array and check exact membership (e.g.,
commitInfo.author.split(/[;,]/).map(s=>s.trim().toLowerCase()).includes(currentAgent.trim().toLowerCase()));
update the check in the freshness handler where currentAgent and
commitInfo.author are referenced and remove the use of includes() for substring
matching.
- Around line 23-28: Replace unsafe shell-interpolated execSync calls with
execFileSync using argv arrays: locate the uses of execSync in this file (the
call that builds the git command with `-- ${JSON.stringify(filePath)}`) and in
audit-context.ts, and change them to call child_process.execFileSync('git',
['log', '-1', '--format=%at|%an|%s', '--', filePath], { encoding: 'utf-8',
timeout: 5000, cwd, stdio: ['pipe','pipe','pipe'] }) (and similarly for the
other git invocations), ensuring you pass filePath as a separate argv element
and keep existing options (encoding, timeout, cwd, stdio) intact to avoid shell
interpolation vulnerabilities.
In `@src/lib/team-manager.test.ts`:
- Around line 154-193: The tests create temporary fixtures (node_modules,
.genie/init.sh, .init-marker) inside TEST_REPO but clean them up inline, which
can leak state on assertion failures; update the test file to create and
teardown fixtures using a safe pattern (use tmpdir or ensure cleanup in
afterEach / try/finally) so teardown always runs: for the tests referencing
createTeam('feat/symlink-nm'...), createTeam('feat/no-nm'...), and
createTeam('feat/init-sh'...), move creation of parentNodeModules, genieDir,
init.sh and marker into per-test setup and remove them in an afterEach that
force-deletes those paths (or wrap each test body in try/finally) to guarantee
cleanup even if assertions fail.
In `@src/lib/team-manager.ts`:
- Around line 230-237: The code currently executes the repo's .genie/init.sh
unconditionally; require an explicit opt-in (env var or team config flag) before
running it by checking a named flag (e.g., process.env.RUN_GENIE_INIT or a
boolean on the team creation config) immediately before the block that computes
initScript and runs $`bash ${initScript}`; if the flag is not set skip execution
and log/debug that the init script was not run, and if it is set run the same
try/catch best-effort logic; reference the initScript variable and the bash
execution call in team-manager.ts to locate where to add the gate.
In `@src/term-commands/team.ts`:
- Around line 372-376: The recent broadcasts appended to kickoffPrompt (from
getRecentBroadcasts) are unbounded and can bloat initialPrompt; limit the
context by capping the number of broadcasts (e.g., take last N items), truncate
each broadcast.text to a max length (e.g., 512 chars) with an ellipsis, and/or
enforce a total character budget (e.g., 2KB) before joining; apply the same
guardrail where broadcasts are appended around lines 432-436 as well, ensuring
you update the mapping that builds broadcastLines and then append the
shortened/filtered string to kickoffPrompt (refer to getRecentBroadcasts,
kickoffPrompt, and the broadcastLines construction).
🪄 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: 4ba210d1-0540-4600-aa46-84180f944f27
📒 Files selected for processing (11)
plugins/genie/agents/council/SOUL.mdsrc/hooks/handlers/__tests__/audit-context.test.tssrc/hooks/handlers/__tests__/brain-inject.test.tssrc/hooks/handlers/__tests__/freshness.test.tssrc/hooks/handlers/audit-context.tssrc/hooks/handlers/brain-inject.tssrc/hooks/handlers/freshness.tssrc/hooks/index.tssrc/lib/team-manager.test.tssrc/lib/team-manager.tssrc/term-commands/team.ts
Comment on lines
+20
to
+25
| const log = execSync(`git log --oneline -n ${MAX_COMMITS} -- ${JSON.stringify(filePath)}`, { | ||
| encoding: 'utf-8', | ||
| timeout: 5000, | ||
| cwd, | ||
| stdio: ['pipe', 'pipe', 'pipe'], | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify all shell-interpolated git execSync usages with filePath in hook handlers
rg -n --type ts 'execSync\(`git .*filePath' src/hooks/handlersRepository: automagik-dev/genie
Length of output: 452
🏁 Script executed:
sed -n '15,30p' src/hooks/handlers/audit-context.tsRepository: automagik-dev/genie
Length of output: 572
🏁 Script executed:
cat -n src/hooks/handlers/freshness.ts | sed -n '20,75p'Repository: automagik-dev/genie
Length of output: 2443
🏁 Script executed:
head -20 src/hooks/handlers/audit-context.tsRepository: automagik-dev/genie
Length of output: 868
Fix shell injection vulnerability: use execFileSync with argv form instead of execSync with template strings.
The current code is vulnerable to shell injection via crafted file paths. JSON.stringify() does not prevent shell metacharacters (backticks, $()) from breaking out. Use execFileSync with the argv array form to avoid shell parsing entirely.
Safer invocation (no shell parsing)
-import { execSync } from 'node:child_process';
+import { execFileSync } from 'node:child_process';
@@
- const log = execSync(`git log --oneline -n ${MAX_COMMITS} -- ${JSON.stringify(filePath)}`, {
+ const log = execFileSync('git', ['log', '--oneline', '-n', String(MAX_COMMITS), '--', filePath], {
encoding: 'utf-8',
timeout: 5000,
cwd,
stdio: ['pipe', 'pipe', 'pipe'],
});Note: The same vulnerability pattern exists in src/hooks/handlers/freshness.ts (lines 23 and 71) and should be fixed as well.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/handlers/audit-context.ts` around lines 20 - 25, The git log
invocation uses execSync with a template string (execSync(`git log ...
${JSON.stringify(filePath)}`)) which allows shell injection; replace it with
execFileSync and pass arguments as an argv array (e.g., execFileSync('git',
['log','--oneline','-n', String(MAX_COMMITS),'--', filePath], {encoding:'utf-8',
timeout:5000, cwd, stdio:['pipe','pipe','pipe']}) so the filePath is not
shell-interpolated; update the call in audit-context (the execSync that assigns
to log and references MAX_COMMITS, filePath, cwd) and make the identical change
in freshness.ts for both occurrences (the execSync at the top and the other call
around line 71).
Comment on lines
+26
to
+28
| function sessionKey(payload: HookPayload): string { | ||
| return payload.session_id ?? `${process.pid}`; | ||
| } |
There was a problem hiding this comment.
process.pid fallback collapses distinct sessions without session_id.
When session_id is missing, all calls in the same process share one key, so only the first gets enrichment even across different working directories.
Suggested fix
function sessionKey(payload: HookPayload): string {
- return payload.session_id ?? `${process.pid}`;
+ const cwdPart = payload.cwd ?? process.cwd();
+ return payload.session_id ?? `${process.pid}:${cwdPart}`;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/handlers/brain-inject.ts` around lines 26 - 28, The sessionKey
function currently uses process.pid as a fallback which collapses distinct
requests in the same process; update sessionKey (the function taking
HookPayload/session_id) to produce a deterministic per-session key when
session_id is absent by incorporating additional unique context from the payload
(for example payload.cwd or working directory, payload.request_id if present) or
by generating a stable unique identifier (UUID/timestamp+random) per incoming
payload so different sessions in the same process don’t share the same key;
change only sessionKey to combine process.pid with payload-specific data or a
generated UUID to ensure uniqueness.
Comment on lines
+31
to
+33
| function isBrainAvailable(): boolean { | ||
| return existsSync(join(BRAIN_DIR, 'package.json')); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify current implementation uses cwd-relative filesystem probing
rg -n --type=ts -C2 "BRAIN_DIR|isBrainAvailable|existsSync\\(|import\\(BRAIN_PKG\\)" src/hooks/handlers/brain-inject.ts
# Verify payload provides cwd (potentially different from process cwd)
rg -n --type=ts -C3 "interface HookPayload|cwd\\??:|session_id\\??:" src/hooks/types.ts
rg -n --type=ts -C2 "payload\\.cwd|process\\.cwd\\(" src/hooks/handlers/brain-inject.tsRepository: automagik-dev/genie
Length of output: 1107
isBrainAvailable() can silently disable enrichment in workspace setups.
Using existsSync(join('node_modules/@automagik/genie-brain', 'package.json')) checks a cwd-relative path that fails when node_modules lives in a parent directory (monorepos, workspaces). The early return at line 69 prevents enrichment even though import('@automagik/genie-brain') would succeed, since Node's module resolution is more sophisticated than filesystem probing. The actual import is already wrapped in try-catch (lines 37–40), so the redundant availability check should be removed to let the import fail gracefully only when needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/handlers/brain-inject.ts` around lines 31 - 33, The
isBrainAvailable() filesystem probe (which checks existsSync(join(BRAIN_DIR,
'package.json'))) can falsely report the brain package missing in
workspace/monorepo setups; remove the redundant availability check and the early
return that uses it so the code relies solely on the dynamic
import('@automagik/genie-brain') wrapped in its try/catch. Specifically, delete
the isBrainAvailable function and remove the conditional that returns early (the
check around isBrainAvailable() that prevents enrichment), allowing the existing
try/catch import logic to determine availability and handle errors.
Comment on lines
+23
to
+28
| const output = execSync(`git log -1 --format="%at|%an|%s" -- ${JSON.stringify(filePath)}`, { | ||
| encoding: 'utf-8', | ||
| timeout: 5000, | ||
| cwd, | ||
| stdio: ['pipe', 'pipe', 'pipe'], | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find shell-interpolated git execSync usage in handlers
rg -n --type ts 'execSync\(`git .*filePath' src/hooks/handlersRepository: automagik-dev/genie
Length of output: 452
🏁 Script executed:
cat -n src/hooks/handlers/freshness.ts | head -80Repository: automagik-dev/genie
Length of output: 3439
🏁 Script executed:
cat -n src/hooks/handlers/audit-context.ts | head -30Repository: automagik-dev/genie
Length of output: 1278
🏁 Script executed:
rg -n 'execSync\(`' src/hooks/handlers --type ts -A 4Repository: automagik-dev/genie
Length of output: 2628
Replace shell-interpolated execSync calls with argv-based child-process APIs.
Three git calls across two handlers use unsafe shell-interpolated execSync: freshness.ts (lines 23 and 71) and audit-context.ts (line 20). Even with JSON.stringify, shell metacharacter injection is possible.
Replace all three with execFileSync and argv arrays:
Safer command execution pattern
-import { execSync } from 'node:child_process';
+import { execFileSync } from 'node:child_process';
@@
-const output = execSync(`git log -1 --format="%at|%an|%s" -- ${JSON.stringify(filePath)}`, {
+const output = execFileSync('git', ['log', '-1', '--format=%at|%an|%s', '--', filePath], {
@@
-const status = execSync(`git status --porcelain -- ${JSON.stringify(filePath)}`, {
+const status = execFileSync('git', ['status', '--porcelain', '--', filePath], {
@@
-const log = execSync(`git log --oneline -n ${MAX_COMMITS} -- ${JSON.stringify(filePath)}`, {
+const log = execFileSync('git', ['log', '--oneline', '-n', String(MAX_COMMITS), '--', filePath], {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/handlers/freshness.ts` around lines 23 - 28, Replace unsafe
shell-interpolated execSync calls with execFileSync using argv arrays: locate
the uses of execSync in this file (the call that builds the git command with `--
${JSON.stringify(filePath)}`) and in audit-context.ts, and change them to call
child_process.execFileSync('git', ['log', '-1', '--format=%at|%an|%s', '--',
filePath], { encoding: 'utf-8', timeout: 5000, cwd, stdio:
['pipe','pipe','pipe'] }) (and similarly for the other git invocations),
ensuring you pass filePath as a separate argv element and keep existing options
(encoding, timeout, cwd, stdio) intact to avoid shell interpolation
vulnerabilities.
|
|
||
| if (commitInfo && commitInfo.age < STALENESS_THRESHOLD_SECS) { | ||
| // Skip warning if the current agent made the change | ||
| if (currentAgent && commitInfo.author.includes(currentAgent)) return; |
There was a problem hiding this comment.
Use exact author matching, not substring matching.
includes() can treat different authors as the same agent (e.g., qa vs aqua) and incorrectly suppress warnings.
Safer identity check
- if (currentAgent && commitInfo.author.includes(currentAgent)) return;
+ if (currentAgent && commitInfo.author.trim() === currentAgent.trim()) return;📝 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
| if (currentAgent && commitInfo.author.includes(currentAgent)) return; | |
| if (currentAgent && commitInfo.author.trim() === currentAgent.trim()) return; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/handlers/freshness.ts` at line 111, The code currently uses
substring matching (commitInfo.author.includes(currentAgent)) which can
false-match; change this to an exact identity check by comparing normalized full
author names (e.g., commitInfo.author.trim().toLowerCase() ===
currentAgent.trim().toLowerCase()) or, if commitInfo.author can contain multiple
names/emails, split it into an array and check exact membership (e.g.,
commitInfo.author.split(/[;,]/).map(s=>s.trim().toLowerCase()).includes(currentAgent.trim().toLowerCase()));
update the check in the freshness handler where currentAgent and
commitInfo.author are referenced and remove the use of includes() for substring
matching.
Comment on lines
+154
to
+193
| test('symlinks node_modules from parent repo into worktree', async () => { | ||
| // Create a fake node_modules in the test repo | ||
| const parentNodeModules = join(TEST_REPO, 'node_modules'); | ||
| await mkdir(parentNodeModules, { recursive: true }); | ||
| await writeFile(join(parentNodeModules, '.package-lock.json'), '{}'); | ||
|
|
||
| const config = await createTeam('feat/symlink-nm', TEST_REPO, 'dev'); | ||
| const worktreeNodeModules = join(config.worktreePath, 'node_modules'); | ||
|
|
||
| expect(existsSync(worktreeNodeModules)).toBe(true); | ||
| expect(lstatSync(worktreeNodeModules).isSymbolicLink()).toBe(true); | ||
| expect(readlinkSync(worktreeNodeModules)).toBe(parentNodeModules); | ||
|
|
||
| // Clean up | ||
| await rm(parentNodeModules, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| test('skips node_modules symlink when parent has no node_modules', async () => { | ||
| const config = await createTeam('feat/no-nm', TEST_REPO, 'dev'); | ||
| const worktreeNodeModules = join(config.worktreePath, 'node_modules'); | ||
|
|
||
| expect(existsSync(worktreeNodeModules)).toBe(false); | ||
| }); | ||
|
|
||
| test('runs .genie/init.sh in worktree after clone', async () => { | ||
| // Create a .genie/init.sh that writes a marker file | ||
| const genieDir = join(TEST_REPO, '.genie'); | ||
| await mkdir(genieDir, { recursive: true }); | ||
| await writeFile(join(genieDir, 'init.sh'), '#!/bin/bash\necho "init-ran" > .init-marker'); | ||
| await chmod(join(genieDir, 'init.sh'), 0o755); | ||
|
|
||
| const config = await createTeam('feat/init-sh', TEST_REPO, 'dev'); | ||
| const marker = join(config.worktreePath, '.init-marker'); | ||
|
|
||
| expect(existsSync(marker)).toBe(true); | ||
|
|
||
| // Clean up | ||
| await rm(join(genieDir, 'init.sh')); | ||
| await rm(marker, { force: true }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make fixture cleanup failure-safe (afterEach or try/finally).
These tests clean up at the end of each test body. If an assertion fails early, state can leak (node_modules, .genie/init.sh, marker files) and cascade into later tests.
As per coding guidelines, "Use tmpdir with cleanup in afterEach for test fixtures".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/team-manager.test.ts` around lines 154 - 193, The tests create
temporary fixtures (node_modules, .genie/init.sh, .init-marker) inside TEST_REPO
but clean them up inline, which can leak state on assertion failures; update the
test file to create and teardown fixtures using a safe pattern (use tmpdir or
ensure cleanup in afterEach / try/finally) so teardown always runs: for the
tests referencing createTeam('feat/symlink-nm'...), createTeam('feat/no-nm'...),
and createTeam('feat/init-sh'...), move creation of parentNodeModules, genieDir,
init.sh and marker into per-test setup and remove them in an afterEach that
force-deletes those paths (or wrap each test body in try/finally) to guarantee
cleanup even if assertions fail.
Comment on lines
+230
to
+237
| // Run .genie/init.sh if it exists in the repo (post-clone hook convention) | ||
| const initScript = join(repoPath, '.genie', 'init.sh'); | ||
| if (existsSync(initScript)) { | ||
| try { | ||
| await $`bash ${initScript}`.cwd(worktreePath).quiet(); | ||
| } catch { | ||
| // Best-effort — init script failure shouldn't block team creation | ||
| } |
There was a problem hiding this comment.
Require explicit trust opt-in before running .genie/init.sh.
This runs repository script code automatically during team creation. That is implicit code execution and should be gated behind an explicit opt-in (env/config/flag) before execution.
Suggested hardening
- if (existsSync(initScript)) {
+ const allowRepoInit = process.env.GENIE_ALLOW_REPO_INIT === '1';
+ if (allowRepoInit && existsSync(initScript)) {
try {
await $`bash ${initScript}`.cwd(worktreePath).quiet();
} catch {
// Best-effort — init script failure shouldn't block team creation
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/team-manager.ts` around lines 230 - 237, The code currently executes
the repo's .genie/init.sh unconditionally; require an explicit opt-in (env var
or team config flag) before running it by checking a named flag (e.g.,
process.env.RUN_GENIE_INIT or a boolean on the team creation config) immediately
before the block that computes initScript and runs $`bash ${initScript}`; if the
flag is not set skip execution and log/debug that the init script was not run,
and if it is set run the same try/catch best-effort logic; reference the
initScript variable and the bash execution call in team-manager.ts to locate
where to add the gate.
Comment on lines
+372
to
+376
| const broadcasts = await getRecentBroadcasts(config.name); | ||
| if (broadcasts.length > 0) { | ||
| const broadcastLines = broadcasts.map((b) => `[${b.sender}] ${b.text}`).join('\n'); | ||
| kickoffPrompt += `\n\nRecent team broadcasts for context:\n${broadcastLines}`; | ||
| } |
There was a problem hiding this comment.
Cap broadcast context size before appending to initialPrompt.
Injected message bodies are unbounded here; large broadcasts can bloat the kickoff prompt and cause downstream spawn/arg-size failures.
Proposed guardrail
+ const MAX_BROADCAST_CHARS = 4000;
+ const MAX_LINE_CHARS = 300;
const broadcasts = await getRecentBroadcasts(config.name);
if (broadcasts.length > 0) {
- const broadcastLines = broadcasts.map((b) => `[${b.sender}] ${b.text}`).join('\n');
+ const broadcastLines = broadcasts
+ .map((b) => `[${b.sender}] ${b.text.slice(0, MAX_LINE_CHARS)}`)
+ .join('\n')
+ .slice(0, MAX_BROADCAST_CHARS);
kickoffPrompt += `\n\nRecent team broadcasts for context:\n${broadcastLines}`;
}Also applies to: 432-436
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/term-commands/team.ts` around lines 372 - 376, The recent broadcasts
appended to kickoffPrompt (from getRecentBroadcasts) are unbounded and can bloat
initialPrompt; limit the context by capping the number of broadcasts (e.g., take
last N items), truncate each broadcast.text to a max length (e.g., 512 chars)
with an ellipsis, and/or enforce a total character budget (e.g., 2KB) before
joining; apply the same guardrail where broadcasts are appended around lines
432-436 as well, ensuring you update the mapping that builds broadcastLines and
then append the shortened/filtered string to kickoffPrompt (refer to
getRecentBroadcasts, kickoffPrompt, and the broadcastLines construction).
Tests now run against a dedicated pgserve spawned in --ram mode (native postgres with data in /dev/shm) on port 20900+, isolated from the production daemon at port 19642 and from ~/.genie/data/pgserve. Eliminates PG deadlock flakiness in team.test.ts that was blocking the pre-push hook. Root cause: tests and the live genie serve daemon shared one PostgreSQL database, and their asymmetric catalog-lock orderings tripped PG's deadlock detector under concurrent load. Full analysis in .genie/wishes/test-pg-ram-isolation/TRACE.md. Changes: - bunfig.toml: [test] preload = ["./src/lib/test-setup.ts"] - src/lib/test-setup.ts: spawns pgserve --ram on 20900-20999, reaps orphaned wrappers from previous runs at startup (bun test hard-exits without firing exit hooks). - src/lib/db.ts: ensurePgserve() honors GENIE_TEST_PG_PORT, production paths unchanged when unset. - src/lib/test-setup.test.ts: proves preload wiring and port binding. Validation: - 5 consecutive full-suite runs: 2052 pass, 0 fail, ~51s each. - bun run check passes on first attempt in 61s. - Orphan reaper keeps steady state of 1 pgserve between runs.
fix(omni-bridge): harden session lifecycle + liveness check + collision-resistant window names
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 18760271 | Triggered | Generic Password | a506587 | src/lib/test-setup.ts | View secret |
| 18760271 | Triggered | Generic Password | a506587 | src/lib/test-setup.ts | View secret |
| 18760271 | Triggered | Generic Password | c866c9a | src/lib/test-setup.ts | View secret |
| 18760271 | Triggered | Generic Password | c866c9a | src/lib/test-setup.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rolling Promotion PR
Auto-maintained rolling promotion PR from
devtomain.Process:
ready-to-mergeadded when all checks passSummary by CodeRabbit
Bug Fixes
New Features
app-pty) and Claude-equivalent provider handling.Documentation
Tests
Chores