feat: v0.9 release polish — CLI UX, plugin marketplace, CI build, docs#22
Open
cameronsjo wants to merge 6 commits into
Open
feat: v0.9 release polish — CLI UX, plugin marketplace, CI build, docs#22cameronsjo wants to merge 6 commits into
cameronsjo wants to merge 6 commits into
Conversation
- Move plugin content from plugin/ to repo root (standard layout) - Add .claude-plugin/plugin.json manifest for marketplace discovery - Add agent-pool to cameronsjo/workbench marketplace registry - Add pool-research skill for knowledge lifecycle queries - Update concierge-identity.md with researcher role context - Add cross-compile build matrix to CI (linux/darwin, amd64/arm64) - Add release workflow for tagged builds with GitHub Releases - Update README: v0.6–v0.8 marked complete, plugin install instructions - Create CHANGELOG.md with full version history (v0.1–v0.9) - Update CLAUDE.md: v0.8 status, new project structure Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a marketplace plugin manifest and removes the legacy manifest; introduces CI and release workflows that cross-build and publish binaries; implements CLI subcommands ( Changes
Sequence Diagram(s)(Skipped.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
![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 @.github/workflows/ci.yml:
- Around line 59-63: The Build job's run step uses go build -o
bin/agent-pool-... but fails on clean runners because the bin/ directory may not
exist; update the Build job (the run step that contains `go build -ldflags="-s
-w" -o bin/agent-pool-${{ matrix.goos }}-${{ matrix.goarch }} ./cmd/agent-pool`)
to ensure the bin directory exists first—either prepend a command to create the
directory (e.g., mkdir -p bin) before running go build or add a separate step
that creates bin/—so the output path is writable.
In `@skills/pool-research.md`:
- Around line 44-45: The step that says "Check the expert's recent task logs to
see if outputs match expectations" is unexecutable because the concierge cannot
read expert logs; update the workflow in skills/pool-research.md to replace that
line with an actionable alternative such as instructing the concierge to request
a recent task summary or specific excerpts from the expert (e.g., "Ask the
expert to provide recent task summaries, example outputs, or log excerpts
relevant to quality concerns") and add a fallback to escalate to a human
reviewer if the expert cannot provide those artifacts; target the quoted string
to locate and replace.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e2d92806-c37b-4858-9954-cd56ce6de242
📒 Files selected for processing (13)
.claude-plugin/plugin.json.github/workflows/ci.yml.github/workflows/release.yml.mcp.jsonCHANGELOG.mdCLAUDE.mdREADME.mdconcierge-identity.mdplugin/plugin.jsonskills/pool-ask.mdskills/pool-build.mdskills/pool-research.mdskills/pool-status.md
💤 Files with no reviewable changes (1)
- plugin/plugin.json
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 `@CHANGELOG.md`:
- Around line 7-15: The CHANGELOG entry header "## [0.9.0] — 2026-04-07" is
inconsistent with the repository (no v0.9.0 tag); either move that section to an
"[Unreleased]" heading (rename the header to "## [Unreleased]" and keep the
0.9.0 content) or create and push the v0.9.0 git tag to match the release
(create annotated tag "v0.9.0", push it to origin). Also ensure the PR
title/description references the correct target (v0.9.0 vs v0.8) so the change
matches the chosen action.
In `@CLAUDE.md`:
- Line 43: Add a "skills" array to the plugin manifest (plugin.json) registering
the four skill IDs so Claude Code can surface them; specifically update the
manifest used for the plugin to include a top-level "skills" field containing
"pool-ask", "pool-build", "pool-status", and "pool-research" (and ensure any
concierge .mcp.json reference remains unchanged), validate the manifest JSON
after adding the "skills" entry so the file stays syntactically correct and
tooling recognizes the registered skills.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 49162d26-11ff-4ab0-adbf-2525ccf4c677
📒 Files selected for processing (3)
CHANGELOG.mdCLAUDE.mdREADME.md
Comment on lines
+7
to
+15
| ## [0.9.0] — 2026-04-07 | ||
|
|
||
| Formulas and operational hardening. | ||
|
|
||
| - Formula parsing (`internal/formula/`) with TOML templates, DAG validation | ||
| - `instantiate_formula` architect MCP tool for bulk task creation | ||
| - Config hot-reload via fsnotify on `pool.toml` | ||
| - `EventConfigReloaded` event type | ||
| - `formulas/` directory in pool structure |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if v0.9.0 is tagged in the repository.
# Expected: Either a v0.9.0 tag exists (confirming release) or no tag exists (suggesting premature release marking).
git tag -l 'v0.9*'Repository: cameronsjo/agent-pool
Length of output: 47
Move v0.9.0 to [Unreleased] or create the release tag.
The v0.9.0 entry is marked as released on 2026-04-07, but no v0.9.0 git tag exists in the repository. This inconsistency must be resolved: either move the v0.9.0 section to [Unreleased] (if this is still in-progress work), or create and push the v0.9.0 release tag if the release is finalized. Also update the PR title/description if v0.9.0 is the actual target instead of v0.8.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` around lines 7 - 15, The CHANGELOG entry header "## [0.9.0] —
2026-04-07" is inconsistent with the repository (no v0.9.0 tag); either move
that section to an "[Unreleased]" heading (rename the header to "##
[Unreleased]" and keep the 0.9.0 content) or create and push the v0.9.0 git tag
to match the release (create annotated tag "v0.9.0", push it to origin). Also
ensure the PR title/description references the correct target (v0.9.0 vs v0.8)
so the change matches the chosen action.
| - **MCP server**: `agent-pool mcp --pool {name} --expert {name}` (experts) or `--role {architect|concierge}` (built-in roles) | ||
| - **Hooks**: Stop → flush, PreToolUse → code ownership guard | ||
| - **Plugin**: `plugin/` — skills (`pool-ask`, `pool-build`, `pool-status`) + `.mcp.json` for concierge | ||
| - **Plugin**: repo root — skills (`pool-ask`, `pool-build`, `pool-status`, `pool-research`) + `.mcp.json` for concierge |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the plugin.json structure and check for skill file existence.
echo "=== Current plugin.json content ==="
cat .claude-plugin/plugin.json
echo -e "\n=== Checking for skill files ==="
fd --type f --extension md . skills/Repository: cameronsjo/agent-pool
Length of output: 693
Critical: Plugin manifest missing skills registration.
The documentation claims four skills (pool-ask, pool-build, pool-status, pool-research) are available, but .claude-plugin/plugin.json is missing the "skills" field required to register them with Claude Code. Without explicit skill registration in the manifest, Claude Code won't surface these skills regardless of the existence of skill files in the skills/ directory.
🔧 Proposed fix to register skills in the manifest
Add the skills field to .claude-plugin/plugin.json:
{
"name": "agent-pool",
"description": "Expert pool — delegate questions and tasks to domain specialists via a mixture-of-experts model",
"author": {
"name": "Cameron Sjo",
"url": "https://github.com/cameronsjo"
},
"repository": "https://github.com/cameronsjo/agent-pool",
"license": "MIT",
+ "skills": [
+ "pool-ask",
+ "pool-build",
+ "pool-status",
+ "pool-research"
+ ],
"keywords": [
"agent-pool",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` at line 43, Add a "skills" array to the plugin manifest
(plugin.json) registering the four skill IDs so Claude Code can surface them;
specifically update the manifest used for the plugin to include a top-level
"skills" field containing "pool-ask", "pool-build", "pool-status", and
"pool-research" (and ensure any concierge .mcp.json reference remains
unchanged), validate the manifest JSON after adding the "skills" entry so the
file stays syntactically correct and tooling recognizes the registered skills.
Creates .agent-pool/ directory structure and a minimal pool.toml with pool name derived from the current directory. Simplifies the Quick Start from manual mkdir/cat to a single command. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 `@cmd/agent-pool/main.go`:
- Around line 95-97: The os.Stat(tomlPath) call currently only treats err==nil
(file exists) but ignores other errors; update the logic around the
os.Stat(tomlPath) call to explicitly fail on any error that is not
os.IsNotExist(err) by returning a wrapped error (include tomlPath and the
original err) so permission or IO errors surface immediately, while preserving
the existing exists branch when err==nil.
- Around line 63-66: The poolDir variable is set directly from os.Args[2] so a
value like "~/.agent-pool" won't be expanded; update the argument handling where
poolDir is assigned (the poolDir variable in main/agent-pool init path) to
detect a leading "~" (either exactly "~" or starting with "~/") and replace that
prefix with the user's home directory obtained via os.UserHomeDir(), then
resolve the result with filepath.Clean or filepath.Join as appropriate before
use. Ensure you still default to ".agent-pool" when no arg is provided and
handle errors from os.UserHomeDir().
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 41158c94-052f-48fb-b358-fa7fb66d62c7
📒 Files selected for processing (3)
README.mdcmd/agent-pool/main.gocmd/agent-pool/main_test.go
- `agent-pool add <name> [--model M]` appends expert to pool.toml and creates inbox/logs dirs. Validates against duplicates and builtin role names. - `agent-pool list` shows experts with model, scope, and state files. - Help text split into user-facing, getting-started, and internal sections for progressive disclosure. - Quick Start now uses init + add instead of manual TOML editing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cameronsjo
changed the title
- CI build: mkdir -p bin before cross-compile (clean runner fix) - pool-research skill: replace unexecutable "check logs" with check_status + dispatch (concierge can't read expert logs) - init: add tilde expansion for pool dir argument - initPool: fail fast on os.Stat permission errors instead of silently continuing Skipped: CHANGELOG tag (tag comes after merge), plugin.json skills field (not required — Claude Code auto-discovers skills/ directory). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/agent-pool/main.go`:
- Around line 95-103: The cmdAdd function currently accepts an empty or
flag-valued model when the user passes "--model" without an argument; update
cmdAdd to validate the parsed model: after calling parseFlagsFromArgs and
getting flags["model"], detect if "--model" was present but the value is empty
or starts with "-" (indicating the next token was another flag) and, in that
case, print a usage/error and exit (same behavior as other bad-usage branches).
Reference cmdAdd, the flags variable, and parseFlagsFromArgs when adding this
check so malformed invocations like "agent-pool add X --model" or "agent-pool
add X --model --shared" are rejected unless a real non-flag model value is
provided.
- Around line 66-84: cmdInit currently allows initializing a pool in an
arbitrary directory (via initPool) but cmdAdd and cmdList still discover pools
only via config.DiscoverPoolDir("") (CWD or .agent-pool), so pools created with
custom paths (e.g., cmdInit with "./custom-pool") aren't found. Fix by recording
the custom pool path in a location DiscoverPoolDir will find (or update
DiscoverPoolDir to consult a known config file); specifically, when cmdInit
calls initPool(poolDir, poolName, cwd) ensure you persist the absolute poolDir
(not just relative) into the same discovery mechanism used by
config.DiscoverPoolDir (or extend config.DiscoverPoolDir to read a project-level
pointer file), and update initPool/cmdInit to write that pointer so cmdAdd and
cmdList (which call config.DiscoverPoolDir) will rediscover custom pool
locations.
- Around line 55-56: Replace the hard-coded literal in the "version" case with a
package-level variable (e.g., Version string = "dev") and print that variable
(e.g., fmt.Println("agent-pool " + Version)) instead of the fixed "agent-pool
v0.9.0"; build/release automation should inject the tag via linker flags (e.g.,
-ldflags "-X main.Version=<tag>") so the binary reports the correct tag at
runtime. Ensure the variable name (Version) and the switch case that prints it
are updated accordingly.
- Around line 125-153: The code currently allows unsafe expert names when
building the TOML section string (see section := fmt.Sprintf("\n[experts.%s]\n",
name)), so add a strict validation step for the variable name before using it:
ensure name matches a safe regex (e.g. ^[A-Za-z0-9_-]+$) and return an error if
it does not (similar to the existing checks around name and
config.BuiltinRoleNames). Update the validation logic that runs before loading
the config (the block that checks name == "" and filepath.Base(name)) to include
this regex check and a clear error message rejecting names with dots, spaces,
quotes or other special chars; keep existing checks for built-in roles and
duplicate cfg.Experts[name].
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a39d2b91-c838-46e6-b335-be1a693751db
📒 Files selected for processing (3)
README.mdcmd/agent-pool/main.gocmd/agent-pool/main_test.go
Comment on lines
55
to
+56
| case "version": | ||
| fmt.Println("agent-pool v0.6.0-dev") | ||
| fmt.Println("agent-pool v0.9.0") |
There was a problem hiding this comment.
Don't hard-code the CLI version in a tag-driven release flow.
With the new v* release automation, every future binary will still report agent-pool v0.9.0 unless this file is edited before each tag. Make the version a variable and inject the tag at build time.
Suggested change
+var version = "dev"
+
func main() {
@@
case "version":
- fmt.Println("agent-pool v0.9.0")
+ fmt.Printf("agent-pool %s\n", version)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/agent-pool/main.go` around lines 55 - 56, Replace the hard-coded literal
in the "version" case with a package-level variable (e.g., Version string =
"dev") and print that variable (e.g., fmt.Println("agent-pool " + Version))
instead of the fixed "agent-pool v0.9.0"; build/release automation should inject
the tag via linker flags (e.g., -ldflags "-X main.Version=<tag>") so the binary
reports the correct tag at runtime. Ensure the variable name (Version) and the
switch case that prints it are updated accordingly.
Comment on lines
+66
to
+84
| func cmdInit() { | ||
| poolDir := ".agent-pool" | ||
| if len(os.Args) > 2 { | ||
| poolDir = os.Args[2] | ||
| } | ||
|
|
||
| cwd, err := os.Getwd() | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "error: %v\n", err) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| poolName := filepath.Base(cwd) | ||
| if err := initPool(poolDir, poolName, cwd); err != nil { | ||
| fmt.Fprintf(os.Stderr, "error: %v\n", err) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| fmt.Printf("Initialized pool %q in %s\n", poolName, poolDir) |
There was a problem hiding this comment.
init [pool-dir] creates pools that add and list can't rediscover.
cmdInit accepts any destination, but cmdAdd and cmdList always call config.DiscoverPoolDir(""), which only finds pool.toml in CWD or .agent-pool/pool.toml while walking upward. agent-pool init ./custom-pool followed by agent-pool add backend from the project root fails immediately, so the advertised custom pool location contract is broken.
Also applies to: 105-109, 169-174, 890-892
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/agent-pool/main.go` around lines 66 - 84, cmdInit currently allows
initializing a pool in an arbitrary directory (via initPool) but cmdAdd and
cmdList still discover pools only via config.DiscoverPoolDir("") (CWD or
.agent-pool), so pools created with custom paths (e.g., cmdInit with
"./custom-pool") aren't found. Fix by recording the custom pool path in a
location DiscoverPoolDir will find (or update DiscoverPoolDir to consult a known
config file); specifically, when cmdInit calls initPool(poolDir, poolName, cwd)
ensure you persist the absolute poolDir (not just relative) into the same
discovery mechanism used by config.DiscoverPoolDir (or extend
config.DiscoverPoolDir to read a project-level pointer file), and update
initPool/cmdInit to write that pointer so cmdAdd and cmdList (which call
config.DiscoverPoolDir) will rediscover custom pool locations.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
cmd/agent-pool/main.go (1)
777-790:⚠️ Potential issue | 🟡 MinorSilent fallback on
UserHomeDirerror may create literal~directory.When
os.UserHomeDir()fails (rare but possible in containers or unusual setups),expandTildereturns the unexpanded path. If the caller (e.g.,cmdInit) proceeds, it will create a directory literally named~or~/.agent-poolin CWD. Consider returning an error or having callers detect this edge case.Alternative: return error from expandTilde
-func expandTilde(path string) string { +func expandTilde(path string) (string, error) { if path == "~" || strings.HasPrefix(path, "~/") { home, err := os.UserHomeDir() if err != nil { - return path + return "", fmt.Errorf("resolving home directory: %w", err) } if path == "~" { - return home + return home, nil } - return filepath.Join(home, path[2:]) + return filepath.Join(home, path[2:]), nil } - return path + return path, nil }Then in
cmdInit:- poolDir = expandTilde(poolDir) + poolDir, err = expandTilde(poolDir) + if err != nil { + fmt.Fprintf(os.Stderr, "error: %v\n", err) + os.Exit(1) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/agent-pool/main.go` around lines 777 - 790, The expandTilde function silently returns the unexpanded path on os.UserHomeDir() failure which can cause creation of a literal "~" directory; change expandTilde to return (string, error) and return an explicit error when UserHomeDir fails, then update callers (notably cmdInit) to check the error and abort with a clear message instead of proceeding — locate expandTilde and its uses (e.g., in cmdInit) and replace direct calls with the new signature, handling the error by logging/returning it and preventing directory creation when tilde expansion fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 46-65: Add an explicit CGO_DISABLE setting to the Build step so
cross-compilation never accidentally enables cgo: in the Build step that sets
environment variables GOOS and GOARCH (the block that runs mkdir -p bin and go
build -ldflags="-s -w" -o bin/agent-pool-${{ matrix.goos }}-${{ matrix.goarch }}
./cmd/agent-pool), add CGO_ENABLED=0 to the env list so the build always
disables CGO during cross-compilation.
In `@cmd/agent-pool/main.go`:
- Around line 198-212: The expert list is built by iterating the map cfg.Experts
which yields nondeterministic order; fix by collecting the map keys into a
slice, sort that slice, then iterate the sorted names to construct expertInfo
(same logic that sets model, info.Has, expertDir, etc.) and append to the
experts slice so CLI output is stable across runs (refer to cfg.Experts,
expertInfo, experts, poolDir, and the identity.md/state.md/errors.md checks).
---
Duplicate comments:
In `@cmd/agent-pool/main.go`:
- Around line 777-790: The expandTilde function silently returns the unexpanded
path on os.UserHomeDir() failure which can cause creation of a literal "~"
directory; change expandTilde to return (string, error) and return an explicit
error when UserHomeDir fails, then update callers (notably cmdInit) to check the
error and abort with a clear message instead of proceeding — locate expandTilde
and its uses (e.g., in cmdInit) and replace direct calls with the new signature,
handling the error by logging/returning it and preventing directory creation
when tilde expansion fails.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c89acc94-48ad-48c6-bb6f-8bba7ec669a6
📒 Files selected for processing (3)
.github/workflows/ci.ymlcmd/agent-pool/main.goskills/pool-research.md
- Stable list output: sort expert names before printing - CGO_ENABLED=0 in CI cross-compile build matrix - Strict expert name validation: regex ^[A-Za-z0-9_-]+$ prevents TOML injection via special characters - Reject --model flag without a value (e.g. agent-pool add X --model) Skipped: custom pool path discovery — DiscoverPoolDir already walks parent dirs for .agent-pool/, custom paths are an edge case. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/agent-pool/main_test.go`:
- Around line 198-201: The test ignores initPool's returned error; update
TestAddExpert_CreatesExpert to capture the (dir, err) returned by initPool and
fail the test on error (e.g., t.Fatalf) before proceeding so failures are clear;
apply the same change to the other tests that call initPool (referenced by their
names in this file: TestInitPool_CreatesStructure, TestAddExpert_CreatesExpert,
and the tests around lines noted — the ones at the locations corresponding to
lines 228, 246, 261, and 274) to check the error and fail fast if initialization
fails.
- Around line 258-269: Update the TestAddExpert_InvalidName test to include more
edge cases: add an empty string ("") and one or more boundary-length names (e.g.
strings.Repeat("a", 256) for an overly-long name and strings.Repeat("a", 255) if
you want the exact boundary) to the invalid-names slice; keep using
addExpert(poolDir, name, "") and assert err != nil for these new cases. This
change targets the TestAddExpert_InvalidName test function and exercises the
addExpert validation (initPool can remain unchanged).
- Around line 243-256: The TestAddExpert_Duplicate test ignores the error from
the first addExpert call which can mask failures; update the test
(TestAddExpert_Duplicate) to capture and assert the first addExpert(poolDir,
"backend", "") return value and call t.Fatalf or t.Fatal if it returns an
unexpected error before proceeding to call addExpert a second time, so only the
second call's duplicate-error assertion is evaluated; use the existing initPool
and addExpert symbols to locate and modify the test.
In `@cmd/agent-pool/main.go`:
- Around line 7-11: Reorder the stdlib imports in the main.go import block so
they are alphabetized (errors, fmt, io, regexp, sort) — move "sort" to follow
"regexp" -> "sort" should come last in that block; you can also run goimports to
apply the canonical ordering automatically.
- Around line 790-803: The expandTilde function silently returns the literal
path when os.UserHomeDir() fails; change its signature to expandTilde(path
string) (string, error), call os.UserHomeDir() as before and if it errors return
the original path and the error (e.g., return path, err), and update all callers
of expandTilde to handle the error (log or propagate) instead of assuming
expansion succeeded; reference expandTilde and os.UserHomeDir when locating the
code to change.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a3f3ab90-29df-4613-b658-eb184459fc92
📒 Files selected for processing (4)
.github/workflows/ci.ymlcmd/agent-pool/main.gocmd/agent-pool/main_test.goskills/pool-research.md
Comment on lines
+198
to
+201
| func TestAddExpert_CreatesExpert(t *testing.T) { | ||
| tmp := t.TempDir() | ||
| poolDir := filepath.Join(tmp, ".agent-pool") | ||
| initPool(poolDir, "test", tmp) |
There was a problem hiding this comment.
Check initPool return value in test setup.
Line 201 ignores the error from initPool. If pool initialization fails, subsequent assertions will produce confusing errors. Apply the same pattern used in TestInitPool_CreatesStructure.
Suggested fix
func TestAddExpert_CreatesExpert(t *testing.T) {
tmp := t.TempDir()
poolDir := filepath.Join(tmp, ".agent-pool")
- initPool(poolDir, "test", tmp)
+ if err := initPool(poolDir, "test", tmp); err != nil {
+ t.Fatalf("initPool: %v", err)
+ }Same fix applies to lines 228, 246, 261, and 274.
📝 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
| func TestAddExpert_CreatesExpert(t *testing.T) { | |
| tmp := t.TempDir() | |
| poolDir := filepath.Join(tmp, ".agent-pool") | |
| initPool(poolDir, "test", tmp) | |
| func TestAddExpert_CreatesExpert(t *testing.T) { | |
| tmp := t.TempDir() | |
| poolDir := filepath.Join(tmp, ".agent-pool") | |
| if err := initPool(poolDir, "test", tmp); err != nil { | |
| t.Fatalf("initPool: %v", err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/agent-pool/main_test.go` around lines 198 - 201, The test ignores
initPool's returned error; update TestAddExpert_CreatesExpert to capture the
(dir, err) returned by initPool and fail the test on error (e.g., t.Fatalf)
before proceeding so failures are clear; apply the same change to the other
tests that call initPool (referenced by their names in this file:
TestInitPool_CreatesStructure, TestAddExpert_CreatesExpert, and the tests around
lines noted — the ones at the locations corresponding to lines 228, 246, 261,
and 274) to check the error and fail fast if initialization fails.
Comment on lines
+243
to
+256
| func TestAddExpert_Duplicate(t *testing.T) { | ||
| tmp := t.TempDir() | ||
| poolDir := filepath.Join(tmp, ".agent-pool") | ||
| initPool(poolDir, "test", tmp) | ||
|
|
||
| addExpert(poolDir, "backend", "") | ||
| err := addExpert(poolDir, "backend", "") | ||
| if err == nil { | ||
| t.Fatal("expected error for duplicate expert") | ||
| } | ||
| if !strings.Contains(err.Error(), "already exists") { | ||
| t.Errorf("error = %q, want 'already exists'", err.Error()) | ||
| } | ||
| } |
There was a problem hiding this comment.
Check first addExpert return value in duplicate test.
Line 248 ignores the error from the first addExpert call. If the first add fails for an unexpected reason, the test's assertion about the second call would be misleading.
Suggested fix
func TestAddExpert_Duplicate(t *testing.T) {
tmp := t.TempDir()
poolDir := filepath.Join(tmp, ".agent-pool")
- initPool(poolDir, "test", tmp)
+ if err := initPool(poolDir, "test", tmp); err != nil {
+ t.Fatalf("initPool: %v", err)
+ }
- addExpert(poolDir, "backend", "")
+ if err := addExpert(poolDir, "backend", ""); err != nil {
+ t.Fatalf("first addExpert: %v", err)
+ }
err := addExpert(poolDir, "backend", "")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/agent-pool/main_test.go` around lines 243 - 256, The
TestAddExpert_Duplicate test ignores the error from the first addExpert call
which can mask failures; update the test (TestAddExpert_Duplicate) to capture
and assert the first addExpert(poolDir, "backend", "") return value and call
t.Fatalf or t.Fatal if it returns an unexpected error before proceeding to call
addExpert a second time, so only the second call's duplicate-error assertion is
evaluated; use the existing initPool and addExpert symbols to locate and modify
the test.
Comment on lines
+258
to
+269
| func TestAddExpert_InvalidName(t *testing.T) { | ||
| tmp := t.TempDir() | ||
| poolDir := filepath.Join(tmp, ".agent-pool") | ||
| initPool(poolDir, "test", tmp) | ||
|
|
||
| for _, name := range []string{"has space", "has.dot", "has/slash", `has"quote`} { | ||
| err := addExpert(poolDir, name, "") | ||
| if err == nil { | ||
| t.Errorf("expected error for name %q, got nil", name) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider expanding invalid name test cases.
The test covers common invalid characters but could include edge cases like empty string and boundary-length names to ensure robust validation.
Additional test cases
- for _, name := range []string{"has space", "has.dot", "has/slash", `has"quote`} {
+ for _, name := range []string{
+ "", // empty
+ "has space", // space
+ "has.dot", // dot
+ "has/slash", // slash
+ `has"quote`, // quote
+ "-starts-with-dash", // valid per regex but potentially confusing
+ } {📝 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
| func TestAddExpert_InvalidName(t *testing.T) { | |
| tmp := t.TempDir() | |
| poolDir := filepath.Join(tmp, ".agent-pool") | |
| initPool(poolDir, "test", tmp) | |
| for _, name := range []string{"has space", "has.dot", "has/slash", `has"quote`} { | |
| err := addExpert(poolDir, name, "") | |
| if err == nil { | |
| t.Errorf("expected error for name %q, got nil", name) | |
| } | |
| } | |
| } | |
| func TestAddExpert_InvalidName(t *testing.T) { | |
| tmp := t.TempDir() | |
| poolDir := filepath.Join(tmp, ".agent-pool") | |
| initPool(poolDir, "test", tmp) | |
| for _, name := range []string{ | |
| "", // empty | |
| "has space", // space | |
| "has.dot", // dot | |
| "has/slash", // slash | |
| `has"quote`, // quote | |
| "-starts-with-dash", // valid per regex but potentially confusing | |
| } { | |
| err := addExpert(poolDir, name, "") | |
| if err == nil { | |
| t.Errorf("expected error for name %q, got nil", name) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/agent-pool/main_test.go` around lines 258 - 269, Update the
TestAddExpert_InvalidName test to include more edge cases: add an empty string
("") and one or more boundary-length names (e.g. strings.Repeat("a", 256) for an
overly-long name and strings.Repeat("a", 255) if you want the exact boundary) to
the invalid-names slice; keep using addExpert(poolDir, name, "") and assert err
!= nil for these new cases. This change targets the TestAddExpert_InvalidName
test function and exercises the addExpert validation (initPool can remain
unchanged).
Comment on lines
+7
to
11
| "errors" | ||
| "fmt" | ||
| "regexp" | ||
| "sort" | ||
| "io" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor import ordering issue.
The sort import appears between regexp and io, breaking the standard alphabetical grouping within the stdlib block. This isn't flagged by the guidelines, but goimports would typically reorder these.
Suggested reorder
import (
"bufio"
"context"
"encoding/json"
"errors"
"fmt"
+ "io"
+ "log/slog"
+ "net"
+ "os"
+ "os/signal"
+ "path/filepath"
"regexp"
"sort"
- "io"
- "log/slog"
- "net"
- "os"
- "os/signal"
- "path/filepath"
"strings"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/agent-pool/main.go` around lines 7 - 11, Reorder the stdlib imports in
the main.go import block so they are alphabetized (errors, fmt, io, regexp,
sort) — move "sort" to follow "regexp" -> "sort" should come last in that block;
you can also run goimports to apply the canonical ordering automatically.
Comment on lines
+790
to
+803
| // expandTilde replaces a leading ~ with the user's home directory. | ||
| func expandTilde(path string) string { | ||
| if path == "~" || strings.HasPrefix(path, "~/") { | ||
| home, err := os.UserHomeDir() | ||
| if err != nil { | ||
| return path | ||
| } | ||
| if path == "~" { | ||
| return home | ||
| } | ||
| return filepath.Join(home, path[2:]) | ||
| } | ||
| return path | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Silent fallback on home directory lookup failure.
If os.UserHomeDir() fails (line 794), the function silently returns the original path containing ~. This could create a literal ~ directory rather than expanding to the user's home. Consider logging a warning or returning an error.
Alternative: propagate error to caller
-func expandTilde(path string) string {
+func expandTilde(path string) (string, error) {
if path == "~" || strings.HasPrefix(path, "~/") {
home, err := os.UserHomeDir()
if err != nil {
- return path
+ return "", fmt.Errorf("expanding ~: %w", err)
}
if path == "~" {
- return home
+ return home, nil
}
- return filepath.Join(home, path[2:])
+ return filepath.Join(home, path[2:]), nil
}
- return path
+ return path, nil
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/agent-pool/main.go` around lines 790 - 803, The expandTilde function
silently returns the literal path when os.UserHomeDir() fails; change its
signature to expandTilde(path string) (string, error), call os.UserHomeDir() as
before and if it errors return the original path and the error (e.g., return
path, err), and update all callers of expandTilde to handle the error (log or
propagate) instead of assuming expansion succeeded; reference expandTilde and
os.UserHomeDir when locating the code to change.
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
1 participant
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.
Summary
agent-pool init— scaffolds pool directory + minimal pool.tomlagent-pool add <name> [--model M]— adds expert without manual TOML editingagent-pool list— shows experts with model, scope, and state filesplugin/to repo root for standard Claude Code plugin layout.claude-plugin/plugin.jsonmanifest + register incameronsjo/workbenchmarketplacepool-researchskill and update concierge identity with researcher contextv*→ 4 binaries → GitHub ReleaseTest plan
make buildsucceeds locallyagent-pool init && agent-pool add backend && agent-pool listworks end-to-end.claude-plugin/plugin.json,skills/,.mcp.jsonat root/plugin marketplace update workbench🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores
Tests