kaizen: fleet refresh CLI と dogfood script を追加し、登録済み workspace の sync/setup/verify readi (#78)#82
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: kaizen-agents-org/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds ChangesFleet refresh command
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 623bbcee4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Line 20: The npm script entry for dogfood:fleet-refresh currently invokes
scripts/dogfood-fleet-refresh.sh directly, which depends on the executable bit
being set. Update that package.json script to run the shell script through sh
instead, keeping the same script name but changing the command so it works
regardless of file mode.
In `@src/commands/fleet.ts`:
- Around line 80-83: Validate the registry workspace path before calling
workspace.ensure() in fleet refresh so a malformed registry entry cannot trigger
destructive repairs on an arbitrary local directory during --sync. Add a guard
around the workspace!.ensure() call in the sync path to confirm
project.workspacePath points to the expected dedicated Kaizen workspace before
any clone/repair logic runs, and skip or fail the step if it does not.
In `@test/report.test.ts`:
- Around line 278-288: The temporary directory setup in the test helper is
hardcoded to "/tmp", which makes the suite non-portable. Update the temp dir
creation in the helper that uses fs.mkdtemp and execFileAsync to build the
prefix from os.tmpdir() instead, and ensure node:os is imported in this test
file so the isolated temp directory works across platforms.
🪄 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: Repository: kaizen-agents-org/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a019da42-bdaa-4bca-a8e3-68493ab890ae
📒 Files selected for processing (8)
docs/02-cli-spec.mdpackage.jsonscripts/dogfood-fleet-refresh.shsrc/cli.tssrc/commands/fleet.tssrc/workspace/manager.tstest/fleet.test.tstest/report.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/fleet.ts`:
- Around line 89-92: The RunLock.isActiveError branch in fleet command handling
is missing the sync result when a locked run blocks a synced refresh. Update the
error path in the fleet command flow to also append a failed sync step alongside
workspace, setup, and verify, using the same “skipped because run is already
active” message so the JSON output stays complete for --sync runs.
- Around line 82-86: The `fleet` command currently makes
`GitClient.remoteUrl('origin')` a hard failure when `--sync` is used, so a
missing or invalid local checkout stops refresh before falling back to the
registered repo. Update the `remoteUrl` selection in `src/commands/fleet.ts` to
catch failures from the `origin` lookup and use `project.repo` as the fallback,
then pass that resolved value into `WorkspaceManager` so workspace repair can
continue even when the local checkout is broken.
🪄 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: Repository: kaizen-agents-org/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 975443ad-b75c-4810-b65a-cfcbdcbaaad9
📒 Files selected for processing (2)
src/commands/fleet.tstest/fleet.test.ts
|
PR Guardian final status for this pass: Mergeability: not mergeable yet. Watched CI runs:
Fixes pushed:
Feedback addressed:
Review feedback state: paginated GraphQL review-thread and PR-comment reads completed with Remaining blocker: external CodeRabbit status context remains pending, so GitHub still reports the PR as blocked. No merge was attempted. |
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/commands/fleet.ts (1)
121-135: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate the workspace path before no-sync setup/verify too.
The guard currently runs only in the
syncbranch. With--no-sync, a malformed registry entry pointingworkspacePathat any existing Git checkout passes the.gitcheck and then setup/verify commands execute there.🛡️ Proposed fix
} else { workspaceReady = await runStep(steps, 'workspace', async () => { + assertSafeWorkspacePath(slug, project.workspacePath); await fs.access(path.join(project.workspacePath, '.git')); }); }As per coding guidelines, “Do not modify secrets, credentials, billing, destructive data changes, or production infrastructure without explicit human approval.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/fleet.ts` around lines 121 - 135, The workspace path validation is only applied in the sync branch, so no-sync flows can still operate on an unsafe workspacePath. Update the fleet command’s workspaceReady logic in the runStep workflow to call assertSafeWorkspacePath(slug, project.workspacePath) before both the sync and no-sync checks, keeping the guard in the workspace verification path so setup/verify cannot run against a malformed registry entry.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/fleet.ts`:
- Around line 208-212: The workspace safety check in assertSafeWorkspacePath is
relying on workspaceDir(slug) after the slug has already been accepted, so
unsafe registry keys can still escape the intended workspaces root. Add slug
validation at the registry boundary in resolveProject and upsertProject, or
harden assertSafeWorkspacePath to reject slugs containing path separators, dot
segments, or other unsafe characters before calling workspaceDir(slug). Keep the
fix centered around slugFromRepo, resolveProject, upsertProject, and
assertSafeWorkspacePath so the workspace path can never be derived from an
unsafe slug.
---
Outside diff comments:
In `@src/commands/fleet.ts`:
- Around line 121-135: The workspace path validation is only applied in the sync
branch, so no-sync flows can still operate on an unsafe workspacePath. Update
the fleet command’s workspaceReady logic in the runStep workflow to call
assertSafeWorkspacePath(slug, project.workspacePath) before both the sync and
no-sync checks, keeping the guard in the workspace verification path so
setup/verify cannot run against a malformed registry entry.
🪄 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: Repository: kaizen-agents-org/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 990454a6-30f1-4d92-8ecf-cdc0ddf5a299
📒 Files selected for processing (4)
package.jsonsrc/commands/fleet.tstest/fleet.test.tstest/report.test.ts
| function assertSafeWorkspacePath(slug: string, projectWorkspacePath: string): void { | ||
| if (path.resolve(projectWorkspacePath) !== path.resolve(workspaceDir(slug))) { | ||
| throw new Error(`Refusing to refresh unsafe workspace path for ${slug}: ${projectWorkspacePath}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect to find validation that rejects '/', '\', '.', and '..' in registry slugs.
rg -n -C4 'loadRegistry|resolveProject|saveRegistry|RegistryProject|slug|workspaceDir' src/config src/utils src/commands/fleet.tsRepository: kaizen-agents-org/kaizen-loop
Length of output: 11269
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the registry schema and any slug validation used before registry entries are created.
sed -n '1,220p' src/config/schema.ts
printf '\n---\n'
sed -n '1,220p' src/utils/slug.ts
printf '\n---\n'
rg -n -C3 'upsertProject\(|registry\.projects|projectSlug|slugify\(|slugFromRepo\(|registrySchema|registryProjectSchema' srcRepository: kaizen-agents-org/kaizen-loop
Length of output: 23341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the registry schema and any slug validation used before registry entries are created.
sed -n '1,220p' src/config/schema.ts
printf '\n---\n'
sed -n '1,220p' src/utils/slug.ts
printf '\n---\n'
rg -n -C3 'upsertProject\(|registry\.projects|projectSlug|slugify\(|slugFromRepo\(|registrySchema|registryProjectSchema' srcRepository: kaizen-agents-org/kaizen-loop
Length of output: 23341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any explicit slug validation or normalization beyond slugFromRepo.
rg -n -C2 'slug.*regex|regex\(/.*slug|slugFromRepo\(|projectSlug|registry\.projects\[|workspaceDir\(' src/config src/init src/commands src/utils src/workspaceRepository: kaizen-agents-org/kaizen-loop
Length of output: 3103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any explicit slug validation or normalization beyond slugFromRepo.
rg -n -C2 'slug.*regex|regex\(/.*slug|slugFromRepo\(|projectSlug|registry\.projects\[|workspaceDir\(' src/config src/init src/commands src/utils src/workspaceRepository: kaizen-agents-org/kaizen-loop
Length of output: 3103
Enforce registry slug validation before deriving workspace paths.
resolveProject and upsertProject accept arbitrary string keys, and slugFromRepo() only strips /, so a registry slug containing .., \, or other separators can still push workspaceDir(slug) outside workspaces. Reject unsafe slugs at the registry boundary, or guard assertSafeWorkspacePath before it uses workspaceDir(slug).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/fleet.ts` around lines 208 - 212, The workspace safety check in
assertSafeWorkspacePath is relying on workspaceDir(slug) after the slug has
already been accepted, so unsafe registry keys can still escape the intended
workspaces root. Add slug validation at the registry boundary in resolveProject
and upsertProject, or harden assertSafeWorkspacePath to reject slugs containing
path separators, dot segments, or other unsafe characters before calling
workspaceDir(slug). Keep the fix centered around slugFromRepo, resolveProject,
upsertProject, and assertSafeWorkspacePath so the workspace path can never be
derived from an unsafe slug.
Closes #78
Summary
fleet refresh CLI と dogfood script を追加し、登録済み workspace の sync/setup/verify readiness を検証できるようにしました。
失敗時は JSON/終了コードで可視化し、回帰テストと CLI ドキュメントも追加しました。
Builder notes
検証: npm test, npm run typecheck, npm run build, shared skill file checks passed. Commit: 623bbce kaizen: add fleet refresh verification (#78)
Verification
npm testnpm run typechecknpm run buildtest -f skills/gh-link-issue-pr/SKILL.md && test -f skills/kaizen-bug-router/SKILL.md && test -f skills/pr-guardian/SKILL.mdKaizen risk policy
Verifier cleared PR: Open PR with 0 should_fix item(s); risk is low.
Changed files: 8
Changed lines: 380
Summary by CodeRabbit
New Features
kaizen fleet refreshto refresh and validate all registered workspaces in one command.--projecttargeting,--syncbehavior, and--jsonoutput.Bug Fixes
Documentation
fleet refreshcommand behavior, verification flow, and exit conditions.Tests