docs(proactive-agents): rewrite cron/radio guards as positive conditions#953
Conversation
The cron and integration examples both used `if (a !== x || b !== y) return;`
early-return guards. The mono font used in the rendered code block
ligatures `!==` into a glyph where the `!` is nearly invisible — readers
saw `event.source = 'cron'` and reasonably concluded the guard fires
*on* cron rather than gating *for* it.
Flipped both to `if (a === x && b === y) { ... }` so:
- No `!==` to misrender.
- The happy path is what reads naturally.
- The intent is the same; the dispatch handler in the Combined example
already uses this style.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTwo TypeScript handler examples in the proactive agents docs were changed to use positive conditional blocks instead of early-return guards: the cron/digest example now runs inline harness execution, posts to Slack, and saves memory on success; the GitHub issue listener now wraps payload extraction and commenting in an explicit conditional. ChangesProactive Agent Handler Examples
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
web/content/docs/proactive-agents.mdxParsing error: Expression expected. 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 |
Previous version handwaved with `summarizeOvernightShips(ctx) // your code` — readers saw the dispatch shape but not what the inside of a handler actually looks like, which is where the interesting `ctx` surface lives. Replaced with a real ~18-line implementation that exercises: - ctx.persona.inputs for deploy-time config - ctx.harness.run for AI work over the Relayfile mount - ctx.slack.post for delivery (typed integration client) - ctx.log for structured logging visible in `deployments logs` - ctx.memory.save for cross-firing persistence Added a short bullet list immediately under the snippet annotating each `ctx` call so a first-time reader knows what they're looking at. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Preview deployed!
This preview will be cleaned up when the PR is merged or closed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@web/content/docs/proactive-agents.mdx`:
- Around line 99-100: The docs say ctx.harness.run() returns { output, exitCode,
durationMs } but the example only destructures { output, exitCode }; update the
example where ctx.harness.run is used to destructure all three fields (output,
exitCode, durationMs) so it matches the documented return shape, or
alternatively add a short inline note in the example that durationMs is omitted
because it’s unused; locate the usage of ctx.harness.run and adjust the
destructuring there (e.g., const { output, exitCode, durationMs } = await
ctx.harness.run({...}) or add a parenthetical comment about omitted fields).
🪄 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: CHILL
Plan: Pro Plus
Run ID: e71693de-a4d1-4726-a4b5-8a37bdd96783
📒 Files selected for processing (1)
web/content/docs/proactive-agents.mdx
CodeRabbit on #953: the prose said `Returns { output, exitCode, durationMs }` but the example only destructured `{ output, exitCode }`. Reader could think they had to handle durationMs. Tightened the prose to list all fields including `usage?` (matches the HarnessRunResult shape in @agentworkforce/runtime) and added an explicit "destructure the fields you need" so the example's partial pull is clearly intentional. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Follow-up to #952. The rendered code block ligatures `!==` into a glyph where the `!` is nearly invisible — readers see `event.source = 'cron'` and reasonably conclude the guard fires on cron rather than gating for it. Same problem in the Integration-based example.
Before:
```ts
if (event.source !== 'cron' || event.name !== 'daily') return;
const digest = await summarizeOvernightShips(ctx);
await ctx.slack!.post('ops-daily', digest);
```
After:
```ts
if (event.source === 'cron' && event.name === 'daily') {
const digest = await summarizeOvernightShips(ctx);
await ctx.slack!.post('ops-daily', digest);
}
```
Benefits:
Added one sentence after the cron example noting the intentional choice — important once a single handler reacts to multiple listener kinds.
Test plan
🤖 Generated with Claude Code