Remove task prompt injection#32
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 15 minutes and 19 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request systematically removes system prompt configuration capabilities across the codebase by eliminating Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
52a9599 to
4ef5aee
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/crons/runtime_test.go (1)
42-50: Assert the dispatched task shape, not justTaskID.This fixture changed specifically around the dispatch payload, but the test still only checks
dispatched.TaskIDlater. A regression in forwardingPrompt,PermissionMode,CWD, orClaudeSessionID—or accidental reintroduction of the removed prompt-injection field ifprotocol.Taskstill exposes it—would still pass.Suggested test tightening
- if dispatched == nil || dispatched.TaskID != "task-1" { - t.Fatalf("expected claimed task to be dispatched, got %+v", dispatched) - } + if dispatched == nil { + t.Fatal("expected claimed task to be dispatched") + } + if dispatched.TaskID != "task-1" || + dispatched.SessionID != "session-1" || + dispatched.ChannelID != "cron-cron-1-123" || + dispatched.Prompt != "run tests" || + dispatched.Model != "claude-opus-4-6[1m]" || + dispatched.Effort != "max" || + dispatched.PermissionMode != "acceptEdits" || + dispatched.CWD != "/tmp/project" || + dispatched.ClaudeSessionID != "claude-session-1" { + t.Fatalf("unexpected dispatched task: %+v", dispatched) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/crons/runtime_test.go` around lines 42 - 50, The test currently only asserts dispatched.TaskID; update the assertion to validate the full dispatched task shape by checking dispatched.Prompt, dispatched.PermissionMode, dispatched.CWD, dispatched.ClaudeSessionID, dispatched.Model, dispatched.Effort, dispatched.ChannelID and dispatched.SessionID match the fixture values and assert that any deprecated/removed field from protocol.Task (e.g. the old prompt-injection field) is not present or is empty; locate the dispatched variable in the runtime_test.go test and add these explicit equality (and absent/empty) checks against the fixture to prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/crons/runtime_test.go`:
- Around line 42-50: The test currently only asserts dispatched.TaskID; update
the assertion to validate the full dispatched task shape by checking
dispatched.Prompt, dispatched.PermissionMode, dispatched.CWD,
dispatched.ClaudeSessionID, dispatched.Model, dispatched.Effort,
dispatched.ChannelID and dispatched.SessionID match the fixture values and
assert that any deprecated/removed field from protocol.Task (e.g. the old
prompt-injection field) is not present or is empty; locate the dispatched
variable in the runtime_test.go test and add these explicit equality (and
absent/empty) checks against the fixture to prevent regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e35952c-4d39-4531-909f-63d55a2e82d7
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
go.modinternal/api/cron_claim.gointernal/api/cron_claim_test.gointernal/claude/executor.gointernal/claude/executor_test.gointernal/crons/runtime.gointernal/crons/runtime_test.gointernal/loop/daemon.gointernal/session/actor.gointernal/session/actor_test.go
💤 Files with no reviewable changes (5)
- internal/loop/daemon.go
- internal/claude/executor.go
- internal/session/actor_test.go
- internal/claude/executor_test.go
- internal/session/actor.go
Summary
Verification
Post-merge
Summary by CodeRabbit
Release Notes