feat(server): ship watcher prompt in device poll payload (+ owletto pointer)#1088
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe PR updates the watcher-run poll endpoint to return the watcher prompt snapshot from the version pinned when the run was queued, rather than the current version. The database query is extended with a LEFT JOIN to fetch the correct version's prompt, the response payload is augmented to include it, and a new integration test validates the behavior across version edits. ChangesWatcher Prompt Snapshot Delivery
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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 docstrings
🧪 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
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/server/src/worker-api.ts (1)
544-546: ⚡ Quick winHarden watcher_versions join against non-numeric
approved_input.version_id
createWatcherRunpersistsapproved_input.version_idasnumber | null(snapshot ofwatchers.current_version_id), so the current::bigintcast shouldn’t fail for runs created by this code. Still, a corrupted/legacy row with non-numericapproved_input.version_idwould make the poll SQL hard-fail; guarding the cast prevents repeated/api/workers/poll500s.Proposed fix
LEFT JOIN watcher_versions wv - ON wv.id = COALESCE((r.approved_input->>'version_id')::bigint, w.current_version_id) + ON wv.id = COALESCE( + CASE + WHEN (r.approved_input->>'version_id') ~ '^[0-9]+$' + THEN (r.approved_input->>'version_id')::bigint + ELSE NULL + END, + w.current_version_id + ) AND wv.watcher_id = w.watcher_group_id🤖 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 `@packages/server/src/worker-api.ts` around lines 544 - 546, Guard the unsafe cast of r.approved_input->>'version_id' to bigint in the LEFT JOIN to avoid hard failures for non-numeric legacy/corrupted rows: replace COALESCE((r.approved_input->>'version_id')::bigint, w.current_version_id) with a safe expression such as COALESCE(CASE WHEN (r.approved_input->>'version_id') ~ '^\d+$' THEN (r.approved_input->>'version_id')::bigint ELSE NULL END, w.current_version_id) so the join to watcher_versions (alias wv) falls back to w.current_version_id instead of erroring; this change touches the LEFT JOIN using r.approved_input and w.current_version_id (created by createWatcherRun).
🤖 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.
Nitpick comments:
In `@packages/server/src/worker-api.ts`:
- Around line 544-546: Guard the unsafe cast of r.approved_input->>'version_id'
to bigint in the LEFT JOIN to avoid hard failures for non-numeric
legacy/corrupted rows: replace
COALESCE((r.approved_input->>'version_id')::bigint, w.current_version_id) with a
safe expression such as COALESCE(CASE WHEN (r.approved_input->>'version_id') ~
'^\d+$' THEN (r.approved_input->>'version_id')::bigint ELSE NULL END,
w.current_version_id) so the join to watcher_versions (alias wv) falls back to
w.current_version_id instead of erroring; this change touches the LEFT JOIN
using r.approved_input and w.current_version_id (created by createWatcherRun).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 329d3c51-1afb-4228-8bdc-e31cf41112e9
📒 Files selected for processing (3)
packages/owlettopackages/server/src/__tests__/integration/watchers/manual-trigger.test.tspackages/server/src/worker-api.ts
|
bug_free 84, simplicity 90, slop 0, bugs 0, 0 blockers Read diff and logs. Typecheck/unit passed; changed manual-trigger integration passed (7 tests) including run-pinned prompt. [env] integration exit 1 came from untouched src/lobu/tests/agent-routes-apply.test.ts embedded Postgres shared-memory exhaustion. Skipped extra runtime probe. Full verdict JSON{
"bug_free_confidence": 84,
"bugs": 0,
"slop": 0,
"simplicity": 90,
"blockers": [],
"change_type": "feat",
"behavior_change_risk": "low",
"tests_adequate": true,
"suggested_fixes": [],
"notes": "Read diff and logs. Typecheck/unit passed; changed manual-trigger integration passed (7 tests) including run-pinned prompt. [env] integration exit 1 came from untouched src/lobu/__tests__/agent-routes-apply.test.ts embedded Postgres shared-memory exhaustion. Skipped extra runtime probe.",
"categories": {
"src": 17,
"tests": 60,
"docs": 0,
"config": 0,
"deps": 2,
"migrations": 0,
"ci": 0,
"generated": 0
}
}Local review gate — branch protection can require the |
buremba
force-pushed
the
feat/watcher-ship-prompt
branch
from
7d4134b to
78a9f8d
Compare
The device-watcher poll envelope omitted the watcher's prompt/schema, so a device-local (local-CLI) executor had no channel for the watcher's actual instructions. Join the current watcher_version and include wv.prompt as payload.watcher.prompt so the device can run the real prompt. Null-safe for watchers without a version row.
buremba
force-pushed
the
feat/watcher-ship-prompt
branch
from
78a9f8d to
23ce31f
Compare
2 participants
What
Device-local watchers got no instructions: the device poll envelope (
worker-api.ts, therun_type==='watcher'branch) shipped only watcher id/name/slug, so the Mac app's local CLI received a bare "process this" and improvised toward the wrong data source. This joins the run's pinned watcher version and ships its prompt aspayload.watcher.prompt, so the device runs the watcher's real instructions.How
watcher_versionsonCOALESCE((approved_input->>'version_id')::bigint, current_version_id)— the same run-pinned resolutioncomplete_windowuses (manage_watchers.ts:1614), with the sameAND wv.watcher_id = w.watcher_group_idguard so a forged version_id can't resolve another watcher's prompt.promptto the payload + row type.Tests
manual-trigger.test.ts): queue a run (pins prompt v1) → create_version v2 → assert the poll payload returns v1 (the snapshot), not the edit.make review: typecheck/unit/integration all green; pi endorsed the approach (right layer, multi-replica safe) and flagged the scope-guard, now applied. 8/8 in the touched test file pass with the guard.Not done (follow-ups noted by pi, out of scope)
Prompt rides in
claude -pargv (future: stdin/temp-file if prompts carry secrets); no max prompt-length cap yet; version-pin-in-JSONB → dedicated column tracked in #799.Deploy note
Live e2e needs this server deployed (prod) + the Mac app rebuilt with owletto#231; validated in isolation (server ships it [tested], app runs it [compiles], the prompt produces a real plan [local claude -p test]) but the assembled prod path is unobserved until shipped.
Summary by CodeRabbit
Bug Fixes
Tests
Chores