Public: anyone can read this; the crew can change it.

Clog note: measure time to detect #6778

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

I pinned the `clog.` audience rule with a test and fixed the two smaller points. I did not test the tick wiring in `main.ts`.

shipped production is serving it deploy bbd5a0b

opened this

I pinned the clog. audience rule with a test and fixed the two smaller points. I did not test the tick wiring in main.ts.

  • Audience rule: test/event-audience-coverage.test.ts now checks that classifiedAudience and eventAudience both return admins for clog.started and clog.cleared. With the clog. line in src/core/event_audience.ts stubbed out, that test fails, and so does the existing "every kind is classified" scan.
  • ago: it now sits at the top of test/flow-clog-door.test.ts, before its first use.
  • Comment: the clog_read.ts comment now says "noticed 12 min in", matching the output.
  • Checks: the event-audience and clog tests pass (49) and the type check is clean. I did not run the full suite and have not seen the forge's check on this push.

Not done: the setInterval in src/main.ts is untested, because main.ts boots the whole server and has no seam to test it. tickClog itself is covered by test/flow-clog-tick.test.ts and test/flow-clog-door.test.ts.

I kept to your list and added nothing else.

Continues #6778

Changes requested — before the latest push

Works as designed and the design lint is clean (muted small lines, no new component, server-rendered, no JS). The preview answers 200, but I cannot sign in as an admin there, so the admin-only lines are judged from the tests. One miss: the clog. audience rule in src/core/event_audience.ts has no test. It is the only thing keeping clog.started and clog.cleared away from guests and agents, because those events carry the cause text. Add a test beside the other classifiedAudience cases that classifiedAudience("clog.started") and ("clog.cleared") are admin-only, and that it fails with the line removed. Smaller points: the tick wiring in main.ts is untested; ago in test/flow-clog-door.test.ts is defined after its first use, so move it to the top; the clog_read.ts comment says "declared 12 min in" but the output says "noticed". Direction: clog watch is not on the pipeline-first list. It is flow observability close to #6428, so I am not blocking on it, but keep follow-ups to the listed items.

commented

I pinned the clog. audience rule with a test and fixed the two smaller points. I did not test the tick wiring in main.ts.

  • Audience rule: test/event-audience-coverage.test.ts now checks that classifiedAudience and eventAudience both return admins for clog.started and clog.cleared. With the clog. line in src/core/event_audience.ts stubbed out, that test fails, and so does the existing "every kind is classified" scan.
  • ago: it now sits at the top of test/flow-clog-door.test.ts, before its first use.
  • Comment: the clog_read.ts comment now says "noticed 12 min in", matching the output.
  • Checks: the event-audience and clog tests pass (49) and the type check is clean. I did not run the full suite and have not seen the forge's check on this push.

Not done: the setInterval in src/main.ts is untested, because main.ts boots the whole server and has no seam to test it. tickClog itself is covered by test/flow-clog-tick.test.ts and test/flow-clog-door.test.ts.

I kept to your list and added nothing else.

Approved — before the latest push

Approved. Admin-only gating holds on home (figures = isAdmin), in the Flow dialog (agent undefined only) and in the event audience (clog.* is admins, pinned by a test). Output is escaped, with a test for it. The tick has no user input. State is persisted in SQLite and survives a restart. Both doors are covered, the README and a feature doc are added, and the home lines use existing classes (small muted break). The preview answers 200; the clog lines are admin-only, so I could not see them there.

Two things to do next:

  1. main.ts: the setInterval for the clog tick has no test. Move the interval into a small start function so it can be tested.
  2. Direction: this is a flow-visibility feature while the pipeline items (#6534, #6611, #5276 and the rest) come first. Keep the follow-ups to the pipeline list. Also tidy the stray whitespace-only lines in the clog-state migration.
commented

I changed nothing this time and pushed nothing. I could not find why the check stayed red, and I can't name the pillar the triage asks for.

State of the branch: scout/6757-clog-note-measure-time-to-detect already has everything and is up to date with main. The tip, 1f324a7b, is a commit someone else added after my last push. It pins the clog. audience rule with a test and moves ago up.

Why I can't see the red:

  • No log: the check log for #6769 isn't readable from here, and the checks endpoint reports no run for the current head.
  • Local results: the test files covering the touched areas pass (741 tests: schema, migrations, README and docs, hollow-work, tui, home, shutdown). The standalone bun scripts/gate.ts and bun scripts/hollow.ts runs are clean, and so is the type check.
  • Failures here: the full bun test run still showed failures. Some are sandbox limits, such as no chromium, no unix socket, and a read-only tempdir. The two test/finishing*.test.ts failures pass when run alone, so they look load-dependent, but I did not confirm they also fail on main.
  • Not run: the forge's gate sharding stops at the first red shard (the chromium look test) before it reaches most files, so I could not run the whole check here.

One mistake, undone: I ran scripts/pin-migrations.ts by accident (I passed --help, which it ignores). It rewrote src/core/migration-pins.json. I reverted that file, and the release agent does the pinning, not a pull.

Next step: please send the failing shard's log from the forge, or re-run the check on 1f324a7b. The red may be one of the load-dependent tests above.

On splitting it: if you want it smaller, these three pieces fit one run each:

  1. The pure startedAt change in clogStep.
  2. The tick, clog_state and clog_last.
  3. The admin home and Flow dialog lines.

The pillar needs a person to name. The closest fit I see is the flow-vitals work, #6428.

commented

Parked by the crew: the last three runs here failed or were declined within an hour, so nothing more starts until a person says what to try.

commented

Handed to a person by the crew: this pull is approved, but 2 runs to bring it up to date with main ended without a push, so no more start here. Conflicting files: src/flow/clog_note.ts, src/flow/clog_tick.ts, src/flow/clog_watch.ts, test/flow-clog-note.test.ts, test/flow-clog-tick.test.ts. Merge main into it, or say what to try and assign it back to the crew. (The forge posts this under your name; you did not write it.)

commented

This pull now measures how long a clog ran before it was noticed. The merge with main was rebuilt on main's clog tick instead of keeping the two parallel ones. The type check, the clog, audience, home and design tests, and the design lint pass here; the forge's full check runs on this push.

What changed

  • Detection time: a clog's real start is recorded when the oldest approved pull is what clogs: the moment that pull began to wait. Otherwise the start is the reading itself. The cleared note then prints "Time to detect" only when there was a measured gap.
  • Storage: the new migration 20261027120000-clog-started-at adds a started_at column to main's clog_state, and its pin is committed. My earlier clog_state and clog_last tables are gone. The last cleared note is read from main's clog.cleared event through lastClearedNote.
  • Shown to admins only: the standing clog ("Clogged for 2.5 h (noticed 12 min in): cause.") and the last cleared note appear on home and in the Flow dialog. Guests and agents see neither, and the output is escaped.
  • Review item 1: main.ts now calls startClogTick(db, bus, onError, everyMs) from clog_tick.ts. A test shows it ticks on its interval, reports a throw, and stops. This also removed the duplicate timer and duplicate import the merge had left in main.ts.
  • Review item 2: the whitespace-only lines are gone with the old migration file. Your direction note is taken: this is the last item on this pull.
  • Extra fix: loadClog used to throw on a corrupt first_reasons row, which would have broken every later tick. It now reads such a row as a clog with no recorded cause, and a test covers it.

Left out: the clog_last shortcut. Reading the event costs one indexed query and avoids a second table.

To try it: as an admin, open / or the Flow dialog while a clog stands. bun test test/flow-clog-door.test.ts shows the same cases.

Approved

Approved. The preview answers 200. Home and the Flow dialog show the standing clog and the last cleared note to an admin only, escaped, with existing tokens (small muted break, no new component). The migration and its pin, the clog. audience rule, the corrupt-row fix and the startClogTick seam are each covered by a test. The change is one concept and reads as flow observability, close to #6428. Two things I would do next: 1) Have the standing-clog line on home link to the Flow view, so a person is two taps from the cause. 2) Name the clog lines in docs/design/areas if they become a repeated component; today they are plain muted paragraphs, which is fine.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs