feat(server): durable PR monitor feedback delivery and reporting - #182
Merged
ronak-guliani merged 3 commits intoAug 12, 2026
Merged
Conversation
github-actions
Bot
added
vouch:trusted
This was referenced Aug 11, 2026
Base automatically changed from
feat/pr-monitor-a-contracts-observe
to
base/pr-monitor-v2
August 11, 2026 19:37
Add feedback items/revisions/deliveries with debounce and circuit breakers, queue-mode V2 delivery via ThreadManagementService, MCP context/report tools, status audit fields, and migration 051. Never steers active turns. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ronak-guliani
force-pushed
the
feat/pr-monitor-b-feedback-reporting
branch
from
August 11, 2026 19:37
65d74d3 to
6805485
Compare
There was a problem hiding this comment.
Pull request overview
Adds durable PR-monitor feedback ingestion, queued delivery, reporting, and audit visibility across server, MCP, contracts, and web UI.
Changes:
- Persists feedback revisions, deliveries, reports, debounce, and circuit state.
- Adds queued thread delivery plus MCP context/report tools.
- Exposes feedback status through RPC and the web monitor strip.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
packages/contracts/src/rpc.ts |
Adds context/report RPC contracts. |
packages/contracts/src/pullRequestMonitor.ts |
Defines feedback schemas and status fields. |
packages/client-runtime/src/state/pullRequests.ts |
Adds context/report client operations. |
apps/web/src/components/pullRequest/PullRequestMonitorStrip.tsx |
Displays feedback audit summaries. |
apps/server/src/ws.ts |
Registers new RPC handlers. |
apps/server/src/server.ts |
Wires the feedback service layer. |
apps/server/src/pullRequestMonitor/wakePrompt.ts |
Builds bounded delivery prompts. |
apps/server/src/pullRequestMonitor/wakePrompt.test.ts |
Tests prompt formatting. |
apps/server/src/pullRequestMonitor/PullRequestMonitorService.ts |
Integrates feedback and reporting. |
apps/server/src/pullRequestMonitor/PullRequestMonitorService.test.ts |
Updates service status tests. |
apps/server/src/pullRequestMonitor/PullRequestMonitorFeedbackStore.ts |
Implements feedback persistence. |
apps/server/src/pullRequestMonitor/PullRequestMonitorFeedbackService.ts |
Implements ingestion and delivery workflow. |
apps/server/src/persistence/Migrations/051_PullRequestMonitorFeedback.ts |
Creates feedback tables and indexes. |
apps/server/src/persistence/Migrations.ts |
Registers migration 051. |
apps/server/src/mcp/toolkits/orchestrator/tools.ts |
Declares monitor MCP tools. |
apps/server/src/mcp/toolkits/orchestrator/handlers.ts |
Implements monitor MCP handlers. |
apps/server/src/auth/RpcAuthorization.ts |
Assigns RPC authorization scopes. |
Suppressed comments (7)
apps/server/src/pullRequestMonitor/PullRequestMonitorFeedbackService.ts:156
- The primary web start path sends no
ownerThreadId(PullRequestMonitorStrip.tsx:83-86), so this return silently discards every actionable event for those monitors. Ownership should gate materialization/delivery, not durable ingestion; otherwise the new feedback audit remains empty and later ownership cannot recover events already consumed by the monitor cursor.
if (!input.monitor.ownerThreadId) return;
apps/server/src/pullRequestMonitor/PullRequestMonitorFeedbackService.ts:454
- This uses the monitor's current head SHA rather than the
head_sharecorded on each pending revision. If the head changes during the debounce window, a later poll updatesmonitor.headSha, this batch is keyed to the new SHA, and fresh revalidation passes while still delivering revisions from the old head. Load the revision heads and suppress or split batches whose revisions do not match the fresh head.
const ids = stableDeliveryIds({
monitorId: monitor.id,
threadId: monitor.ownerThreadId,
revisionIds,
headSha: monitor.headSha ?? "unknown",
});
apps/server/src/pullRequestMonitor/PullRequestMonitorFeedbackService.ts:511
- This starts a second background fiber for the same non-claimed flush already run every five seconds above. The fibers can select the same due delivery concurrently; a late failure can overwrite a successful
deliveredupdate withfailed, causing needless retries. Keep one flusher or add an atomic claim/lease before allowing overlapping workers.
yield* Stream.fromSchedule(Schedule.spaced(Duration.seconds(20))).pipe(
Stream.mapEffect(() => flushDueDeliveries),
Stream.runDrain,
Effect.forkScoped,
Effect.interruptible,
apps/server/src/pullRequestMonitor/PullRequestMonitorFeedbackService.ts:357
- The snapshot was freshly fetched, but the prompt status comes from the previously polled monitor record. Checks, reviews, or mergeability can change without a head change, so delivery can tell the agent a stale blocker/readiness state. Recompute readiness from
snapshot(with the appropriate thread-version baseline) before building the prompt.
const readiness = monitor.readiness ?? {
ready: false,
label: "blocked" as const,
blockers: [{ kind: "checks-missing" as const }],
};
apps/server/src/mcp/toolkits/orchestrator/handlers.ts:125
- This mutation is not scoped to the invoking MCP thread/project, and the caller-provided
reporterThreadIdcan overridescope.threadId. An agent can therefore mutate another monitor's feedback and forge the audit attribution. Validate the monitor against the invocation scope and always derive the reporter ID fromscope.
const scope = yield* McpInvocationContext;
const monitors = yield* PullRequestMonitorService;
return yield* monitors.report({
...input,
reporterThreadId: input.reporterThreadId ?? scope.threadId,
});
apps/server/src/pullRequestMonitor/PullRequestMonitorFeedbackService.ts:558
- The disposition update commits before the required audit report is inserted. If the insert fails, the feedback item has changed state with no report, while the caller receives an error and may retry. Persist the disposition and report in one transaction to preserve the promised durable audit trail.
yield* feedbackStore.setDisposition({
itemId: item.id,
disposition: input.disposition,
note: input.note ?? null,
at: now,
apps/server/src/pullRequestMonitor/PullRequestMonitorFeedbackService.ts:494
- The due list can contain multiple batches for one monitor, but they are delivered concurrently while sharing one
deliveryFailureCount/circuit state. Two failures can both read the same count and collapse into one, or a success and failure can race to leave either final state; newer feedback may also be queued before older feedback. Serialize deliveries per monitor while retaining concurrency across different monitors.
const due = yield* feedbackStore.listDueDeliveries(now, 16);
yield* Effect.forEach(due, deliverOne, { concurrency: 2 });
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+360
to
+364
| yield* feedback | ||
| .ingestSnapshot({ | ||
| monitor: updated, | ||
| snapshot, | ||
| readiness, |
Comment on lines
+422
to
+426
| yield* store.update({ | ||
| ...monitor, | ||
| nextPollAt: now, | ||
| updatedAt: now, | ||
| }); |
Comment on lines
+113
to
+116
| t3_pr_monitor_context: (input) => | ||
| Effect.gen(function* () { | ||
| const monitors = yield* PullRequestMonitorService; | ||
| return yield* monitors.context(input); |
| export const PullRequestMonitorContextResult = Schema.Struct({ | ||
| monitor: Schema.NullOr(PullRequestMonitorRecord), | ||
| latestSnapshot: Schema.NullOr(PullRequestMonitorSnapshot), | ||
| openItems: Schema.Array(PullRequestMonitorFeedbackItem), |
Comment on lines
+151
to
+154
| const ingestSnapshot: (typeof PullRequestMonitorFeedbackService.Service)["ingestSnapshot"] = ( | ||
| input, | ||
| ) => | ||
| Effect.gen(function* () { |
Comment on lines
190
to
+193
| assert.isNotNull(status.monitor); | ||
| assert.isNotNull(status.latestSnapshot); | ||
| assert.strictEqual(status.latestSnapshot?.headSha, "deadbeef"); | ||
|
|
||
| const stopped = yield* service.stop({ monitorId: started.monitor.id }); | ||
| assert.strictEqual(stopped.monitor.enabled, false); | ||
| assert.strictEqual(stopped.monitor.status, "stopped"); | ||
| assert.isArray(status.openFeedback); | ||
| assert.isArray(status.recentDeliveries); | ||
| assert.isArray(status.recentReports); |
Owner
Author
Review follow-upAddressed on tip branch
Stack tip: #184 |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Owner
Author
PR B review disposition (assessed against stack design + heads)
Deferred?
Transactional-state / PR size
Where code is
Recommend rebasing C→D onto an updated B (or merging tip-first) so intermediate PR diffs match tip correctness. |
Cursor advances only after feedback ingest, durable ingest without owner, MCP project scope, transactional disposition reports, poll-only recheck updates, per-monitor delivery serialization, and items rename. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Owner
Author
Back-port landed
C and D were rebuilt onto this tip. #182 is ready to merge once CI is green (merge into its base / stack order A→B). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stack PR B (phases 3–4), stacked on A (
feat/pr-monitor-a-contracts-observe).Behavior
051)ThreadManagementService.sendToThread(mode: "queue") — never steers active turnst3_pr_monitor_context,t3_pr_monitor_report(accepted/rejected/resolved/needs-human) with immediate recheckMigrations
051_PullRequestMonitorFeedbackTests
apps/server/src/pullRequestMonitor/*including wakePrompt + service status feedback fieldsStack