Skip to content

Monitoring 1: uptime and response time - #106

Open
hhff wants to merge 16 commits into
mainfrom
monitoring-uptime
Open

hhff wants to merge 16 commits into
mainfrom
monitoring-uptime

Conversation

@hhff

@hhff hhff commented Nov 5, 2026

Copy link
Copy Markdown
Member

Builds on #105 (deploys on merge). PR 1 of the monitoring spec: uptime and response time. No new files in the kit.

TL;DR

  • Settings. Monitors are owner settings (monitors, monitor_defaults) in a new ladder class, monitor-change, which starts at "ask with a default" (ceiling 3). merge accepts a ladder class's settings change under an answered question, a decision in the class naming the PR, or the class's rung 3. It logs the change as an act of the class.
  • URL checks. A built-in check, { "kind": "http", "url", "expect": { "status", "contains" }, "slo": { "uptime", "p95_ms" } }, records status, latency and TLS expiry.
    • Redirects fail unless asked for (follow_redirects), so a login redirect isn't "up".
    • At most 1 MB of the body is read, and only when contains is set.
  • wait polls them when one is due, before spawning anything, with a one-minute timeout. It returns once on a new outage ("production needs you"), never on a standing finding.
  • Local samples, meaningful observations.
    • Samples stay in run/monitors/<name>/.
    • Observations go on monitor:<name>:
      • down: severity 4, after down_after failed checks
      • up: refs the outage
      • slow: once per breach, when the last hour's p95, from at least 20 good checks, beats the target 3 checks running
      • a daily UTC rollup: uptime, p50, p95, outages; missed days are caught up
    • State is seeded from the log after a lost run/ or a fleet move, so an outage isn't doubled.
  • Findings:
    • monitor-down: medium, so wait doesn't loop
    • cert-expiring: high at 3 days
    • slo-breach-week
  • Dashboard:
    • a Monitoring nav section: a card per monitor with uptime 24h and 7d, p50 and p95, the certificate, and a latency chart against the target; a feed of what the monitors noticed
    • a red banner on every page while something is down
    • a #monitor/<name> sidepeek: settings and state, 24h and 7d charts, failed checks, history
  • Commands: monitor status|test|poll|series.

Review

Fable reviewed both PRs. Fixed:

  • a merge's --decision wasn't checked like an act's: corrected, lapsed, or naming another target
  • settings changes in a ladder class weren't logged as the class's act alongside an owner answer, or under a decision
  • "p95" was the max of the last 20 checks, so one slow answer flagged the site slow
  • wait could hang on a stuck poll, and spawned a poll every cycle
  • concurrent polls raced
  • whole bodies were read
  • a redirect counted as up
  • a monitor name could write outside run/
  • samples were parsed several times a refresh
  • docs disagreed with code

Proxies are documented (wait sets NODE_USE_ENV_PROXY=1).

Tests

A local server in a child process walks one monitor through:

  • up
  • one failure (not an outage)
  • down (one alert; a medium finding)
  • up (refs the outage)
  • a few slow answers among fast ones (not slow)
  • a slow spell (one slow)
  • a rollup
  • --due, the series, dashboard data and peek
  • wait waking on a new outage
  • monitor-change at rung 2

The owner-settings test merges a monitors change under a monitor-change decision, and refuses a decision for another PR.


Generated by Claude Code

claude added 12 commits November 4, 2026 23:37
… is a release and needs release-production's authority

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5z3hZtpGaQUGsTaAMQrAg
…, observed on change, on the dashboard

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5z3hZtpGaQUGsTaAMQrAg
… with their own width and colors

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5z3hZtpGaQUGsTaAMQrAg
…corrected, not lapsed, naming this target); the release act carries it, not the merge

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5z3hZtpGaQUGsTaAMQrAg
… class's act, under its decision or on its rung

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5z3hZtpGaQUGsTaAMQrAg
…there are 20 checks, one poll at a time, bounded body, redirects fail, state seeded from the log, rollups for missed days, safe names, samples read once, wait spawns only when due with a timeout)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5z3hZtpGaQUGsTaAMQrAg
@hhff

hhff commented Nov 5, 2026

Copy link
Copy Markdown
Member Author

Adversarial review (upstream loop): #105 CHANGES, #106 CHANGES

A fresh reviewer reproduced each blocker on Node 18.20.4, in a throwaway project with forge git and a local bare origin. Line numbers are in devteam.mjs on monitoring-uptime.

Blockers

  1. (Monitoring 1: uptime and response time #106) The supervisor can change any owner setting with no owner involved (:2388, :2394).
    • merge accepts --approved if the question's class is any one of the classes the PR opens. A monitors change adds monitor-change, which is a ladder class, not owner_only, so answer --from-chat accepts it.
    • Reproduced: one chore changed monitors, agent_command, gates: {} and secret_commands. The supervisor asked a monitor-change question, answered it --from-chat, and merge --chore … --approved <q> merged all four keys to main.
    • Fix: every changed owner_only class needs its own answer (allow several --approved). A ladder class's answer never covers an owner_only key. Settings PRs refuse channel: 'chat' answers, or monitor-change joins the reserved set.
  2. (Deploys on merge: a merge that deploys is a release #105, Monitoring 1: uptime and response time #106) A hand-written decision line can reuse an owner answer that was already used (decisionAuthorityGap :3749, used by merge :2388, :2411).
    • It checks only that approved_by is an answered question in the class: not that it's unused, nor that it was about this ticket or its approves. CLI decide refuses reuse; a hand-written line skips that.
    • Reproduced: a release answer for ticket-A, reused by an appended decision with refs: ["chore-app"], passed merge --decision. log-check was silent.
    • Note: this function is on main too. The upstream loop is fixing it there in its own PR (the earliest decision or act citing an answer wins; the question's ticket/approves must overlap the decision's refs). Merge main in once it lands.

Should-fix

  1. (both) One decision can be carried out more than once (a monitor-change decision merged twice on two branches). Refuse a decision when an act already carries it for the same target.
  2. (Monitoring 1: uptime and response time #106) A monitor's name can be overridden by m.name or monitor_defaults.name (:6633). Polls then write outside run/; reproduced with ../../../../../ESCAPED-MON. Spread m first, set name last, and refuse a name field.
  3. (Monitoring 1: uptime and response time #106) A new monitor whose site is already down never goes down (:6735, :6731). While unknown, each poll re-seeds fails: 0. Seed only when state.json is missing.
  4. (Monitoring 1: uptime and response time #106) Redirects count as up (:6672; r.status < 400 passes 302), which contradicts the PR and supervisor.md.
  5. (Monitoring 1: uptime and response time #106) Secrets in monitor URLs reach samples, state, the monitor status output, the down observation, the monitor-down finding and wait's ALERT line unredacted (userinfo, and ?token=). Redact userinfo and the query everywhere a URL is shown; refuse credentials in settings.

Nits

  1. "Logged as an act of the class" doesn't hold under --approved (:2396).
  2. NODE_USE_ENV_PROXY does nothing on Node 18, 20 or 22; tlsDaysLeft ignores proxies and sets servername for IP hosts; a poll killed at 60 s leaves its lock for 2 min.
  3. (Deploys on merge: a merge that deploys is a release #105) Deploy-relevant files outside paths (Procfile, package.json, Dockerfile); {paths} without on: "merge" is silently a no-op; plan-undefined in targets.

What held

  • --no-renames catches renames out of deployed paths, and --force, --chore log and plan merges don't bypass the release check.
  • Correction, lapse-after-rung-drop and target checks work, and a project without release: "merge" is unaffected.
  • Rungs rise only on owner-answered trust questions.
  • Monitors run only kind: "http", with no headers or commands, and only in wait / monitor poll.
  • Response bodies never reach prompts or the log.
  • None of the shipped tests cover findings 1–7.

hhff and others added 2 commits November 5, 2026 23:41
Conflicts resolved keeping main's rules: ownerAnswer (matched, #113),
question_hash (#116), owner_rules (#115), per-entry link binding and
links in home only (#108), one-pass linkify (#107).

Review fixes:
- merge: each changed owner settings class needs its own settling; an
  owner_only class only by the owner's own answer in it (never chat) or
  their standing rule; --approved takes several answers. A ladder-class
  answer (monitor-change) never covers an owner_only key or a kit file.
- merge: a decision is carried out once; settings acts name what settled
  them (answer, rule, decision or rung), and an act a merge logged is
  never a pending landing for a later merge.
- monitors: name set last from the key, a name field refused; credentials
  in a monitor url refused (validate, check-pr/merge); userinfo and query
  never shown (status, alerts, observations, findings, dashboard,
  settings); 2xx only is up; state seeded from the log only when
  state.json is missing; a dead poll's lock is stale at once; no SNI for
  IP hosts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…wn; secret_commands is the changed governance-edit key

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@hhff

hhff commented Nov 6, 2026 •

Copy link
Copy Markdown
Member Author

Review findings, one by one (upstream loop)

I merged origin/main into monitoring-uptime as a merge commit, with no rebase or force-push. Conflicts were resolved in favour of main's authority rules: ownerAnswer must be matched (#113), answers carry question_hash (#116), owner_rules (#115), per-entry link binding and links only in home (#108), and one-pass linkify (#107, with monitor: added to it).

Blockers

  1. A ladder-class or chat-relayed answer approved owner settings. Fixed, following the owner's ruling to keep one answer for the bundle.
    • The hole. Main's own checks (Decisions: an owner's answer approves one call, about what it was asked about #113, gate-change can be settled by the owner's own standing rule #115) didn't stop the Monitoring 1: uptime and response time #106 repro: one monitor-change answer, relayed from chat, merged monitors, agent_command, gates and secret_commands.
    • What merge does now:
      • One owner answer covers the whole bundle, owner_only keys of other classes included. It must be in any one of the owner_only classes the PR changes, and it must be the owner's own: an answer with channel: 'chat' is refused.
      • Where owner_rules allows, the owner's standing rules cover it instead, but only when every changed owner_only class has one (as on main).
      • A ladder-class answer (monitor-change), a decision or rung 3 covers only that ladder class's keys, never an owner_only key.
      • --approved still accepts several answers.
      • Repo-link entries stay bound to what the bundle's question showed (Tickets in other repos: one fleet, one log and one graph across sibling repos #108).
    • Kit files (separate commit 9e90d7f, the owner is deciding). A chore that touches the kit needs the owner's own governance-edit, process-edit or gate-change answer, never a ladder-class answer or one relayed from chat. Before this, any answered question approved it. Reverting that commit removes the rule cleanly.
    • Tests (in "a PR of any kind that changes owner settings…"):
      • a ladder-class (priority-change) answer is refused for gates and upstream_repo;
      • a single governance-edit answer covers both, and passing two answers is accepted;
      • the reviewer's repro is refused with "a ladder class's answer never covers an owner setting", and monitors is still settled by its own answer;
      • an owner_only answer carrying channel: chat (hand-written, since answer --from-chat refuses it) is refused;
      • the kit-file check (commit 9e90d7f).
  2. Decision reuse. Already on main (Decisions: an owner's answer approves one call, about what it was asked about #113 answerUseGap), now covered by a test.
    • A hand-written copy of a decision that cites an answer an earlier decision already used is refused by merge --decision ("already approved a decision").

Should-fix

  1. A decision carried out twice. Fixed.
    • merge --decision refuses a decision that any act already carries out ("was already carried out … decide again").
    • Settings acts now name the merge act they belong to, and such an act never counts as a landing still waiting, so a reused chore slug can't land a second time on the same answer.
    • Test: an act under the decision, then the merge is refused.
  2. A monitor's name could be overridden. Fixed.
    • The name is set last, from the key.
    • validate, check-pr and merge refuse a name field in a monitor or in monitor_defaults, and a refused entry is never polled.
    • Test: ../../ESCAPED-MON in both places; nothing is written outside run/monitors.
  3. fails was re-seeded while unknown. Fixed. State is seeded from the log only when state.json is missing. Test: a new monitor whose site is already down goes down on its second failed check.
  4. Redirects counted as up. Fixed. Only a 2xx is up, unless expect.status says otherwise. Test: a 302 to a login page fails with "HTTP 302".
  5. Secrets in monitor URLs. Fixed.
    • validate, check-pr and merge refuse userinfo or a secret-named parameter in a monitor url, as the links registry does.
    • monitorShown leaves out userinfo and the whole query and fragment in every place a URL is shown: monitor status, wait's ALERT line, the down observation, monitor-down and cert-expiring, dashboard data, settings, and fetch error text (and so samples and state.json).
    • Test: a ?q=s3cret monitor's alert, status, observations, findings, dashboard data, settings and state.json never hold it.

Nits

  1. Fixed. Each changed settings class is logged as its own act, with what settled it: approved_by, the decision, or the rung.
  2. Partly fixed.
    • Fixed: tlsDaysLeft sends no SNI for an IP host, and a dead poll's lock is stale at once (pid check). The docs now say NODE_USE_ENV_PROXY needs Node 24 or later, and that the certificate check doesn't use the proxy.
    • Deferred: proxy support on older Node.
  3. Not done here. These are Deploys on merge: a merge that deploys is a release #105's (deploys-on-merge) and are left for that PR.

Docs: quality.md §4, supervisor.md (Monitoring and Owner settings) and the merge usage and HELP text.

Tests: the full test/kit.test.mjs on Node 24.14.1, 131/131 pass at the pushed head. The updated #106 is merged up through #109–#112, and #112's head passes 135/135.

hhff and others added 2 commits November 6, 2026 02:18
The owner's own answer (never relayed from chat) in any one of the
owner_only classes a PR changes approves every owner setting it changes;
or their standing rules where owner_rules allows. A ladder-class answer,
a decision or rung 3 covers only that ladder class's keys, never an
owner_only key. Several --approved are still accepted. Repo link
entries stay bound to what the bundle's question showed.

The kit-files tightening is taken out here and comes back in its own
commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A chore that changes files outside project/ needs the owner's own
answer of class governance-edit, process-edit or gate-change: never a
ladder class's answer (monitor-change) or one relayed from chat. Before,
any answered question approved it. Pending the owner's decision; revert
this commit to drop it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

2 participants