-
Notifications
You must be signed in to change notification settings - Fork 20
fix(server,web): PR #786 follow-up — pi review findings + dead-code sweep #803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
ce2d66e
feat(web): replace bare Loading… text with skeleton placeholders
buremba d56b292
feat(web): route loaders prefetch primary data + unified pending skel…
buremba 671ca82
feat(web): more route loaders + skeletons (audit follow-up)
buremba 929c6f0
chore(web): drop dead code surfaced by the loader cleanup
buremba 2aed413
feat(server,web): linked-entity inline lookups, feed-list perf, watch…
buremba 027b071
chore(web): knip cleanup — drop dead exports, unexport in-file utilities
buremba d9c7c1b
fix(web): address pi review findings on PR #786
buremba 479f8d8
fix(server): guard agent_id on watcher update + NOT NULL at the schema
buremba 747d505
Merge remote-tracking branch 'origin/main' into feat/web-loading-skel…
buremba f1c45ae
fix: schema.sql worker_id COMMENT placement, list_feeds feed_key narr…
buremba 5b46995
chore(submodule): bump packages/web to merged owletto/main (ca12cd2)
buremba File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| -- migrate:up | ||
|
|
||
| -- Archive watchers that are flagged `status = 'active'` but have no `agent_id`. | ||
| -- The scheduler (packages/server/src/watchers/automation.ts:469) already | ||
| -- filters them out with `WHERE w.agent_id IS NOT NULL`, so these rows are | ||
| -- zombies — visible in the API, redirect-bounced on the watcher detail | ||
| -- route, never actually executing. | ||
| -- | ||
| -- 2026-05-17 prod audit: 28 active orphans across 11 orgs (most originated | ||
| -- from older create paths before `agent_id` was wired in). Archiving them | ||
| -- aligns the stored status with their actual execution state and lets the | ||
| -- `/$owner/watchers/$watcherId` redirect logic stop silently sending users | ||
| -- to /agents. | ||
| -- | ||
| -- The write-time guard (this same PR, manage_watchers.ts) rejects new | ||
| -- watcher creates/updates that set a schedule without an agent, so the | ||
| -- orphan set can't grow again. | ||
|
|
||
| UPDATE public.watchers | ||
| SET status = 'archived', | ||
| updated_at = now() | ||
| WHERE status = 'active' | ||
| AND agent_id IS NULL; | ||
|
|
||
| -- migrate:down | ||
|
|
||
| -- No-op: this is a one-shot data cleanup. The original rows can be restored | ||
| -- manually if needed by assigning an agent_id and flipping status back to | ||
| -- 'active'; without an agent, status='active' is meaningless to the | ||
| -- scheduler. |
34 changes: 34 additions & 0 deletions
34
db/migrations/20260517050000_watcher_agent_id_not_null.sql
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| -- migrate:up | ||
|
|
||
| -- Delete every watcher with `agent_id IS NULL` and enforce the column at | ||
| -- the schema level. These rows are leftovers from before the watcher | ||
| -- scheduler required `agent_id` (see watchers/automation.ts:469) — they | ||
| -- couldn't fire and the prior migration 20260517040000 had already | ||
| -- archived the active subset. Their dependent rows (windows, reactions, | ||
| -- versions, classifiers, field feedback) describe runtime state for | ||
| -- watchers that will never execute; ON DELETE CASCADE clears them. | ||
| -- `runs.watcher_id` is ON DELETE SET NULL, so the 21k historical run | ||
| -- records remain — they just lose the watcher linkage. | ||
| -- | ||
| -- Application-level guards in manage_watchers.ts (handleCreate + | ||
| -- handleCreateFromVersion) reject new inserts without `agent_id`; the | ||
| -- NOT NULL constraint below is the matching DB-level enforcement so | ||
| -- bypass paths can't reintroduce the zombie state. | ||
|
|
||
| DELETE FROM public.watchers WHERE agent_id IS NULL; | ||
|
|
||
| ALTER TABLE public.watchers | ||
| ALTER COLUMN agent_id SET NOT NULL; | ||
|
|
||
| -- The existing index is partial (`WHERE agent_id IS NOT NULL`), which is | ||
| -- a tautology once the column is NOT NULL. Replace it with an unconditional | ||
| -- index so explain plans don't show the dead predicate. | ||
| DROP INDEX IF EXISTS public.idx_watchers_agent_id; | ||
| CREATE INDEX idx_watchers_agent_id ON public.watchers USING btree (agent_id); | ||
|
|
||
| -- migrate:down | ||
|
|
||
| ALTER TABLE public.watchers | ||
| ALTER COLUMN agent_id DROP NOT NULL; | ||
|
|
||
| -- No restoring deleted rows — this is forward-only data cleanup. |
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a schema declares
x-link-entity-typewithoutx-link-lookup-field, this defaults toslug, but several built-in schemas use integer FK columns without an explicit lookup field (for exampleexamples/atlas/models/schema.yaml:11-16andexamples/market/models/schema.yaml:303-308). For those pages the visible metadata values are entity IDs likecompany_id, so this new server-side resolver queriese.slug = ANY(...)with numeric IDs and returns nolinked_entities, regressing the inline linked-entity display that this path is meant to replace.Useful? React with 👍 / 👎.