Skip to content

fix(agent,admin): alert once per snapshot episode - #26

Draft
Oba-One wants to merge 3 commits into
mainfrom
claude/happy-wilbur-bb96e1
Draft

Oba-One wants to merge 3 commits into
mainfrom
claude/happy-wilbur-bb96e1

Conversation

@Oba-One

@Oba-One Oba-One commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Summary

A quarantined record or withheld chapter image previously alerted once per record forever. Migration 030 adds resolved_at and scopes the existing unique indexes to open episodes, retaining notification history. A recovery resolves the episode, and a later recurrence opens one new alert.

  • Serialize content observations and alert changes in a transaction with an advisory lock taken before reading content. This prevents stale renders from reopening resolved episodes or closing newly opened ones.
  • Resolve observed recoveries immediately; there is no minimum episode duration.
  • Use savepoints to keep alert-write failures from aborting the public snapshot or blocking the other alert kind.
  • Expose resolved_at in Directus and show only open episodes in the Quarantined records bookmark. Unchanged renders do not rewrite notification rows.

Refs PRD-1201.

Validation

On the exact PR-head checkout plus these fixes:

  • bun run typecheck
  • bun run test:agent — 78 agent tests and 14 moderation tests
  • bun run test:content — 33 tests
  • Directus Studio/access tests — 15 tests
  • bun run build — 47 pages
  • bun run plans:validate
  • Deterministic regression covers brief recovery and both stale-render sequences; removing the lock or restoring the grace interval makes it fail.
  • Failure stand-in models an aborted transaction, proving that savepoints preserve snapshot delivery.
  • Published tree matches the tested index: d5dc288f5f69082246dd16353b4eb5033f5dc3f7.

The previous scratch database proof passed during implementation. The updated proof now includes immediate recurrence and deliberately overlapping Postgres renders. It has not run on the new implementation: local Postgres is unavailable, and the user requested proceeding with tests that do not need it. Live Directus field/bookmark proof also remains pending. Rendered browser proof: none.

CodeRabbit approved the preceding head ad3c081 without actionable findings. The two additional correctness defects found in our review are fixed by 9bf6e9d; that approval is not evidence for the latest head.

Release

The Network repository uses main and has no develop branch. Applying migration 030, merging/deploying, and rerunning Directus setup require separate authorization. Merging to main auto-deploys the Fly agent. After applying the migration, clear the Directus cache and run directus:content:setup and directus:studio:setup.

Sensitive manifest change in the overall PR: one test script in package.json, with no dependency or lockfile change. The review fixes change only five runtime/test/documentation files.

Publication uses the connected GitHub API because shell Git transport/signing is unavailable. The isolated checkout is kept at the exact published head; the original local branch history is preserved.

Preserve resolved alert history and allow a recurrence to open one new alert.
Batch alert bookkeeping without rewriting unchanged rows; expose resolved_at
and filter active quarantines in Directus.

Refs PRD-1201

Published from local commits 5b311e1, c9f9a23, and 2408391 through the GitHub
connector because the environment blocks shell Git transport. The published
tree exactly matches local HEAD 2408391483a9ef757b21b60d266c230601da2250.
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 5e0ac0e4-f5fa-40f0-a168-18ee32fdd611
📥 Commits

Reviewing files that changed from the base of the PR and between 2546a58 and ad3c081.

📒 Files selected for processing (14)
  • .plans/archive/directus-cms-advancement/handoffs/README.md
  • package.json
  • packages/admin/README.md
  • packages/agent/README.md
  • packages/agent/migrations/030_snapshot_alert_episodes.sql
  • packages/agent/src/content-operations.ts
  • packages/agent/src/public-content.ts
  • scripts/agent-contract.test.ts
  • scripts/chapter-image-fields.integration.ts
  • scripts/directus-operational-content-setup.ts
  • scripts/directus-studio-setup.test.ts
  • scripts/directus-studio-setup.ts
  • scripts/public-content-contract.test.ts
  • scripts/snapshot-alert-episodes.integration.ts

Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

📜 Recent review details
🧰 Additional context used
📚 Code guidelines (2)
AGENTS.md — auto-discovered
CLAUDE.md — auto-discovered
📓 Path-based instructions (2)
Source excerpt: Treat `package.json`, lockfiles, package-manager config, `.github/workflows/**`, `AGENTS.md`, `CLAUDE.md`, `.codex/**`, and `.claude/**` as security-sensitive surfaces.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • package.json
Source excerpt: Do not install or upgrade Bun, Python, or package-manager dependencies unless the user explicitly approves that install in the current task.

📄 CodeRabbit inference engine (CLAUDE.md)

Files:

  • package.json
🪛 Betterleaks (1.8.1)
scripts/snapshot-alert-episodes.integration.ts

[high] 25-25: Detected a password embedded in a service connection URI, which may expose direct access to the referenced service.

(generic-credential-uri)

🪛 Squawk (2.65.0)
packages/agent/migrations/030_snapshot_alert_episodes.sql

[warning] 25-26: By default new constraints require a table scan and block writes to the table while that scan occurs. Use NOT VALID with a later VALIDATE CONSTRAINT call.

(constraint-missing-not-valid)

[warning] 35-35: A normal DROP INDEX acquires an ACCESS EXCLUSIVE lock on the table, blocking other accesses until the index drop can complete. Drop the index CONCURRENTLY.

(require-concurrent-index-deletion)

[warning] 36-38: During normal index creation, table updates are blocked, but reads are still allowed. Use concurrently to avoid blocking writes.

(require-concurrent-index-creation)

[warning] 40-40: A normal DROP INDEX acquires an ACCESS EXCLUSIVE lock on the table, blocking other accesses until the index drop can complete. Drop the index CONCURRENTLY.

(require-concurrent-index-deletion)

[warning] 41-43: During normal index creation, table updates are blocked, but reads are still allowed. Use concurrently to avoid blocking writes.

(require-concurrent-index-creation)

🔇 Additional comments (14)
.plans/archive/directus-cms-advancement/handoffs/README.md (1)

40-43: LGTM!

Also applies to: 59-66, 110-111

packages/agent/migrations/030_snapshot_alert_episodes.sql (1)

18-43: LGTM!

scripts/public-content-contract.test.ts (1)

50-50: LGTM!

Also applies to: 859-884

scripts/snapshot-alert-episodes.integration.ts (1)

1-431: LGTM!

scripts/chapter-image-fields.integration.ts (1)

197-201: LGTM!

packages/agent/src/content-operations.ts (1)

947-1071: LGTM!

packages/agent/src/public-content.ts (1)

10-10: LGTM!

Also applies to: 144-147

scripts/agent-contract.test.ts (1)

19-19: LGTM!

Also applies to: 1049-1246

package.json (1)

58-58: LGTM!

scripts/directus-operational-content-setup.ts (1)

923-923: LGTM!

scripts/directus-studio-setup.ts (1)

120-120: LGTM!

Also applies to: 918-918, 1154-1156

scripts/directus-studio-setup.test.ts (1)

6-6: LGTM!

Also applies to: 203-229

packages/admin/README.md (1)

238-238: LGTM!

Also applies to: 246-251

packages/agent/README.md (1)

125-181: LGTM!

📝 Summary

Summary by CodeRabbit

  • New Features
    • Snapshot alerts for quarantined records and withheld chapter images now track individual episodes. Alerts resolve when an issue is no longer detected and can reopen if it returns.
    • Data Studio now shows when alerts were resolved and filters quarantined records to those still unresolved.
  • Bug Fixes
    • Snapshot responses continue to be served if alert recording fails. A quarantined chapter remains tracked for image alerts while it is absent from the snapshot.

Snapshot alerts now track episodes for quarantined records and withheld chapter images. A migration adds resolution timestamps and updates uniqueness constraints. Snapshot renders reconcile affected alerts, while Directus exposes resolution status and filters its quarantine bookmark to unresolved alerts.

Changes: Snapshot alert episodes

Layer / File(s) Summary
Episode schema and migration checks
packages/agent/migrations/030_snapshot_alert_episodes.sql, scripts/public-content-contract.test.ts, scripts/snapshot-alert-episodes.integration.ts, scripts/chapter-image-fields.integration.ts, .plans/archive/directus-cms-advancement/handoffs/README.md
Migration 030 adds resolved_at, restricts its use to snapshot alert kinds, and changes the unique indexes to cover unresolved alerts. Tests check migration replay and preservation of existing alerts.
Snapshot alert reconciliation
packages/agent/src/content-operations.ts, packages/agent/src/public-content.ts, scripts/agent-contract.test.ts, scripts/snapshot-alert-episodes.integration.ts, package.json
Each snapshot render reconciles quarantine and withheld-image alerts, including when neither list contains records. Alerts can resolve after the 60-second minimum open period. Tests cover recurring alerts, concurrent renders, and failures that do not prevent snapshot delivery.
Directus visibility and rollout guidance
scripts/directus-operational-content-setup.ts, scripts/directus-studio-setup.ts, scripts/directus-studio-setup.test.ts, packages/admin/README.md, packages/agent/README.md
Directus exposes read-only resolved_at and filters its quarantine bookmark to unresolved alerts. Documentation describes alert episodes and migration and test steps.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant PublicContent as public-content.ts
  participant Reconciliation as reconcileSnapshotAlerts
  participant Database as review_notifications
  PublicContent->>Reconciliation: Submit quarantined and withheld records
  Reconciliation->>Database: Insert alerts for affected records
  Reconciliation->>Database: Resolve eligible alerts absent from the snapshot
  Database-->>Reconciliation: Return write results or errors
  Reconciliation-->>PublicContent: Complete reconciliation and log failures
Loading
Merge Risk: ⚪ Minimal · up to ad3c0

No confirmed issue blocks merging. Migration and Directus rollout still require the separately planned deployment steps.

Pre-merge checks · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 9 files. (5 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description directly explains the snapshot alert episode changes, migration 030, alert reconciliation, Directus updates, validation, and release requirements.
Title check Passed The title clearly and concisely describes the primary change: alerts now fire once per snapshot episode and can recur after resolution.
Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 9 files. (5 skipped: 5 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

MCP server "posthog" is unavailable: the server marks every selected tool as able to make changes, and CodeRabbit only uses read-only tools.

MCP server "posthog" is unavailable: the server marks every selected tool as able to make changes, and CodeRabbit only uses read-only tools.

I’m a rabbit; alerts now open when troubles appear.
A fix marks the episode resolved and clear.
If trouble returns, a new alert can begin.
The old row stays saved as a record within.
I nibble a carrot and hop past the queue.
Each snapshot checks what is missing anew.

Comment @coderabbitai help to get the list of available commands.

@Oba-One Oba-One added the automated/codex Created by Codex automation label Oct 13, 2026 — with ChatGPT Codex Connector
Keep the current chapter image-copy scripts and operator documentation while
adding the snapshot alert proof command and episode history notes.

Oba-One commented Oct 13, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Order content reads and alert changes with a transaction-scoped advisory lock.
Remove the grace period so a brief observed recovery ends its episode
immediately. Savepoints preserve public snapshot delivery and independent
alert kinds when a queue statement fails.

Refs PRD-1201

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated/codex Created by Codex automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant