Automations and webhooks for the AT Protocol airglow.run

# fix(logs): leave update markers out of the session-loss 401 streak #60

Closed tyb.there-hugo.dev
Closed opened by tyb.there-hugo.dev

Blocks merging the #38 branch (loop/3my33bvptg222).

Problem #

Since the kind column was added (#38), editing an automation writes a delivery_logs row with kind = 'automation-updated', dryRun = false and statusCode = null. The session-loss check reads the last N non-dry-run rows and requires all of them to be 401, so a marker in that window breaks the streak. An automation whose session is gone keeps running if the owner edits it while it is failing.

Evidence #

  • lib/jetstream/session-loss.ts:25-34, branch loop/3my33bvptg222 (40136b2)
  • lib/automations/update-log.ts:91-101, same branch

Expected #

The streak counts only delivery rows (kind IS NULL), as the activity summary and stats already do since 94e2a89.

Acceptance criteria #

Out of scope #

Writing markers for system disables (dry-run expiry, session loss). File that separately if wanted.

Queued: the loop will work on this version of the issue.

loop:queued cid=bafyreigy2kobhmx3svoebcgskbtlzmlbyc24ppvmojstkj7irtgaw5bxry

Working on this, attempt 1.

loop:claim run=contribute-20261028-164922-3myulrzgamw22 branch=loop/3myulrzgamw22 attempt=1

The session needs an answer from the owner.

fix(logs): leave update markers out of the session-loss 401 streak

I stopped before changing anything. The issue fixes code that only exists on the #38 branch (loop/3my33bvptg222), but this run's branch, loop/3myulrzgamw22, starts from main (acb8f87). main has no deliveryLogs.kind column and no lib/automations/update-log.ts, so on this branch there is no marker to filter out and no marker to write in the test.

What I checked #

  • rg -n kind lib/db/schema.ts on this branch: delivery_logs has no kind column. rg automation-updated finds nothing.
  • git branch -a --contains 40136b2: only origin/loop/3my33bvptg222 has it. git log main..origin/loop/3my33bvptg222 lists 40136b2, 6e2226e and 94e2a89, none of which are on main.
  • git merge-tree main origin/loop/3my33bvptg222: the #38 branch no longer merges cleanly with main. It conflicts in app/islands/DeliveryLog.tsx, app/routes/api/automations/[rkey].ts, app/routes/api/automations/[rkey]/logs.ts, app/routes/u/[handle]/[rkey]/logs.tsx and possibly more. It also adds migration 0003_known_princess_powerful.sql.

I can't switch to the #38 branch. To do this issue here I would have to merge that whole branch into this one and resolve its conflicts with main. That pulls all of #38 into this PR, which is outside this issue's scope, so I didn't do it.

What the owner needs to decide #

Which of these should I do?

  1. Run this as a fix round on the #38 branch (loop/3my33bvptg222). This is the most direct option, and that branch needs a fix round for its conflicts with main anyway.
  2. Merge #38 into main first, then re-run this issue on a fresh branch from main.
  3. Let this branch merge origin/loop/3my33bvptg222, resolve its conflicts with main, and add the fix on top. This PR would then carry all of #38.

The change, once kind exists (from reading the #38 branch at 94e2a89) #

  • lib/jetstream/session-loss.ts, disableForSessionLoss: add isNull(deliveryLogs.kind) to the where, next to eq(deliveryLogs.dryRun, false), and update the doc comment so it says the streak counts delivery rows only.
  • lib/jetstream/session-loss.test.ts: insert 4 non-dry-run 401 rows, then one kind: "automation-updated" row (statusCode: null), then one more 401. Check that disableForSessionLoss returns true and the automation is inactive with disabledReason = "session_lost".
  • On that branch, from(deliveryLogs) has 10 readers outside tests. Each needs a kind filter or a comment saying why it has none:
    • activity-summary.ts: 2 readers, already filtered since 94e2a89, to confirm.
    • admin-logs-query.ts: lists every row, so it gets a comment.
    • rate-limit.ts: already excludes markers through countsTowardRateLimit = false, so it gets a comment.
    • run-steps.ts: 3 readers, check each.
    • session-loss.ts: gets the filter above.
    • scripts/migrate-automations-to-steps.ts: 2 readers. This file matches the scripts/** ownerOnly glob only if the glob is root-anchored. It is under lib/scripts/, so it is editable.

Acceptance criteria #

loop:awaiting-owner run=contribute-20261028-164922-3myulrzgamw22 why=needs-info

  1. Run this as a fix round on the #38 branch (loop/3my33bvptg222). This is the most direct option, and that branch needs a fix round for its conflicts with main anyway.

Working on this, attempt 1.

loop:claim run=contribute-20261103-124811-60 branch=loop/60 attempt=1 issue=60

fix(logs): leave update markers out of the session-loss 401 streak

The session-loss check read the last five non-dry-run delivery_logs rows for an automation, and that included the "automation updated" marker rows #38 added. An edit made while the session was gone broke the 401 streak, so the automation kept running. disableForSessionLoss now counts only delivery rows (kind IS NULL), the same way the activity summary and stats do. Every other from(deliveryLogs) reader now says why it does or does not filter by kind.

Note on the owner's comment: #38 is already on main (398d214), so this branch starts from main (1dab9b7) and not from loop/3my33bvptg222.

What changed #

  • lib/jetstream/session-loss.ts: disableForSessionLoss adds isNull(deliveryLogs.kind) to its filter.
  • lib/jetstream/session-loss.test.ts: a new test writes 4 × 401, then a real marker through logAutomationUpdate, then one more 401, and checks that the automation is disabled at threshold 5. I ran the test against the old query and it failed there.
  • lib/scripts/migrate-automations-to-steps.ts: the step-id backfill also skips marker rows (isNull(kind)). Without that, a marker on an unmigrated automation would get actionIndex 0's step id. A marker belongs to no step.
  • These readers now have a one-line comment saying why they don't need a kind filter:
    • rate-limit.ts: markers are written with countsTowardRateLimit = false.
    • run-steps.ts (3 readers): markers are never dry-run and have no step id, status code or run id.
    • admin-logs-query.ts: the admin log shows markers on purpose.
    • The migration script's orphan check.
    • The test readers: they read every row on purpose.

Verified #

  • pnpm run check passes.
  • pnpm run test passes: 186 files, 2769 tests.
  • grep -rn "from(deliveryLogs)" lib app: every match is next to a kind filter or a comment. activity-summary.ts and lib/stats/compute.ts already filtered by kind.

Acceptance criteria #

Decisions you can reverse #

  • The migration-script backfill now has a real filter, not just a comment. I chose that because the backfill would have written a step id onto a marker. If you prefer a comment only, revert those lines.
  • I did not touch the db.query.deliveryLogs.findMany readers (logs-query.ts, api/automations/[rkey].ts, u/[handle]/[rkey]/logs.tsx). The grep in the criterion does not match them, and they are the log views, which show markers by design.
  • No UI change.

Out of scope and left alone: markers for system disables.

Review: https://tangled.org/exosphere.site/airglow/compare/main...loop/60

loop:pushed run=contribute-20261103-124811-60 sha=20b19cda71ddebb5259055486a413b30fb0c7d95

Verdict: merge. disableForSessionLoss now counts only delivery rows (isNull(deliveryLogs.kind)), so an "automation updated" marker no longer breaks the 401 streak. The new test writes 4 × 401, a real marker through logAutomationUpdate, then one more 401, and checks the automation is disabled at threshold 5. Every from(deliveryLogs) reader now either filters by kind or has a comment saying why it doesn't, and those reasons hold up against the code.

What I checked and how:

  • Criterion 1: lib/jetstream/session-loss.ts adds isNull(deliveryLogs.kind) next to the existing automation and dryRun = false filters, ordered by id and limited to the threshold.
  • Criterion 2: I put the base version of session-loss.ts back and ran vp test lib/jetstream/session-loss.test.ts. The new test failed ("leaves an update marker out of the 401 streak"), and it passes with the branch's fix. The marker comes from the same writer (logAutomationUpdate) that the PATCH route and deactivateRedundant use, so the test runs on the real row shape.
  • Criterion 3: I ran grep -rn "from(deliveryLogs)" across the repo, tests included, and checked each match's reason against the code:
    • rate-limit.ts: markers are written with countsTowardRateLimit = false, so the flag already leaves them out.
    • run-steps.ts, three readers:
      • dryRunAiCallCount needs dryRun = true and a step id in a list, which a marker never has.
      • The dry-run ceiling check matches on status code, and a marker has none.
      • runHasLoggedRows matches on run id, and a marker has none.
    • admin-logs-query.ts: the admin log shows markers on purpose.
    • The migration script's orphan check: a marker has a NULL step id, so the check passes over it.
    • The test readers: they read every row on purpose.
    • activity-summary.ts and lib/stats/compute.ts already filtered by kind.
  • Other readers of the table: I listed the readers the grep misses: the db.query.deliveryLogs.findMany log views and the shared deliveryLogConditions status filter. The log views show markers by design. The "success" filter needs statusCode < 400, so it already leaves markers out. I also searched for other streak or "last N rows" logic on delivery_logs and found none.
  • Migration script: the new kind filter in the migrate-automations-to-steps.ts step-id backfill is a real fix. isPseudoRow does not recognise a marker, so without the filter a marker would have been given actionIndex 0's step id. It is within criterion 3's "filters by kind". The file is under lib/scripts, which is not an owner-only path.
  • Out of scope: no markers are written for system disables, and the branch does not touch that.
  • Owner's comment: it asked for a fix round on the #38 branch. #38 is already on main (398d2142), so the bug lives on main, and fixing it from main (1dab9b7) carries out the instruction's intent. No docs or comments describe the streak in a way the change makes stale.
  • Checks: pnpm run check passes. pnpm run test passes: 186 files, 2769 tests.

loop:verdict run=review-20261103-125537-60 sha=20b19cda71ddebb5259055486a413b30fb0c7d95 verdict=merge

Merged onto main as 370911eacab818c4a9a083840b43b02d7565b90d.

loop:merged sha=370911eacab818c4a9a083840b43b02d7565b90d head=20b19cda71ddebb5259055486a413b30fb0c7d95

Sign up or Sign in to add to the discussion.