Skip to content

Say on the Routines page when nothing is there to fire them - #460

Open
zopeVaibhav wants to merge 1 commit into
CopilotKit:mainfrom
zopeVaibhav:feat/say-when-routines-have-no-worker
Open

Say on the Routines page when nothing is there to fire them#460
zopeVaibhav wants to merge 1 commit into
CopilotKit:mainfrom
zopeVaibhav:feat/say-when-routines-have-no-worker

Conversation

@zopeVaibhav

Copy link
Copy Markdown
Contributor

What this changes

A routine needs a second process to fire it, and a deployment that never started one looked exactly
like a deployment that had. Every sweep now records that it happened, and the Routines page reads
that record: somebody with standing routines and nothing sweeping is told so, instead of being shown
a page that looks correct.

  • routine_sweeps, one row keyed "routines", holding swept_at and the owner that last swept
    (server/drizzle/0029_routine_sweeps.sql, schema at server/src/db/schema/coworker.ts:133-137).
  • routineStore.recordSweep / lastSweptAt (server/src/routines/store.ts:797-814), an upsert on
    the primary key so the table stays exactly one row however many passes run.
  • offerDueRoutines records the pass before it reads what is due
    (server/src/routines/sweep.ts:142), so the Kubernetes CronJob and the laptop loop both report
    through the one funnel they already share.
  • GET /api/routines gains sweep { lastSweptAt, working } (server/src/routines/routes.ts:39-47).
  • The page draws a warning when routines stand and nothing is sweeping, naming when the last pass
    was or saying none ever happened (app/src/components/routines/routines-list.tsx:151).
  • docs/routines.md and docs/configuration.md asserted the old behaviour and now describe this one.

Why the window is fifteen minutes, and fixed

working means a sweep inside SWEEP_SILENCE_MS, which is MINIMUM_INTERVAL_MS — the floor a
routine's own schedule already has (server/src/routines/schedule.ts:4), so a longer silence is one
no routine could have wanted.

It is a constant rather than something derived from the deployment's own cadence, and the chart is
why that holds: routines.schedule defaults to */5 * * * * (charts/openbot/values.yaml:268),
chosen, in that file's own words, to sit "inside the 15-minute floor the tools enforce, so a firing
waits at most one tick". A deployment on the defaults sweeps three times inside the window. Setting a
schedule looser than fifteen minutes would make this page read as quiet between runs — but such a
schedule is already outside what the floor is built for, and the docs now say so.

Deriving the window instead (from observed sweep intervals, or from whether any routine is actually
overdue) would remove that edge, at the cost of history to read and of detection latency that follows
each routine's own cadence — a weekly routine would take a week to notice a dead worker. Fifteen
minutes notices in fifteen minutes, whatever is standing.

Where it runs

  • New state that outlives a request? Yes: one row in routine_sweeps, in Postgres. Not a
    module-level Map.
  • What happens on the second replica? Both write the same row, and the last writer wins. That
    is the intended semantic — the question the page asks is "did anything sweep recently", not
    "did this process sweep". owner records which process wrote last, so a stuck deployment still
    traces back to a claimant.
  • Anything serialised? The write is insert ... on conflict (id) do update, a single
    conditional statement on the primary key, not a check-then-write. Concurrent sweepers cannot
    produce two rows or a lost table.
  • Anything fanned out to a browser? No. The page reads the value on its existing routines
    query; no socket, no push.
  • New listener, port, or schedule? None. The heartbeat rides the sweep that already exists; a
    hundred copies of it write one row.

Boundary and audit

  • Every acting call still goes through the gateway: nothing here acts. recordSweep is
    bookkeeping on the dispatch path and takes no decision.
  • New refusals and new failures each write a row: the heartbeat is wrapped and logs
    routine-sweep-heartbeat-failed rather than writing an audit row, deliberately — it is not an
    acting call, and a broken heartbeat must not be able to stop a sweep that would otherwise have
    worked.
  • Nothing new is trusted from the client that the server can resolve itself: working is
    computed server-side from the stored timestamp; the client is handed a boolean, not a rule.

This widens one response rather than a permission: GET /api/routines now tells a signed-in caller
when the deployment last swept. It is visible only to somebody already entitled to the routines on
that page, and it carries a timestamp and a lease name, not a hostname or a secret.

One skew to know about

GET /api/routines changed shape from { routines } to { routines, sweep }, and the app defaults
a missing sweep to { lastSweptAt: null, working: false }. A new app served by an old server would
therefore show the warning when nothing is wrong. The app and the server ship from this repository
together, so there is no supported configuration where that happens — but it is the failure mode to
expect if anyone ever serves them from different builds, and the default was chosen that way on
purpose: a false warning is recoverable, a silently swallowed one is the bug this PR exists to fix.

Changelog

  • A line under Unreleased: "The Routines page says when nothing is there to run them".

Proof

Reproduced against the bug before fixing it. On main, GET /api/routines answers with keys
[ "routines" ] and there is no field a page could read to tell a dead sweeper from a live one; the
same assertion on this branch answers [ "routines", "sweep" ]. The new tests fail on main for the
same reason and pass here.

Driven in a browser against a live deployment, all four states:

  • worker sweeping, row four minutes old — no warning, routines listed normally
  • worker stopped and the row backdated 22 minutes — "Nothing is running these. The routines worker
    last checked 22 minutes ago. Until it is running again, none of these will fire."
  • row deleted, nothing has ever swept — "No routines worker has ever checked in, so none of these
    will fire. A deployment needs one running to carry them out."
  • a worker started again — the warning clears on the next tick, and the row comes back stamped with
    that process's lease name

Gates: typecheck clean across app, server and worker; lint 597 files, no warnings; format 593 files,
no diff. drizzle-kit generate reports no schema changes, so the snapshot and the schema agree.
Migration 0029_routine_sweeps applied to a live database and confirmed by information_schema:
id NOT NULL, swept_at NOT NULL, owner nullable.

Suite, measured this session against 06633a4 with the two agent-handoff-*.integration files
excluded — they fail 4-5 of their 5 tests on bare main with no branch applied, and including them
makes any comparison meaningless: branch 2544 pass / 2 fail, main 2533 pass / 2 fail, each
identical across three consecutive runs.
The two failures are the same on both sides, and neither
belongs to this change. The +11 tests are exactly this PR's: 5 in app/tests/routines-no-worker.test.ts,
4 in server/tests/routine-sweep-liveness.test.ts, and 2 added to
server/tests/routine-sweep.integration.test.ts.

Closes #459

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A deployment with no routines worker is indistinguishable from one running them

1 participant