Conversation
2. 新增 src/commands/fork/fork.tsx — 完整实现 /fork 命令,复用了 AgentTool 的 fork 子 agent 逻辑
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
📝 WalkthroughWalkthroughThe PR adds defensive error handling in the Ultraplan launch flow to safely access Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tsconfig.json (1)
7-7: Isolatestrict: falseto generated code rather than applying it globally.The change applies
"strict": falseto all source files, but asrc/types/generated/directory exists in the codebase. Limit type-checking relaxation to this directory using a dedicated tsconfig entry or biome.json exclusion instead of disabling strict mode for the entire project.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsconfig.json` at line 7, The tsconfig currently sets "strict": false globally; instead limit non-strict checking to the generated types by creating a dedicated tsconfig (e.g., tsconfig.generated.json) or add an exclusion for src/types/generated and restore "strict": true in the main tsconfig.json; update project references or build scripts to include the new tsconfig for that directory so only files under src/types/generated use relaxed checks while the rest of the codebase remains strict.
🤖 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/commands/ultraplan.tsx`:
- Around line 370-376: The code is using "as any" and redundant Array.isArray
checks even though the branch if (!eligibility.eligible) already narrows
eligibility so eligibility.errors is a BackgroundRemoteSessionPrecondition[].
Replace the Array.isArray((eligibility as any).errors) guards and the as any
casts by directly using eligibility.errors (e.g. precondition_errors:
eligibility.errors.map(e => e.type).join(','), and const reasons =
eligibility.errors.map(formatPreconditionError).join('\n')), or if the compiler
still needs help, cast eligibility to a specific shape (e.g. eligibility as {
errors: BackgroundRemoteSessionPrecondition[] }) rather than using any.
---
Nitpick comments:
In `@tsconfig.json`:
- Line 7: The tsconfig currently sets "strict": false globally; instead limit
non-strict checking to the generated types by creating a dedicated tsconfig
(e.g., tsconfig.generated.json) or add an exclusion for src/types/generated and
restore "strict": true in the main tsconfig.json; update project references or
build scripts to include the new tsconfig for that directory so only files under
src/types/generated use relaxed checks while the rest of the codebase remains
strict.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c1e725f2-3cec-4281-85bd-ba0d36219c00
⛔ Files ignored due to path filters (1)
contributors.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
src/commands/ultraplan.tsxtsconfig.json
| precondition_errors: Array.isArray((eligibility as any).errors) | ||
| ? (eligibility as any).errors.map((e: any) => e.type).join(',') as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS | ||
| : undefined, | ||
| }); | ||
| const reasons = eligibility.errors.map(formatPreconditionError).join('\n'); | ||
| const reasons = Array.isArray((eligibility as any).errors) | ||
| ? (eligibility as any).errors.map(formatPreconditionError).join('\n') | ||
| : ''; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== RemoteAgentPreconditionResult / eligibility definitions =="
rg -nP --type=ts -C3 '\bRemoteAgentPreconditionResult\b|\bcheckRemoteAgentEligibility\b'
echo
echo "== current ultraplan eligibility handling =="
rg -nP --type=tsx -C4 'if \(!eligibility\.eligible\)|eligibility as any|precondition_errors|formatPreconditionError'Repository: claude-code-best/claude-code
Length of output: 4913
🏁 Script executed:
sed -n '360,380p' src/commands/ultraplan.tsxRepository: claude-code-best/claude-code
Length of output: 1088
Remove as any casts; the discriminated union type is already narrowed in this branch.
At line 367 (if (!eligibility.eligible)), the type system guarantees eligibility.errors is present and properly typed as BackgroundRemoteSessionPrecondition[]. The as any casts at lines 370–376 and the redundant Array.isArray() guards weaken type safety and violate the coding guideline: "Use type guards and Record<string, unknown> instead of as any in non-test code."
See the pattern in src/tools/AgentTool/AgentTool.tsx (line 672) for the correct approach: use a specific type assertion instead of as any.
Proposed fix
if (!eligibility.eligible) {
+ const preconditionErrors = eligibility.errors.map(e => e.type)
logEvent('tengu_ultraplan_create_failed', {
reason: 'precondition' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
- precondition_errors: Array.isArray((eligibility as any).errors)
- ? (eligibility as any).errors.map((e: any) => e.type).join(',') as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS
- : undefined,
+ precondition_errors:
+ preconditionErrors.join(',') as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
});
- const reasons = Array.isArray((eligibility as any).errors)
- ? (eligibility as any).errors.map(formatPreconditionError).join('\n')
- : '';
+ const reasons = eligibility.errors.map(formatPreconditionError).join('\n')
enqueuePendingNotification({
value: `ultraplan: cannot launch remote session —\n${reasons}`,
mode: 'task-notification',
});📝 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.
| precondition_errors: Array.isArray((eligibility as any).errors) | |
| ? (eligibility as any).errors.map((e: any) => e.type).join(',') as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS | |
| : undefined, | |
| }); | |
| const reasons = eligibility.errors.map(formatPreconditionError).join('\n'); | |
| const reasons = Array.isArray((eligibility as any).errors) | |
| ? (eligibility as any).errors.map(formatPreconditionError).join('\n') | |
| : ''; | |
| if (!eligibility.eligible) { | |
| const preconditionErrors = eligibility.errors.map((e) => e.type) | |
| logEvent('tengu_ultraplan_create_failed', { | |
| reason: 'precondition' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, | |
| precondition_errors: | |
| preconditionErrors.join(',') as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, | |
| }); | |
| const reasons = eligibility.errors.map(formatPreconditionError).join('\n') | |
| enqueuePendingNotification({ | |
| value: `ultraplan: cannot launch remote session —\n${reasons}`, | |
| mode: 'task-notification', | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/ultraplan.tsx` around lines 370 - 376, The code is using "as
any" and redundant Array.isArray checks even though the branch if
(!eligibility.eligible) already narrows eligibility so eligibility.errors is a
BackgroundRemoteSessionPrecondition[]. Replace the Array.isArray((eligibility as
any).errors) guards and the as any casts by directly using eligibility.errors
(e.g. precondition_errors: eligibility.errors.map(e => e.type).join(','), and
const reasons = eligibility.errors.map(formatPreconditionError).join('\n')), or
if the compiler still needs help, cast eligibility to a specific shape (e.g.
eligibility as { errors: BackgroundRemoteSessionPrecondition[] }) rather than
using any.
Summary by CodeRabbit