From d195235b45b5796c131c21e042a3920f2f3121c9 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 30 Jul 2026 17:04:51 -0400 Subject: [PATCH 1/7] fix(loop-lane): gate the inlined telemetry upsert's body before it writes Every lane that inlines the `gh api` telemetry upsert -- babysit-loop, work-loop, attend-queue -- writes `$BODY_FILE` whole, with no assertion that what it composed is usable. `claude-ops`'s `telemetry-upsert.sh` refuses a degraded body before it writes; an installed plugin cannot invoke a sibling plugin's script, so the three inlined copies inherited none of that. Each block now opens with a pre-write gate: a `$BODY_FILE` that is empty, begins with a literal `@`, falls under the sentinel-plus-16-byte floor, or is not sentinel-prefixed is refused with a visible notice and the cycle skips the upsert fail-closed -- no POST, no PATCH. Co-Authored-By: Claude Opus 5 (1M context) --- docs/conventions/loop-lane/README.md | 9 ++++++--- plugins/claude-ops/.claude-plugin/plugin.json | 2 +- plugins/claude-ops/CHANGELOG.md | 14 +++++++++++++ plugins/claude-ops/skills/lanes/SKILL.md | 7 +++++-- .../source-control/.claude-plugin/plugin.json | 2 +- plugins/source-control/CHANGELOG.md | 20 +++++++++++++++++++ .../reference/telemetry-upsert.md | 13 +++++++++++- plugins/work-items/.claude-plugin/plugin.json | 2 +- plugins/work-items/CHANGELOG.md | 19 ++++++++++++++++++ .../work-items/skills/attend-queue/SKILL.md | 13 +++++++++++- plugins/work-items/skills/work-loop/SKILL.md | 13 +++++++++++- 11 files changed, 103 insertions(+), 11 deletions(-) diff --git a/docs/conventions/loop-lane/README.md b/docs/conventions/loop-lane/README.md index 8ec3fe93c..661f7be8f 100644 --- a/docs/conventions/loop-lane/README.md +++ b/docs/conventions/loop-lane/README.md @@ -423,9 +423,12 @@ identified by a machine sentinel marker and **edited in place** every cycle — `claude-ops`'s `telemetry-upsert.sh` is the interim home of this contract and a compatible reader (`morning-brief` reads the same surface); an installed plugin cannot invoke a sibling plugin's script, so each lane **inlines** the small `gh api` upsert and the coupling to `claude-ops` stays -one-directional. An inlined upsert carries none of the wrapper's body checks, so it is bound by the -`@path`-as-body rule in [`claude-ops` lanes](../../../plugins/claude-ops/skills/lanes/SKILL.md), -section "Never pass a body as an `@path` string". +one-directional. An inlined upsert is bound by the `@path`-as-body rule in +[`claude-ops` lanes](../../../plugins/claude-ops/skills/lanes/SKILL.md), section "Never pass a body as +an `@path` string", and encodes that rule as a **pre-write body gate** in its own block (#943): a body +that is empty, a literal `@path`, under the sentinel-plus-16-byte floor, or not sentinel-prefixed is +refused before any API call, and the cycle skips the upsert fail-closed. The wrapper's post-write +read-back is deliberately not replicated inline — an inlined upsert carries the pre-write half only. **Durable loop state.** Conversation context is lossy across compaction, so a lane persists its adaptive-cap streak counter, its rate-limit-warning latch, its consecutive-no-progress counter, and diff --git a/plugins/claude-ops/.claude-plugin/plugin.json b/plugins/claude-ops/.claude-plugin/plugin.json index 8c1425108..1800c1e96 100644 --- a/plugins/claude-ops/.claude-plugin/plugin.json +++ b/plugins/claude-ops/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "claude-ops", - "version": "0.24.1", + "version": "0.24.2", "description": "Claude Code operations toolkit. Seven skills: observability (read locally captured telemetry — OTEL store, collector, hook-event JSONL, ccusage — with trend reports and store pruning), known-issues (search known Claude product GitHub bugs, check service health, maintain a persistent tracked-issue registry), changelog (ingest Claude Code changelog entries and integrate them into the current repo), plugins (bring a machine's plugin fleet current on demand — marketplace refresh, effective-scope updates including in-repo project/local installs, new-plugin install per policy, scope-divergence detection and explicit convergence), morning-brief (read-only gh-based operator morning view — queue-label counts, merge-ready PRs, parked decisions with their RECOMMENDED lines, and loop-lane telemetry freshness), lanes (start/restart/stop/status loop lanes as named background Claude Code sessions seeded from canonical prompt files, with per-lane model/effort, a repo-pull + marketplace-refresh launch step, and a consume-restarts action — an OS-schedulable reader that relaunches stopped lanes whose telemetry carries a restart_request), and a re-runnable setup action that settles where the known-issues registry lives. Plus a family of seven advisory *-audit telemetry-emitter hooks (API errors, config changes, instruction loads, permission denials, pre-compaction, skill usage, tool failures) that emit the shared hook-telemetry envelope, and a reference sink that maps envelopes into the hook-events.jsonl the observability skill reads.", "author": { "name": "Melodic Software", diff --git a/plugins/claude-ops/CHANGELOG.md b/plugins/claude-ops/CHANGELOG.md index 972013437..200ad9464 100644 --- a/plugins/claude-ops/CHANGELOG.md +++ b/plugins/claude-ops/CHANGELOG.md @@ -3,6 +3,20 @@ All notable changes to the `claude-ops` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.24.2] + +### Changed + +- **The `@path`-as-body rule now records that an inlined upsert enforces it mechanically, not on + trust (#943).** The rule's closing paragraph claimed the prose was "the only thing standing + between" an inlined upsert and a silent observability fail-open. That is no longer true: every + lane that inlines the `gh api` upsert — `source-control:babysit-loop`, `work-items:work-loop`, + `work-items:attend-queue` — now carries a pre-write body gate in its own block, refusing a body + that is empty, a literal `@path`, under a byte floor, or not sentinel-prefixed before any API + call. The paragraph states the split explicitly: an inline gate replicates the wrapper's pre-write + half only, and the post-write read-back stays in `telemetry-upsert.sh`, so a reader of either + surface knows which guarantees travel with which. + ## [0.24.1] ### Fixed diff --git a/plugins/claude-ops/skills/lanes/SKILL.md b/plugins/claude-ops/skills/lanes/SKILL.md index a09cd9a18..2dd9d22ab 100644 --- a/plugins/claude-ops/skills/lanes/SKILL.md +++ b/plugins/claude-ops/skills/lanes/SKILL.md @@ -288,8 +288,11 @@ command's own `--help` (gh 2.95.0); `gh api` has no `--body-file` flag at all. The failure is invisible from the outside (#943): the comment's timestamp still moves, so the telemetry surface looks **fresh** while carrying no data, and a freshness check passes over a blind lane. `telemetry-upsert.sh` refuses such a -body before it writes anything; an inlined upsert has no such gate, so this rule -is the only thing standing between it and a silent observability fail-open. +body before it writes anything, and an inlined upsert carries the same refusal as +a pre-write gate in its own block (empty, leading `@`, under a byte floor, or not +sentinel-prefixed → skip the cycle fail-closed, no API call). An inline gate +replicates the pre-write half only; the post-write read-back stays here in the +wrapper. ## Cross-references diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index bdeeaf803..4c97a0336 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "source-control", - "version": "0.42.1", + "version": "0.42.2", "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop — safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /babysit-loop (the loop-lane merge lane: a standing or drain loop that invokes babysit-prs per cycle, configured through repo-scoped babysit_loop_* keys on the layered source-control.md seam, with merge authority human-only until the target repo's tracked config adopts the lane, a gate-proven C2-mechanical baseline once adopted, and standing merge-rung raises binding from the team-tracked layer only — with one named exception, where an invocation line explicitly typing both the autopilot tier keyword and the dedicated raise argument --merge c3-this-run widens that single invocation's merge authority up to C3 behind a fresh independent frontier-tier resolver, while C4-structural and C5-untrusted-provenance stay unconditionally human-merge), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention merged across its config layers and the babysit-prs config, or apply — interview the repo and write the convention config to a chosen layer), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable via a source-control.md config written by a re-runnable setup skill, layered across a ~/.claude user-global file, the tracked team file, and a gitignored .claude/source-control.local.md personal overlay merged per key; Conventional Commits is the default when no convention is declared.", "author": { "name": "Melodic Software", diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index c8f4cccb9..adb143cc0 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -3,6 +3,26 @@ All notable changes to the `source-control` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.42.2] + +### Fixed + +- **`babysit-loop`'s inlined telemetry upsert refuses a degraded body before it writes one (#943).** + The lane's telemetry comment was observed carrying a literal `@C:/…/telemetry_combined.txt` as its + entire body across three sessions: `gh` expands a leading `@` only for `--body-file` / `-F + field=@file`, so an `@path` interpolated into a body value is sent as text. Nothing about that + failure is visible from outside — the comment's `updatedAt` still moves, so `morning-brief`'s + freshness check passes over a lane that reported nothing. `claude-ops`'s `telemetry-upsert.sh` + already refuses such a body, but an installed plugin cannot invoke a sibling plugin's script, so + this lane inlines its own upsert and inherited none of that protection. The inlined block now opens + with a pre-write gate: a `$BODY_FILE` that is empty, begins with a literal `@`, falls under the + sentinel-plus-16-byte floor, or is not sentinel-prefixed is refused with a visible notice and the + cycle skips the upsert fail-closed — no POST, no PATCH. Refusing leaves the comment **stale**, + which the freshness check does catch, instead of fresh-but-blind, which it cannot. The prefix + assertion compares bytes rather than a whole first line, so a CRLF-terminated body is not + false-rejected. Scope is pre-write only; the wrapper's post-write read-back is deliberately not + replicated into a prose block that ships without a test harness. + ## [0.42.1] ### Fixed diff --git a/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md b/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md index c203b69b5..2e5eb4fed 100644 --- a/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md +++ b/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md @@ -12,7 +12,11 @@ MARKER="source-control:babysit-loop" SENT="" # first line of $BODY_FILE LOOKUP() { gh api --paginate "repos/$REPO/issues/$ISSUE/comments" \ --jq ".[] | select(.body | startswith(\"$SENT\")) | .id"; } -if ! LIST=$(LOOKUP); then +if [ ! -s "$BODY_FILE" ] || [ "$(head -c 1 "$BODY_FILE")" = "@" ] || + [ "$(wc -c <"$BODY_FILE")" -lt $((${#SENT} + 16)) ] || + [ "$(head -c ${#SENT} "$BODY_FILE")" != "$SENT" ]; then + echo "telemetry: body rejected before any write - empty, a literal @path, under the sentinel+16-byte floor, or not sentinel-prefixed; skipping upsert this cycle (fail closed)" >&2 +elif ! LIST=$(LOOKUP); then echo "telemetry: comment lookup failed; skipping upsert this cycle (fail closed)" >&2 else if [ -z "$LIST" ]; then @@ -30,6 +34,13 @@ else fi ``` +**Pre-write body gate (encoded above, #943).** The leading assertions run before any API call — the +mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes skill ("Never pass a body +as an `@path` string"), which an inlined upsert has no wrapper to enforce for it. A degraded body +that reaches the comment still moves its timestamp, so the telemetry surface looks **fresh** while +carrying no data; refusing the write leaves the comment stale instead, which the freshness check does +catch. Pre-write only — the post-write read-back stays in `claude-ops`'s `telemetry-upsert.sh`. + **Creation race reconcile (encoded above).** Two sessions racing the first-ever upsert can both see an empty lookup and both POST, forking the singleton. The upsert converges every cycle duplicates are visible: the LOWEST comment id is canonical (numeric sort, deterministic for diff --git a/plugins/work-items/.claude-plugin/plugin.json b/plugins/work-items/.claude-plugin/plugin.json index 6b0fa7fc1..10adc79bf 100644 --- a/plugins/work-items/.claude-plugin/plugin.json +++ b/plugins/work-items/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "work-items", - "version": "0.30.3", + "version": "0.30.4", "description": "Manages development work items through a provider-neutral tracker seam that ships with the plugin (bundled dispatcher plus github and local-markdown adapters; seam plugin-dir canonical, adapters consumer-local-first): dashboard, taxonomy-labeled creation, a race-safe assignee-plus-lease claim protocol, recurring-schedule checks, TODO scanning, stale-lease auditing, plan decomposition into vertical-slice items, raw-intake triage (issues and unsolicited PRs through raw, verified, briefed, autonomous-eligible states), plus the two work-items loop lanes of the loop-lane convention: a self-paced autonomous work-loop drain (work-class admission gate, adaptive item cap, PR-only) and an attended attend-queue escalation lane. The re-runnable setup skill binds the provider (.work-item-tracker.json), seeds the recurring-schedule seam (.github/recurring-schedule.json), and remaps canonical role labels.", "author": { "name": "Melodic Software", diff --git a/plugins/work-items/CHANGELOG.md b/plugins/work-items/CHANGELOG.md index 01bbce2f1..d3e91c4d8 100644 --- a/plugins/work-items/CHANGELOG.md +++ b/plugins/work-items/CHANGELOG.md @@ -3,6 +3,25 @@ All notable changes to the `work-items` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.30.4] + +### Fixed + +- **`work-loop` and `attend-queue`'s inlined telemetry upserts refuse a degraded body before they + write one (#943).** Both lanes inline the same `gh api` upsert the babysit lane does — an installed + plugin cannot invoke `claude-ops`'s `telemetry-upsert.sh` — and so inherited none of that wrapper's + body checks. The defect that surfaced on the babysit lane is a property of the shared upsert shape, + not of one lane: an `@path` interpolated into a body value is sent as literal text (`gh` expands a + leading `@` only for `--body-file` / `-F field=@file`), and the resulting comment still moves its + `updatedAt`, so a freshness check passes over a lane carrying no data. Both blocks now open with a + pre-write gate: a `$BODY_FILE` that is empty, begins with a literal `@`, falls under the + sentinel-plus-16-byte floor, or is not sentinel-prefixed is refused with a visible notice and the + cycle skips the upsert fail-closed — no POST, no PATCH. Refusing leaves the comment **stale**, + which the freshness check does catch, instead of fresh-but-blind, which it cannot. The prefix + assertion compares bytes rather than a whole first line, so a CRLF-terminated body is not + false-rejected. Scope is pre-write only; the wrapper's post-write read-back is deliberately not + replicated into a prose block that ships without a test harness. + ## [0.30.3] ### Added diff --git a/plugins/work-items/skills/attend-queue/SKILL.md b/plugins/work-items/skills/attend-queue/SKILL.md index 74113388f..26131e128 100644 --- a/plugins/work-items/skills/attend-queue/SKILL.md +++ b/plugins/work-items/skills/attend-queue/SKILL.md @@ -122,7 +122,11 @@ MARKER="work-items:attend-queue" SENT="" # first line of $BODY_FILE LOOKUP() { gh api --paginate "repos/$REPO/issues/$ISSUE/comments" \ --jq ".[] | select(.body | startswith(\"$SENT\")) | .id"; } -if ! LIST=$(LOOKUP); then +if [ ! -s "$BODY_FILE" ] || [ "$(head -c 1 "$BODY_FILE")" = "@" ] || + [ "$(wc -c <"$BODY_FILE")" -lt $((${#SENT} + 16)) ] || + [ "$(head -c ${#SENT} "$BODY_FILE")" != "$SENT" ]; then + echo "telemetry: body rejected before any write - empty, a literal @path, under the sentinel+16-byte floor, or not sentinel-prefixed; skipping upsert this cycle (fail closed)" >&2 +elif ! LIST=$(LOOKUP); then echo "telemetry: comment lookup failed; skipping upsert this cycle (fail closed)" >&2 else if [ -z "$LIST" ]; then @@ -140,6 +144,13 @@ else fi ``` +**Pre-write body gate (encoded above, #943).** The leading assertions run before any API call — the +mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes skill ("Never pass a body +as an `@path` string"), which an inlined upsert has no wrapper to enforce for it. A degraded body +that reaches the comment still moves its timestamp, so the telemetry surface looks **fresh** while +carrying no data; refusing the write leaves the comment stale instead, which the freshness check does +catch. Pre-write only — the post-write read-back stays in `claude-ops`'s `telemetry-upsert.sh`. + **Creation race reconcile (encoded above).** Two sessions racing the first-ever upsert can both see an empty lookup and both POST, forking the singleton. The upsert converges every cycle duplicates are visible: the LOWEST comment id is canonical (numeric sort, deterministic for diff --git a/plugins/work-items/skills/work-loop/SKILL.md b/plugins/work-items/skills/work-loop/SKILL.md index 63531a8c0..c723e3866 100644 --- a/plugins/work-items/skills/work-loop/SKILL.md +++ b/plugins/work-items/skills/work-loop/SKILL.md @@ -77,7 +77,11 @@ MARKER="work-items:work-loop" SENT="" # first line of $BODY_FILE LOOKUP() { gh api --paginate "repos/$REPO/issues/$ISSUE/comments" \ --jq ".[] | select(.body | startswith(\"$SENT\")) | .id"; } -if ! LIST=$(LOOKUP); then +if [ ! -s "$BODY_FILE" ] || [ "$(head -c 1 "$BODY_FILE")" = "@" ] || + [ "$(wc -c <"$BODY_FILE")" -lt $((${#SENT} + 16)) ] || + [ "$(head -c ${#SENT} "$BODY_FILE")" != "$SENT" ]; then + echo "telemetry: body rejected before any write - empty, a literal @path, under the sentinel+16-byte floor, or not sentinel-prefixed; skipping upsert this cycle (fail closed)" >&2 +elif ! LIST=$(LOOKUP); then echo "telemetry: comment lookup failed; skipping upsert this cycle (fail closed)" >&2 else if [ -z "$LIST" ]; then @@ -95,6 +99,13 @@ else fi ``` +**Pre-write body gate (encoded above, #943).** The leading assertions run before any API call — the +mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes skill ("Never pass a body +as an `@path` string"), which an inlined upsert has no wrapper to enforce for it. A degraded body +that reaches the comment still moves its timestamp, so the telemetry surface looks **fresh** while +carrying no data; refusing the write leaves the comment stale instead, which the freshness check does +catch. Pre-write only — the post-write read-back stays in `claude-ops`'s `telemetry-upsert.sh`. + **Creation race reconcile (encoded above).** Two sessions racing the first-ever upsert can both see an empty lookup and both POST, forking the singleton. The upsert converges every cycle duplicates are visible: the LOWEST comment id is canonical (numeric sort, deterministic for From e3ab4eb421ab1ae3bd182403098455e3ad2c2a27 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:33:12 -0400 Subject: [PATCH 2/7] fix(loop-lane): verify the telemetry write landed, not just the file sent An independent review found the pre-write gate structurally blind to the failure #943 actually reports. The observed defect is a body-VALUE flag (`-f body=@FILE` instead of `-F body=@FILE`): the composed file is correct and the gate passes it, then `gh` transmits the literal path. Checking the file can never catch that. Each block now also re-reads the comment after the write and reports the cycle UNREPORTED unless the persisted body still opens with the sentinel and clears the payload floor. Three corrections in the same pass: - The byte floor was measured over the whole file; the wrapper's MIN_BODY_BYTES=16 is measured on the payload BELOW the sentinel. The two now agree exactly. - The `$BODY_FILE` sentinel-first-line contract was implicit in a trailing comment. A body composed without it is not merely rejected -- the lookup would never find it again and the next cycle would post a second comment. Stated in prose. - The prose and CHANGELOGs claimed `morning-brief`'s freshness check is what a degraded body deceives. Verified against its `print_telemetry`: it parses `lane:`/`last-cycle:` out of the BODY, so a degraded comment makes the lane vanish from the report rather than look healthy. The deceived check is a timestamp-based delivery check reading `updatedAt`. Attributed correctly. Co-Authored-By: Claude Opus 5 (1M context) --- docs/conventions/loop-lane/README.md | 13 ++++-- plugins/claude-ops/CHANGELOG.md | 22 +++++---- plugins/claude-ops/skills/lanes/SKILL.md | 20 ++++++--- plugins/source-control/CHANGELOG.md | 28 ++++++------ .../reference/telemetry-upsert.md | 44 +++++++++++++----- plugins/work-items/CHANGELOG.md | 27 ++++++----- .../work-items/skills/attend-queue/SKILL.md | 44 +++++++++++++----- plugins/work-items/skills/work-loop/SKILL.md | 45 ++++++++++++++----- 8 files changed, 166 insertions(+), 77 deletions(-) diff --git a/docs/conventions/loop-lane/README.md b/docs/conventions/loop-lane/README.md index 661f7be8f..65b2ee69f 100644 --- a/docs/conventions/loop-lane/README.md +++ b/docs/conventions/loop-lane/README.md @@ -425,10 +425,15 @@ identified by a machine sentinel marker and **edited in place** every cycle — script, so each lane **inlines** the small `gh api` upsert and the coupling to `claude-ops` stays one-directional. An inlined upsert is bound by the `@path`-as-body rule in [`claude-ops` lanes](../../../plugins/claude-ops/skills/lanes/SKILL.md), section "Never pass a body as -an `@path` string", and encodes that rule as a **pre-write body gate** in its own block (#943): a body -that is empty, a literal `@path`, under the sentinel-plus-16-byte floor, or not sentinel-prefixed is -refused before any API call, and the cycle skips the upsert fail-closed. The wrapper's post-write -read-back is deliberately not replicated inline — an inlined upsert carries the pre-write half only. +an `@path` string", and encodes that rule mechanically in its own block (#943) as two halves. A +**pre-write gate** refuses a body that is empty, a literal `@path`, not sentinel-prefixed, or under a +16-byte payload floor, before any API call. A **post-write read-back** then re-reads what landed, +because the pre-write half is structurally blind to the failure that produced #943 in the first +place: a well-composed file passed through a body-VALUE flag (`-f body=@FILE` rather than `-F +body=@FILE`), where `gh` transmits the literal path and the file was never at fault. A failed +verification means the cycle did not report — record it in durable state, since stderr does not +survive the session. Not replicated inline: the wrapper's 64 KiB cap and its body-file containment +checks. **Durable loop state.** Conversation context is lossy across compaction, so a lane persists its adaptive-cap streak counter, its rate-limit-warning latch, its consecutive-no-progress counter, and diff --git a/plugins/claude-ops/CHANGELOG.md b/plugins/claude-ops/CHANGELOG.md index 200ad9464..f8c34df7b 100644 --- a/plugins/claude-ops/CHANGELOG.md +++ b/plugins/claude-ops/CHANGELOG.md @@ -8,14 +8,20 @@ All notable changes to the `claude-ops` plugin are documented here. Format follo ### Changed - **The `@path`-as-body rule now records that an inlined upsert enforces it mechanically, not on - trust (#943).** The rule's closing paragraph claimed the prose was "the only thing standing - between" an inlined upsert and a silent observability fail-open. That is no longer true: every - lane that inlines the `gh api` upsert — `source-control:babysit-loop`, `work-items:work-loop`, - `work-items:attend-queue` — now carries a pre-write body gate in its own block, refusing a body - that is empty, a literal `@path`, under a byte floor, or not sentinel-prefixed before any API - call. The paragraph states the split explicitly: an inline gate replicates the wrapper's pre-write - half only, and the post-write read-back stays in `telemetry-upsert.sh`, so a reader of either - surface knows which guarantees travel with which. + trust — and corrects which consumer the failure actually deceives (#943).** The rule's closing + paragraph claimed the prose was "the only thing standing between" an inlined upsert and a silent + observability fail-open. That is no longer true: every lane that inlines the `gh api` upsert — + `source-control:babysit-loop`, `work-items:work-loop`, `work-items:attend-queue` — now carries both + a pre-write body gate and a post-write read-back in its own block. The paragraph states which + guarantees travel inline (both halves) and which do not (the 64 KiB cap, the body-file containment + checks). +- **Corrected: `morning-brief` is not the check a degraded telemetry body deceives.** The rule said a + freshness check "passes over a blind lane". Verified against `morning-brief.sh`'s `print_telemetry`: + it parses `lane:` and `last-cycle:` out of the comment BODY, so an `@path` body carries no `lane:` + field and the lane disappears from the report entirely rather than reading as healthy. The check + that is genuinely fooled is a timestamp-based delivery check reading `updatedAt`, which moves on + every successful write regardless of content. The rule now attributes the failure correctly instead + of naming a sibling reader that would in fact surface it. ## [0.24.1] diff --git a/plugins/claude-ops/skills/lanes/SKILL.md b/plugins/claude-ops/skills/lanes/SKILL.md index 2dd9d22ab..15f56f3c7 100644 --- a/plugins/claude-ops/skills/lanes/SKILL.md +++ b/plugins/claude-ops/skills/lanes/SKILL.md @@ -286,13 +286,19 @@ takes `gh issue comment --body-file`, or `gh api -F`/`--field key=@path` — per command's own `--help` (gh 2.95.0); `gh api` has no `--body-file` flag at all. The failure is invisible from the outside (#943): the comment's timestamp still -moves, so the telemetry surface looks **fresh** while carrying no data, and a -freshness check passes over a blind lane. `telemetry-upsert.sh` refuses such a -body before it writes anything, and an inlined upsert carries the same refusal as -a pre-write gate in its own block (empty, leading `@`, under a byte floor, or not -sentinel-prefixed → skip the cycle fail-closed, no API call). An inline gate -replicates the pre-write half only; the post-write read-back stays here in the -wrapper. +moves, so any check keying on `updatedAt` reads the lane as **fresh** while it +carries no data. (This skill's sibling reader `morning-brief` is not that check — +it parses `lane:` and `last-cycle:` out of the body, so a degraded comment makes +the lane vanish from its report rather than look healthy. The deceived check is +the tower's timestamp-based delivery check.) + +`telemetry-upsert.sh` refuses such a body before it writes anything and re-reads +what landed afterward. An inlined upsert now encodes both halves itself: a +pre-write gate (empty, leading `@`, not sentinel-prefixed, or under a 16-byte +payload floor → skip the cycle, no API call) and a post-write read-back, which is +the half that catches a well-composed file sent through `-f body=@FILE` instead of +`-F body=@FILE`. What an inline block does NOT replicate: the 64 KiB cap and the +body-file containment checks. ## Cross-references diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index adb143cc0..d3fdbf1bc 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -7,21 +7,23 @@ All notable changes to the `source-control` plugin are documented here. Format f ### Fixed -- **`babysit-loop`'s inlined telemetry upsert refuses a degraded body before it writes one (#943).** +- **`babysit-loop`'s inlined telemetry upsert now gates its body and verifies what landed (#943).** The lane's telemetry comment was observed carrying a literal `@C:/…/telemetry_combined.txt` as its entire body across three sessions: `gh` expands a leading `@` only for `--body-file` / `-F - field=@file`, so an `@path` interpolated into a body value is sent as text. Nothing about that - failure is visible from outside — the comment's `updatedAt` still moves, so `morning-brief`'s - freshness check passes over a lane that reported nothing. `claude-ops`'s `telemetry-upsert.sh` - already refuses such a body, but an installed plugin cannot invoke a sibling plugin's script, so - this lane inlines its own upsert and inherited none of that protection. The inlined block now opens - with a pre-write gate: a `$BODY_FILE` that is empty, begins with a literal `@`, falls under the - sentinel-plus-16-byte floor, or is not sentinel-prefixed is refused with a visible notice and the - cycle skips the upsert fail-closed — no POST, no PATCH. Refusing leaves the comment **stale**, - which the freshness check does catch, instead of fresh-but-blind, which it cannot. The prefix - assertion compares bytes rather than a whole first line, so a CRLF-terminated body is not - false-rejected. Scope is pre-write only; the wrapper's post-write read-back is deliberately not - replicated into a prose block that ships without a test harness. + field=@file`, so an `@path` passed as a body VALUE is transmitted as text. `claude-ops`'s + `telemetry-upsert.sh` refuses such a body and re-reads what it wrote, but an installed plugin cannot + invoke a sibling plugin's script, so this lane inlines its own upsert and inherited neither + protection. The block now carries both halves, which catch different failures. A **pre-write gate** + rejects a `$BODY_FILE` that is empty, opens with a literal `@`, is not sentinel-prefixed, or holds + under 16 bytes of payload — no POST, no PATCH. A **post-write read-back** then re-reads the comment + and reports the cycle UNREPORTED unless the persisted body still opens with the sentinel and clears + the same floor; this is the half that catches the actual #943 shape, where the composed file is + perfectly fine and the defect is the invocation (`-f body=@FILE` instead of `-F body=@FILE`) — a + file-only check is structurally blind to it. The 16-byte floor is measured on the payload beneath + the sentinel, matching the wrapper's `MIN_BODY_BYTES` exactly; prefix comparison is byte-wise, so a + CRLF body is not false-rejected. The `$BODY_FILE` sentinel-first-line contract is now stated in + prose rather than left implicit in a comment. Not replicated from the wrapper: the 64 KiB cap and + the body-file containment checks. ## [0.42.1] diff --git a/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md b/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md index 2e5eb4fed..3de5d23fc 100644 --- a/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md +++ b/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md @@ -9,13 +9,21 @@ plugin cannot invoke a sibling plugin's scripts. ```bash MARKER="source-control:babysit-loop" -SENT="" # first line of $BODY_FILE +SENT="" # $BODY_FILE MUST open with this line LOOKUP() { gh api --paginate "repos/$REPO/issues/$ISSUE/comments" \ --jq ".[] | select(.body | startswith(\"$SENT\")) | .id"; } -if [ ! -s "$BODY_FILE" ] || [ "$(head -c 1 "$BODY_FILE")" = "@" ] || - [ "$(wc -c <"$BODY_FILE")" -lt $((${#SENT} + 16)) ] || - [ "$(head -c ${#SENT} "$BODY_FILE")" != "$SENT" ]; then - echo "telemetry: body rejected before any write - empty, a literal @path, under the sentinel+16-byte floor, or not sentinel-prefixed; skipping upsert this cycle (fail closed)" >&2 +SENTINEL_OK() { # $1 = text; true iff it opens with $SENT and carries a payload under it + [ "$(printf '%s' "$1" | head -c ${#SENT})" = "$SENT" ] && + [ "$(printf '%s' "$1" | wc -c | tr -d ' ')" -ge $((${#SENT} + 17)) ] +} +VERIFY() { # $1 = comment id; re-read what LANDED, whatever form the write took + BACK="$(gh api "repos/$REPO/issues/comments/$1" --jq '.body' 2>/dev/null | tr -d '\r')" && + SENTINEL_OK "$BACK" +} +if [ ! -s "$BODY_FILE" ] || [ "$(head -c 1 "$BODY_FILE")" = "@" ]; then + echo "telemetry: body is empty or a literal @path - nothing written; fix the body composition, do not re-run blind" >&2 +elif ! SENTINEL_OK "$(cat "$BODY_FILE")"; then + echo "telemetry: body is not sentinel-prefixed or carries no payload - nothing written; fix the body composition, do not re-run blind" >&2 elif ! LIST=$(LOOKUP); then echo "telemetry: comment lookup failed; skipping upsert this cycle (fail closed)" >&2 else @@ -26,6 +34,8 @@ else CANON=$(printf '%s\n' "$LIST" | sort -n | head -n1) if [ -n "$CANON" ]; then gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" + VERIFY "$CANON" || + echo "telemetry: comment $CANON does NOT carry this cycle's telemetry after the write - treat the lane as UNREPORTED and record it in durable state; do not trust the timestamp" >&2 for DUP in $(printf '%s\n' "$LIST" | sort -n | tail -n +2); do gh api -X PATCH "repos/$REPO/issues/comments/$DUP" \ -f body="Superseded duplicate - canonical telemetry comment: $CANON" || true @@ -34,12 +44,24 @@ else fi ``` -**Pre-write body gate (encoded above, #943).** The leading assertions run before any API call — the -mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes skill ("Never pass a body -as an `@path` string"), which an inlined upsert has no wrapper to enforce for it. A degraded body -that reaches the comment still moves its timestamp, so the telemetry surface looks **fresh** while -carrying no data; refusing the write leaves the comment stale instead, which the freshness check does -catch. Pre-write only — the post-write read-back stays in `claude-ops`'s `telemetry-upsert.sh`. +**`$BODY_FILE` contract.** The file's FIRST line must be exactly `$SENT`, with the cycle's telemetry +below it. The lookup matches on that prefix, so a body composed without it is not merely rejected here +— it would never be found again, and the next cycle would post a second comment. Compose the sentinel +into the file; do not rely on anything downstream to add it. + +**Body gate and post-write verification (encoded above, #943).** Two halves, because they catch +different failures. The **pre-write** assertions run before any API call and reject a `$BODY_FILE` +that is empty, opens with a literal `@`, is not sentinel-prefixed, or carries under 16 bytes of +payload — the mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes skill +("Never pass a body as an `@path` string"). The **post-write** `VERIFY` re-reads what actually landed, +which is what catches the failure the pre-write half structurally cannot: a correctly composed file +sent through a body-value flag (`-f body=@"$BODY_FILE"`) rather than `-F body=@`, where `gh` transmits +the literal path and the file itself was never at fault. + +A degraded body that lands still moves the comment's timestamp, so any check keying on `updatedAt` +reads the lane as **fresh** while it carries nothing. Refusing, or reporting the write UNREPORTED, +is what keeps that from passing silently. Not replicated from the wrapper: the 64 KiB cap and the +body-file containment checks. **Creation race reconcile (encoded above).** Two sessions racing the first-ever upsert can both see an empty lookup and both POST, forking the singleton. The upsert converges every cycle diff --git a/plugins/work-items/CHANGELOG.md b/plugins/work-items/CHANGELOG.md index d3e91c4d8..223d2cd0e 100644 --- a/plugins/work-items/CHANGELOG.md +++ b/plugins/work-items/CHANGELOG.md @@ -7,20 +7,23 @@ All notable changes to the `work-items` plugin are documented here. Format follo ### Fixed -- **`work-loop` and `attend-queue`'s inlined telemetry upserts refuse a degraded body before they - write one (#943).** Both lanes inline the same `gh api` upsert the babysit lane does — an installed +- **`work-loop` and `attend-queue`'s inlined telemetry upserts now gate their body and verify what + landed (#943).** Both lanes inline the same `gh api` upsert the babysit lane does — an installed plugin cannot invoke `claude-ops`'s `telemetry-upsert.sh` — and so inherited none of that wrapper's body checks. The defect that surfaced on the babysit lane is a property of the shared upsert shape, - not of one lane: an `@path` interpolated into a body value is sent as literal text (`gh` expands a - leading `@` only for `--body-file` / `-F field=@file`), and the resulting comment still moves its - `updatedAt`, so a freshness check passes over a lane carrying no data. Both blocks now open with a - pre-write gate: a `$BODY_FILE` that is empty, begins with a literal `@`, falls under the - sentinel-plus-16-byte floor, or is not sentinel-prefixed is refused with a visible notice and the - cycle skips the upsert fail-closed — no POST, no PATCH. Refusing leaves the comment **stale**, - which the freshness check does catch, instead of fresh-but-blind, which it cannot. The prefix - assertion compares bytes rather than a whole first line, so a CRLF-terminated body is not - false-rejected. Scope is pre-write only; the wrapper's post-write read-back is deliberately not - replicated into a prose block that ships without a test harness. + not of one lane: an `@path` passed as a body VALUE is transmitted as literal text (`gh` expands a + leading `@` only for `--body-file` / `-F field=@file`). Both blocks now carry two halves. A + **pre-write gate** rejects a `$BODY_FILE` that is empty, opens with a literal `@`, is not + sentinel-prefixed, or holds under 16 bytes of payload — no POST, no PATCH. A **post-write + read-back** then re-reads the comment and reports the cycle UNREPORTED unless the persisted body + still opens with the sentinel and clears the same floor; this is the half that catches the actual + #943 shape, where the composed file is fine and the defect is the invocation (`-f body=@FILE` + instead of `-F body=@FILE`) — a file-only check cannot see it. The 16-byte floor is measured on the + payload beneath the sentinel, matching the wrapper's `MIN_BODY_BYTES`; prefix comparison is + byte-wise, so a CRLF body is not false-rejected. `work-loop` additionally records a refusal or + failed verification in durable loop state, since stderr does not survive the session and a cycle + that did not report must stay visible to the next one. The `$BODY_FILE` sentinel-first-line contract + is now stated in prose. Not replicated from the wrapper: the 64 KiB cap and containment checks. ## [0.30.3] diff --git a/plugins/work-items/skills/attend-queue/SKILL.md b/plugins/work-items/skills/attend-queue/SKILL.md index 26131e128..e644a02c0 100644 --- a/plugins/work-items/skills/attend-queue/SKILL.md +++ b/plugins/work-items/skills/attend-queue/SKILL.md @@ -119,13 +119,21 @@ handled, the answers written, and the guard mode. Same inlined upsert as the wor ```bash MARKER="work-items:attend-queue" -SENT="" # first line of $BODY_FILE +SENT="" # $BODY_FILE MUST open with this line LOOKUP() { gh api --paginate "repos/$REPO/issues/$ISSUE/comments" \ --jq ".[] | select(.body | startswith(\"$SENT\")) | .id"; } -if [ ! -s "$BODY_FILE" ] || [ "$(head -c 1 "$BODY_FILE")" = "@" ] || - [ "$(wc -c <"$BODY_FILE")" -lt $((${#SENT} + 16)) ] || - [ "$(head -c ${#SENT} "$BODY_FILE")" != "$SENT" ]; then - echo "telemetry: body rejected before any write - empty, a literal @path, under the sentinel+16-byte floor, or not sentinel-prefixed; skipping upsert this cycle (fail closed)" >&2 +SENTINEL_OK() { # $1 = text; true iff it opens with $SENT and carries a payload under it + [ "$(printf '%s' "$1" | head -c ${#SENT})" = "$SENT" ] && + [ "$(printf '%s' "$1" | wc -c | tr -d ' ')" -ge $((${#SENT} + 17)) ] +} +VERIFY() { # $1 = comment id; re-read what LANDED, whatever form the write took + BACK="$(gh api "repos/$REPO/issues/comments/$1" --jq '.body' 2>/dev/null | tr -d '\r')" && + SENTINEL_OK "$BACK" +} +if [ ! -s "$BODY_FILE" ] || [ "$(head -c 1 "$BODY_FILE")" = "@" ]; then + echo "telemetry: body is empty or a literal @path - nothing written; fix the body composition, do not re-run blind" >&2 +elif ! SENTINEL_OK "$(cat "$BODY_FILE")"; then + echo "telemetry: body is not sentinel-prefixed or carries no payload - nothing written; fix the body composition, do not re-run blind" >&2 elif ! LIST=$(LOOKUP); then echo "telemetry: comment lookup failed; skipping upsert this cycle (fail closed)" >&2 else @@ -136,6 +144,8 @@ else CANON=$(printf '%s\n' "$LIST" | sort -n | head -n1) if [ -n "$CANON" ]; then gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" + VERIFY "$CANON" || + echo "telemetry: comment $CANON does NOT carry this cycle's telemetry after the write - treat the lane as UNREPORTED and record it in durable state; do not trust the timestamp" >&2 for DUP in $(printf '%s\n' "$LIST" | sort -n | tail -n +2); do gh api -X PATCH "repos/$REPO/issues/comments/$DUP" \ -f body="Superseded duplicate - canonical telemetry comment: $CANON" || true @@ -144,12 +154,24 @@ else fi ``` -**Pre-write body gate (encoded above, #943).** The leading assertions run before any API call — the -mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes skill ("Never pass a body -as an `@path` string"), which an inlined upsert has no wrapper to enforce for it. A degraded body -that reaches the comment still moves its timestamp, so the telemetry surface looks **fresh** while -carrying no data; refusing the write leaves the comment stale instead, which the freshness check does -catch. Pre-write only — the post-write read-back stays in `claude-ops`'s `telemetry-upsert.sh`. +**`$BODY_FILE` contract.** The file's FIRST line must be exactly `$SENT`, with the pass report below +it. The lookup matches on that prefix, so a body composed without it is not merely rejected here — it +would never be found again, and the next pass would post a second comment. Compose the sentinel into +the file; do not rely on anything downstream to add it. + +**Body gate and post-write verification (encoded above, #943).** Two halves, because they catch +different failures. The **pre-write** assertions run before any API call and reject a `$BODY_FILE` +that is empty, opens with a literal `@`, is not sentinel-prefixed, or carries under 16 bytes of +payload — the mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes skill +("Never pass a body as an `@path` string"). The **post-write** `VERIFY` re-reads what actually landed, +which is what catches the failure the pre-write half structurally cannot: a correctly composed file +sent through a body-value flag (`-f body=@"$BODY_FILE"`) rather than `-F body=@`, where `gh` transmits +the literal path and the file itself was never at fault. + +A degraded body that lands still moves the comment's timestamp, so any check keying on `updatedAt` +reads the lane as **fresh** while it carries nothing. Refusing, or reporting the write UNREPORTED, is +what keeps that from passing silently. Not replicated from the wrapper: the 64 KiB cap and the +body-file containment checks. **Creation race reconcile (encoded above).** Two sessions racing the first-ever upsert can both see an empty lookup and both POST, forking the singleton. The upsert converges every cycle diff --git a/plugins/work-items/skills/work-loop/SKILL.md b/plugins/work-items/skills/work-loop/SKILL.md index c723e3866..4942ac810 100644 --- a/plugins/work-items/skills/work-loop/SKILL.md +++ b/plugins/work-items/skills/work-loop/SKILL.md @@ -74,13 +74,21 @@ invoke a sibling plugin's scripts: ```bash MARKER="work-items:work-loop" -SENT="" # first line of $BODY_FILE +SENT="" # $BODY_FILE MUST open with this line LOOKUP() { gh api --paginate "repos/$REPO/issues/$ISSUE/comments" \ --jq ".[] | select(.body | startswith(\"$SENT\")) | .id"; } -if [ ! -s "$BODY_FILE" ] || [ "$(head -c 1 "$BODY_FILE")" = "@" ] || - [ "$(wc -c <"$BODY_FILE")" -lt $((${#SENT} + 16)) ] || - [ "$(head -c ${#SENT} "$BODY_FILE")" != "$SENT" ]; then - echo "telemetry: body rejected before any write - empty, a literal @path, under the sentinel+16-byte floor, or not sentinel-prefixed; skipping upsert this cycle (fail closed)" >&2 +SENTINEL_OK() { # $1 = text; true iff it opens with $SENT and carries a payload under it + [ "$(printf '%s' "$1" | head -c ${#SENT})" = "$SENT" ] && + [ "$(printf '%s' "$1" | wc -c | tr -d ' ')" -ge $((${#SENT} + 17)) ] +} +VERIFY() { # $1 = comment id; re-read what LANDED, whatever form the write took + BACK="$(gh api "repos/$REPO/issues/comments/$1" --jq '.body' 2>/dev/null | tr -d '\r')" && + SENTINEL_OK "$BACK" +} +if [ ! -s "$BODY_FILE" ] || [ "$(head -c 1 "$BODY_FILE")" = "@" ]; then + echo "telemetry: body is empty or a literal @path - nothing written; fix the body composition, do not re-run blind" >&2 +elif ! SENTINEL_OK "$(cat "$BODY_FILE")"; then + echo "telemetry: body is not sentinel-prefixed or carries no payload - nothing written; fix the body composition, do not re-run blind" >&2 elif ! LIST=$(LOOKUP); then echo "telemetry: comment lookup failed; skipping upsert this cycle (fail closed)" >&2 else @@ -91,6 +99,8 @@ else CANON=$(printf '%s\n' "$LIST" | sort -n | head -n1) if [ -n "$CANON" ]; then gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" + VERIFY "$CANON" || + echo "telemetry: comment $CANON does NOT carry this cycle's telemetry after the write - treat the lane as UNREPORTED and record it in durable state; do not trust the timestamp" >&2 for DUP in $(printf '%s\n' "$LIST" | sort -n | tail -n +2); do gh api -X PATCH "repos/$REPO/issues/comments/$DUP" \ -f body="Superseded duplicate - canonical telemetry comment: $CANON" || true @@ -99,12 +109,25 @@ else fi ``` -**Pre-write body gate (encoded above, #943).** The leading assertions run before any API call — the -mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes skill ("Never pass a body -as an `@path` string"), which an inlined upsert has no wrapper to enforce for it. A degraded body -that reaches the comment still moves its timestamp, so the telemetry surface looks **fresh** while -carrying no data; refusing the write leaves the comment stale instead, which the freshness check does -catch. Pre-write only — the post-write read-back stays in `claude-ops`'s `telemetry-upsert.sh`. +**`$BODY_FILE` contract.** The file's FIRST line must be exactly `$SENT`, with the cycle's telemetry +below it. The lookup matches on that prefix, so a body composed without it is not merely rejected here +— it would never be found again, and the next cycle would post a second comment. Compose the sentinel +into the file; do not rely on anything downstream to add it. + +**Body gate and post-write verification (encoded above, #943).** Two halves, because they catch +different failures. The **pre-write** assertions run before any API call and reject a `$BODY_FILE` +that is empty, opens with a literal `@`, is not sentinel-prefixed, or carries under 16 bytes of +payload — the mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes skill +("Never pass a body as an `@path` string"). The **post-write** `VERIFY` re-reads what actually landed, +which is what catches the failure the pre-write half structurally cannot: a correctly composed file +sent through a body-value flag (`-f body=@"$BODY_FILE"`) rather than `-F body=@`, where `gh` transmits +the literal path and the file itself was never at fault. + +A degraded body that lands still moves the comment's timestamp, so any check keying on `updatedAt` +reads the lane as **fresh** while it carries nothing. Refusing, or reporting the write UNREPORTED, +is what keeps that from passing silently. A refusal or a failed verification is a cycle that did not +report: record it in durable state so the next cycle sees it, since stderr does not survive the +session. Not replicated from the wrapper: the 64 KiB cap and the body-file containment checks. **Creation race reconcile (encoded above).** Two sessions racing the first-ever upsert can both see an empty lookup and both POST, forking the singleton. The upsert converges every cycle From 35440a49a35e688322c5c691df132f1a8c16abc1 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:04:19 -0400 Subject: [PATCH 3/7] fix(loop-lane): report a degraded telemetry create, don't fall through silently The post-write read-back covered the PATCH path only. On the first-ever upsert the POST runs, the block re-lists, and everything downstream -- including VERIFY -- sits behind `if [ -n "$CANON" ]`. A POST that lands degraded carries no sentinel, so the re-lookup returns nothing, `$CANON` is empty, and the whole branch is skipped: no PATCH, no verification, no notice. A degraded PATCH reported UNREPORTED while a degraded create said nothing at all. The same asymmetry swallowed a failed re-list, which `|| LIST=""` already tolerated on the way to the same empty `$CANON`. Both now take the else branch and report the cycle UNREPORTED, so every path that ends without a verified telemetry comment is visible to the next cycle. The prose in all three inlined blocks, the loop-lane convention, and the two CHANGELOG entries state the create path explicitly rather than leaving "re-reads the comment after the write" to imply a coverage the block did not have. Co-Authored-By: Claude Opus 5 (1M context) --- docs/conventions/loop-lane/README.md | 7 ++++--- plugins/source-control/CHANGELOG.md | 4 +++- .../skills/babysit-loop/reference/telemetry-upsert.md | 5 +++++ plugins/work-items/CHANGELOG.md | 4 +++- plugins/work-items/skills/attend-queue/SKILL.md | 5 +++++ plugins/work-items/skills/work-loop/SKILL.md | 5 +++++ 6 files changed, 25 insertions(+), 5 deletions(-) diff --git a/docs/conventions/loop-lane/README.md b/docs/conventions/loop-lane/README.md index 65b2ee69f..bb6d8216a 100644 --- a/docs/conventions/loop-lane/README.md +++ b/docs/conventions/loop-lane/README.md @@ -430,9 +430,10 @@ an `@path` string", and encodes that rule mechanically in its own block (#943) a 16-byte payload floor, before any API call. A **post-write read-back** then re-reads what landed, because the pre-write half is structurally blind to the failure that produced #943 in the first place: a well-composed file passed through a body-VALUE flag (`-f body=@FILE` rather than `-F -body=@FILE`), where `gh` transmits the literal path and the file was never at fault. A failed -verification means the cycle did not report — record it in durable state, since stderr does not -survive the session. Not replicated inline: the wrapper's 64 KiB cap and its body-file containment +body=@FILE`), where `gh` transmits the literal path and the file was never at fault. A degraded +create is covered on the same footing: it leaves no sentinel-prefixed comment to re-read, so that +branch reports UNREPORTED rather than falling through silently. A failed verification means the +cycle did not report — record it in durable state, since stderr does not survive the session. Not replicated inline: the wrapper's 64 KiB cap and its body-file containment checks. **Durable loop state.** Conversation context is lossy across compaction, so a lane persists its diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index 5295fa5df..5eab0f5b8 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -19,7 +19,9 @@ All notable changes to the `source-control` plugin are documented here. Format f and reports the cycle UNREPORTED unless the persisted body still opens with the sentinel and clears the same floor; this is the half that catches the actual #943 shape, where the composed file is perfectly fine and the defect is the invocation (`-f body=@FILE` instead of `-F body=@FILE`) — a - file-only check is structurally blind to it. The 16-byte floor is measured on the payload beneath + file-only check is structurally blind to it. The create path is covered by the same cycle's + PATCH, and a degraded POST leaves no sentinel-prefixed comment to re-read, so that branch now + reports UNREPORTED too instead of falling through silently. The 16-byte floor is measured on the payload beneath the sentinel, matching the wrapper's `MIN_BODY_BYTES` exactly; prefix comparison is byte-wise, so a CRLF body is not false-rejected. The `$BODY_FILE` sentinel-first-line contract is now stated in prose rather than left implicit in a comment. Not replicated from the wrapper: the 64 KiB cap and diff --git a/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md b/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md index 2a6250afc..2d72ca10b 100644 --- a/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md +++ b/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md @@ -40,6 +40,8 @@ else gh api -X PATCH "repos/$REPO/issues/comments/$DUP" \ -f body="Superseded duplicate - canonical telemetry comment: $CANON" || true done + else + echo "telemetry: the write left no sentinel-prefixed comment to verify - treat the lane as UNREPORTED and record it in durable state; the next cycle retries the create" >&2 fi fi ``` @@ -57,6 +59,9 @@ payload — the mechanical form of the `@path`-as-body rule owned by the `claude which is what catches the failure the pre-write half structurally cannot: a correctly composed file sent through a body-value flag (`-f body=@"$BODY_FILE"`) rather than `-F body=@`, where `gh` transmits the literal path and the file itself was never at fault. +On the first-ever upsert the same cycle's PATCH is what verifies the create, so a POST that lands +degraded carries no sentinel, the re-lookup finds nothing, and there is no comment to re-read. That +path reports UNREPORTED as well rather than passing silently, and the next cycle retries the create. A degraded body that lands still moves the comment's timestamp, so any check keying on `updatedAt` reads the lane as **fresh** while it carries nothing. Refusing, or reporting the write UNREPORTED, diff --git a/plugins/work-items/CHANGELOG.md b/plugins/work-items/CHANGELOG.md index 223d2cd0e..f92ad804c 100644 --- a/plugins/work-items/CHANGELOG.md +++ b/plugins/work-items/CHANGELOG.md @@ -18,7 +18,9 @@ All notable changes to the `work-items` plugin are documented here. Format follo read-back** then re-reads the comment and reports the cycle UNREPORTED unless the persisted body still opens with the sentinel and clears the same floor; this is the half that catches the actual #943 shape, where the composed file is fine and the defect is the invocation (`-f body=@FILE` - instead of `-F body=@FILE`) — a file-only check cannot see it. The 16-byte floor is measured on the + instead of `-F body=@FILE`) — a file-only check cannot see it. The create path is covered by the same + cycle's PATCH, and a degraded POST leaves no sentinel-prefixed comment to re-read, so that branch + now reports UNREPORTED too instead of falling through silently. The 16-byte floor is measured on the payload beneath the sentinel, matching the wrapper's `MIN_BODY_BYTES`; prefix comparison is byte-wise, so a CRLF body is not false-rejected. `work-loop` additionally records a refusal or failed verification in durable loop state, since stderr does not survive the session and a cycle diff --git a/plugins/work-items/skills/attend-queue/SKILL.md b/plugins/work-items/skills/attend-queue/SKILL.md index bbabbcddf..952f4ba30 100644 --- a/plugins/work-items/skills/attend-queue/SKILL.md +++ b/plugins/work-items/skills/attend-queue/SKILL.md @@ -150,6 +150,8 @@ else gh api -X PATCH "repos/$REPO/issues/comments/$DUP" \ -f body="Superseded duplicate - canonical telemetry comment: $CANON" || true done + else + echo "telemetry: the write left no sentinel-prefixed comment to verify - treat the lane as UNREPORTED and record it in durable state; the next cycle retries the create" >&2 fi fi ``` @@ -167,6 +169,9 @@ payload — the mechanical form of the `@path`-as-body rule owned by the `claude which is what catches the failure the pre-write half structurally cannot: a correctly composed file sent through a body-value flag (`-f body=@"$BODY_FILE"`) rather than `-F body=@`, where `gh` transmits the literal path and the file itself was never at fault. +On the first-ever upsert the same cycle's PATCH is what verifies the create, so a POST that lands +degraded carries no sentinel, the re-lookup finds nothing, and there is no comment to re-read. That +path reports UNREPORTED as well rather than passing silently, and the next cycle retries the create. A degraded body that lands still moves the comment's timestamp, so any check keying on `updatedAt` reads the lane as **fresh** while it carries nothing. Refusing, or reporting the write UNREPORTED, is diff --git a/plugins/work-items/skills/work-loop/SKILL.md b/plugins/work-items/skills/work-loop/SKILL.md index 6e90fe7a5..d06642f06 100644 --- a/plugins/work-items/skills/work-loop/SKILL.md +++ b/plugins/work-items/skills/work-loop/SKILL.md @@ -105,6 +105,8 @@ else gh api -X PATCH "repos/$REPO/issues/comments/$DUP" \ -f body="Superseded duplicate - canonical telemetry comment: $CANON" || true done + else + echo "telemetry: the write left no sentinel-prefixed comment to verify - treat the lane as UNREPORTED and record it in durable state; the next cycle retries the create" >&2 fi fi ``` @@ -122,6 +124,9 @@ payload — the mechanical form of the `@path`-as-body rule owned by the `claude which is what catches the failure the pre-write half structurally cannot: a correctly composed file sent through a body-value flag (`-f body=@"$BODY_FILE"`) rather than `-F body=@`, where `gh` transmits the literal path and the file itself was never at fault. +On the first-ever upsert the same cycle's PATCH is what verifies the create, so a POST that lands +degraded carries no sentinel, the re-lookup finds nothing, and there is no comment to re-read. That +path reports UNREPORTED as well rather than passing silently, and the next cycle retries the create. A degraded body that lands still moves the comment's timestamp, so any check keying on `updatedAt` reads the lane as **fresh** while it carries nothing. Refusing, or reporting the write UNREPORTED, From 8b1ed72f53f7f1fd031fb2c31c5b87b9ae2e1f02 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:49:50 -0400 Subject: [PATCH 4/7] fix(loop-lane): check the telemetry write's exit status before trusting the read-back An independent review found the read-back could not carry the guarantee its own prose claimed. Three defects, all in the same seam: - The PATCH's exit status was never checked. A write that failed -- 403, 422, secondary rate limit, dropped connection -- fell through to VERIFY, which re-read the comment, found the PREVIOUS cycle's well-formed body, and returned success. The most common write failure reported a verified cycle over a stale comment. The wrapper exits 5 there before its own read-back runs; the inline copies dropped that and kept the read-back. - The duplicate-supersede pass ran even when VERIFY had already failed, so a cycle whose own write was unproven still tombstoned every other sentinel comment. If a racing session held the only good telemetry, the cycle destroyed it and left nothing to recover from. - The byte floor was `${#SENT} + 17` over the whole body, which assumes exactly one separator byte. A CRLF body puts two there, so the effective payload floor sat one byte below the wrapper's -- while the CHANGELOG claimed it matched `MIN_BODY_BYTES` "exactly". The floor is now measured on everything below line 1 and compared against 16 directly, which agrees byte-for-byte on LF and CRLF alike and drops the magic number. Prose corrected to what the block actually does. It no longer justifies the read-back with `-f body=@FILE`, an invocation the block never emits; it now names what the read-back does catch (a write that reported success and stored something else) and states the limits it inherits rather than fixes: a PATCH that succeeds while storing the previous body still verifies, the read-back proves SOME well-formed telemetry is present rather than THIS cycle's, and every inline branch exits 0 and reports through stderr alone. `morning-brief` was correctly exonerated, but the replacement named "the tower's timestamp-based delivery check", which exists nowhere in this repo. Attributed to any consumer keying on the comment's timestamp instead of its body -- which is what is actually true. Also removes two stray `|||||||` diff3 markers left in the claude-ops and source-control CHANGELOGs by the earlier merges; no gate catches those. Co-Authored-By: Claude Opus 5 (1M context) --- docs/conventions/loop-lane/README.md | 20 ++++--- plugins/claude-ops/CHANGELOG.md | 9 ++- plugins/claude-ops/skills/lanes/SKILL.md | 20 ++++--- plugins/source-control/CHANGELOG.md | 28 +++++---- .../reference/telemetry-upsert.md | 55 +++++++++++------- plugins/work-items/CHANGELOG.md | 31 ++++++---- .../work-items/skills/attend-queue/SKILL.md | 57 ++++++++++-------- plugins/work-items/skills/work-loop/SKILL.md | 58 +++++++++++-------- 8 files changed, 165 insertions(+), 113 deletions(-) diff --git a/docs/conventions/loop-lane/README.md b/docs/conventions/loop-lane/README.md index bb6d8216a..243f4e745 100644 --- a/docs/conventions/loop-lane/README.md +++ b/docs/conventions/loop-lane/README.md @@ -425,16 +425,18 @@ identified by a machine sentinel marker and **edited in place** every cycle — script, so each lane **inlines** the small `gh api` upsert and the coupling to `claude-ops` stays one-directional. An inlined upsert is bound by the `@path`-as-body rule in [`claude-ops` lanes](../../../plugins/claude-ops/skills/lanes/SKILL.md), section "Never pass a body as -an `@path` string", and encodes that rule mechanically in its own block (#943) as two halves. A +an `@path` string", and encodes that rule mechanically in its own block (#943) as three checks. A **pre-write gate** refuses a body that is empty, a literal `@path`, not sentinel-prefixed, or under a -16-byte payload floor, before any API call. A **post-write read-back** then re-reads what landed, -because the pre-write half is structurally blind to the failure that produced #943 in the first -place: a well-composed file passed through a body-VALUE flag (`-f body=@FILE` rather than `-F -body=@FILE`), where `gh` transmits the literal path and the file was never at fault. A degraded -create is covered on the same footing: it leaves no sentinel-prefixed comment to re-read, so that -branch reports UNREPORTED rather than falling through silently. A failed verification means the -cycle did not report — record it in durable state, since stderr does not survive the session. Not replicated inline: the wrapper's 64 KiB cap and its body-file containment -checks. +16-byte payload floor measured below the sentinel line, before any API call. The **write's own exit +status** is then checked, because a failed write leaves the previous cycle's body in place — which a +read-back running regardless would accept. A **post-write read-back** re-reads what the write stored, +the only check that sees a write which reported success and stored something else. Every branch that +ends without a verified body reports UNREPORTED and skips the duplicate-supersede pass, so a cycle +whose own write is unproven never tombstones a racing session's comment; carry that forward, since +stderr does not survive the session. Known limits inherited from the wrapper: a PATCH that succeeds +while storing the previous body still verifies, and the read-back proves *some* well-formed telemetry +is present, not *this* cycle's. Not replicated inline: the 64 KiB cap, the body-file containment +checks, retries, and the wrapper's distinct non-zero exits — every inline branch exits 0. **Durable loop state.** Conversation context is lossy across compaction, so a lane persists its adaptive-cap streak counter, its rate-limit-warning latch, its consecutive-no-progress counter, and diff --git a/plugins/claude-ops/CHANGELOG.md b/plugins/claude-ops/CHANGELOG.md index 698b8cdd2..d92d83c38 100644 --- a/plugins/claude-ops/CHANGELOG.md +++ b/plugins/claude-ops/CHANGELOG.md @@ -18,11 +18,10 @@ All notable changes to the `claude-ops` plugin are documented here. Format follo - **Corrected: `morning-brief` is not the check a degraded telemetry body deceives.** The rule said a freshness check "passes over a blind lane". Verified against `morning-brief.sh`'s `print_telemetry`: it parses `lane:` and `last-cycle:` out of the comment BODY, so an `@path` body carries no `lane:` - field and the lane disappears from the report entirely rather than reading as healthy. The check - that is genuinely fooled is a timestamp-based delivery check reading `updatedAt`, which moves on - every successful write regardless of content. The rule now attributes the failure correctly instead - of naming a sibling reader that would in fact surface it. -||||||| ec7ed1b3 + field and the lane disappears from the report entirely rather than reading as healthy. What a + degraded body deceives is any consumer keying on the comment's timestamp instead of its body — the + timestamp moves on every successful write regardless of content. The rule now attributes the + failure that way rather than naming a sibling reader that would in fact surface it. ## [0.24.4] diff --git a/plugins/claude-ops/skills/lanes/SKILL.md b/plugins/claude-ops/skills/lanes/SKILL.md index b38153090..18cf2ca14 100644 --- a/plugins/claude-ops/skills/lanes/SKILL.md +++ b/plugins/claude-ops/skills/lanes/SKILL.md @@ -289,16 +289,22 @@ The failure is invisible from the outside (#943): the comment's timestamp still moves, so any check keying on `updatedAt` reads the lane as **fresh** while it carries no data. (This skill's sibling reader `morning-brief` is not that check — it parses `lane:` and `last-cycle:` out of the body, so a degraded comment makes -the lane vanish from its report rather than look healthy. The deceived check is -the tower's timestamp-based delivery check.) +the lane vanish from its report rather than look healthy. What a degraded body +deceives is any consumer that keys on the comment's timestamp instead of reading +its body.) `telemetry-upsert.sh` refuses such a body before it writes anything and re-reads -what landed afterward. An inlined upsert now encodes both halves itself: a +what landed afterward. An inlined upsert now encodes three checks itself: a pre-write gate (empty, leading `@`, not sentinel-prefixed, or under a 16-byte -payload floor → skip the cycle, no API call) and a post-write read-back, which is -the half that catches a well-composed file sent through `-f body=@FILE` instead of -`-F body=@FILE`. What an inline block does NOT replicate: the 64 KiB cap and the -body-file containment checks. +payload floor measured below the sentinel line → skip the cycle, no API call), a +check of the write's own exit status (a failed write leaves the previous cycle's +body in place, which a read-back running regardless would accept), and a +post-write read-back of what the write stored. What an inline block does NOT +replicate: the 64 KiB cap, the body-file containment checks, retries, and this +script's distinct non-zero exit codes — an inline branch always exits 0 and +reports through stderr. It also inherits this script's own limits: a PATCH that +succeeds while storing the previous body still verifies, and the read-back proves +*some* well-formed telemetry is present, not *this* cycle's. ## Cross-references diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index 40d27cb02..dfda52f8f 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -15,18 +15,24 @@ All notable changes to the `source-control` plugin are documented here. Format f invoke a sibling plugin's script, so this lane inlines its own upsert and inherited neither protection. The block now carries both halves, which catch different failures. A **pre-write gate** rejects a `$BODY_FILE` that is empty, opens with a literal `@`, is not sentinel-prefixed, or holds - under 16 bytes of payload — no POST, no PATCH. A **post-write read-back** then re-reads the comment - and reports the cycle UNREPORTED unless the persisted body still opens with the sentinel and clears - the same floor; this is the half that catches the actual #943 shape, where the composed file is - perfectly fine and the defect is the invocation (`-f body=@FILE` instead of `-F body=@FILE`) — a - file-only check is structurally blind to it. The create path is covered by the same cycle's + under 16 bytes of payload — no POST, no PATCH. The **write's own exit status** is then checked, because a + failed PATCH leaves the previous cycle's body in place and a read-back running regardless would + accept it. A **post-write read-back** then re-reads what the write stored and reports the cycle + UNREPORTED unless that body still opens with the sentinel and clears the same floor; this is the + check that would have caught the actual #943 shape, where the composed file is perfectly fine and + the defect is the invocation (`-f body=@FILE` instead of `-F body=@FILE`) — a file-only check is + structurally blind to it. The create path is covered by the same cycle's PATCH, and a degraded POST leaves no sentinel-prefixed comment to re-read, so that branch now - reports UNREPORTED too instead of falling through silently. The 16-byte floor is measured on the payload beneath - the sentinel, matching the wrapper's `MIN_BODY_BYTES` exactly; prefix comparison is byte-wise, so a - CRLF body is not false-rejected. The `$BODY_FILE` sentinel-first-line contract is now stated in - prose rather than left implicit in a comment. Not replicated from the wrapper: the 64 KiB cap and - the body-file containment checks. -||||||| 23521e3d + reports UNREPORTED too instead of falling through silently. The 16-byte floor is measured on everything below the + sentinel LINE, so it matches the wrapper's `MIN_BODY_BYTES` byte-for-byte on LF and CRLF alike; + prefix comparison is byte-wise, so a CRLF body is not false-rejected. Every branch that ends without + a verified body reports UNREPORTED and skips the duplicate-supersede pass, so a cycle whose own + write is unproven never tombstones a racing session's comment. The `$BODY_FILE` sentinel-first-line contract is now stated in + prose rather than left implicit in a comment. Two wrapper limits are inherited rather than fixed: a + PATCH that succeeds while storing the previous body still verifies, and the read-back proves *some* + well-formed telemetry is present, not *this* cycle's. Not replicated at all: the 64 KiB cap, the + body-file containment checks, retries, and the wrapper's distinct non-zero exits — every inline + branch exits 0 and reports through stderr. ## [0.43.0] diff --git a/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md b/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md index 2d72ca10b..b737d3738 100644 --- a/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md +++ b/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md @@ -12,9 +12,9 @@ MARKER="source-control:babysit-loop" SENT="" # $BODY_FILE MUST open with this line LOOKUP() { gh api --paginate "repos/$REPO/issues/$ISSUE/comments" \ --jq ".[] | select(.body | startswith(\"$SENT\")) | .id"; } -SENTINEL_OK() { # $1 = text; true iff it opens with $SENT and carries a payload under it +SENTINEL_OK() { # $1 = text; true iff line 1 is exactly $SENT and >=16 payload bytes follow [ "$(printf '%s' "$1" | head -c ${#SENT})" = "$SENT" ] && - [ "$(printf '%s' "$1" | wc -c | tr -d ' ')" -ge $((${#SENT} + 17)) ] + [ "$(printf '%s' "$1" | tail -n +2 | wc -c | tr -d ' ')" -ge 16 ] } VERIFY() { # $1 = comment id; re-read what LANDED, whatever form the write took BACK="$(gh api "repos/$REPO/issues/comments/$1" --jq '.body' 2>/dev/null | tr -d '\r')" && @@ -32,16 +32,17 @@ else LIST=$(LOOKUP) || LIST="" # re-list; a failure here converges next cycle fi CANON=$(printf '%s\n' "$LIST" | sort -n | head -n1) - if [ -n "$CANON" ]; then - gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" - VERIFY "$CANON" || - echo "telemetry: comment $CANON does NOT carry a well-formed telemetry body after the write - treat the lane as UNREPORTED and record it in durable state; do not trust the timestamp" >&2 + if [ -z "$CANON" ]; then + echo "telemetry: no sentinel-prefixed comment to write to - treat the lane as UNREPORTED and carry that forward to the next cycle" >&2 + elif ! gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" >/dev/null; then + echo "telemetry: the PATCH of comment $CANON failed - treat the lane as UNREPORTED and carry that forward to the next cycle; the comment still holds an earlier cycle" >&2 + elif ! VERIFY "$CANON"; then + echo "telemetry: comment $CANON does NOT carry a well-formed telemetry body after the write - treat the lane as UNREPORTED and carry that forward to the next cycle; do not trust the timestamp" >&2 + else for DUP in $(printf '%s\n' "$LIST" | sort -n | tail -n +2); do gh api -X PATCH "repos/$REPO/issues/comments/$DUP" \ -f body="Superseded duplicate - canonical telemetry comment: $CANON" || true done - else - echo "telemetry: the write left no sentinel-prefixed comment to verify - treat the lane as UNREPORTED and record it in durable state; the next cycle retries the create" >&2 fi fi ``` @@ -51,22 +52,32 @@ below it. The lookup matches on that prefix, so a body composed without it is no — it would never be found again, and the next cycle would post a second comment. Compose the sentinel into the file; do not rely on anything downstream to add it. -**Body gate and post-write verification (encoded above, #943).** Two halves, because they catch +**Body gate, write check, and read-back (encoded above, #943).** Three checks, because they catch different failures. The **pre-write** assertions run before any API call and reject a `$BODY_FILE` -that is empty, opens with a literal `@`, is not sentinel-prefixed, or carries under 16 bytes of -payload — the mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes skill -("Never pass a body as an `@path` string"). The **post-write** `VERIFY` re-reads what actually landed, -which is what catches the failure the pre-write half structurally cannot: a correctly composed file -sent through a body-value flag (`-f body=@"$BODY_FILE"`) rather than `-F body=@`, where `gh` transmits -the literal path and the file itself was never at fault. -On the first-ever upsert the same cycle's PATCH is what verifies the create, so a POST that lands -degraded carries no sentinel, the re-lookup finds nothing, and there is no comment to re-read. That -path reports UNREPORTED as well rather than passing silently, and the next cycle retries the create. +that is empty, opens with a literal `@`, is not sentinel-prefixed, or carries under 16 payload bytes +below the sentinel — the mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes +skill ("Never pass a body as an `@path` string"). The floor is measured on everything below line 1, +so it matches the wrapper's `MIN_BODY_BYTES` byte-for-byte whether that line ends in LF or CRLF. The +**write's own exit status** is checked next: a PATCH that fails leaves the previous cycle's body in +place, which a read-back running regardless would happily accept. The **post-write** `VERIFY` then +re-reads what the write stored — the only check that sees a write which reported success and stored +something else: a mangled body, a concurrent overwrite, a deleted comment. It is also the half that +would have caught #943 itself, where the composed file was correct and the defect was the invocation +(`-f body=@FILE` transmits the literal path; this block only ever uses `-F body=@`). + +Every branch that ends without a verified body says so and skips the duplicate-supersede pass, so a +cycle whose own write is unproven never tombstones a racing session's comment. A degraded body that +does land still moves the comment's timestamp, so any consumer keying on that timestamp rather than +on the body reads the lane as **fresh** while it carries nothing — which is why a refusal, a failed +write, and a failed verification all have to be carried forward: stderr does not survive the session, +and the next cycle must see that this one did not report. A lane with durable loop state records it +there; a lane without one carries it in the cycle's own summary. -A degraded body that lands still moves the comment's timestamp, so any check keying on `updatedAt` -reads the lane as **fresh** while it carries nothing. Refusing, or reporting the write UNREPORTED, -is what keeps that from passing silently. Not replicated from the wrapper: the 64 KiB cap and the -body-file containment checks. +Known limits, inherited from the wrapper: a PATCH that succeeds while storing the previous body still +verifies, and `VERIFY` asserts that *some* well-formed telemetry is present, not that *this* cycle's +write is what is present. Not replicated at all: the 64 KiB cap, the body-file containment checks, +retries, and the wrapper's distinct non-zero exit codes — every branch here exits 0 and reports +through stderr alone. **Creation race reconcile (encoded above).** Two sessions racing the first-ever upsert can both see an empty lookup and both POST, forking the singleton. The upsert converges every cycle diff --git a/plugins/work-items/CHANGELOG.md b/plugins/work-items/CHANGELOG.md index f92ad804c..e70b5b1d7 100644 --- a/plugins/work-items/CHANGELOG.md +++ b/plugins/work-items/CHANGELOG.md @@ -12,20 +12,27 @@ All notable changes to the `work-items` plugin are documented here. Format follo plugin cannot invoke `claude-ops`'s `telemetry-upsert.sh` — and so inherited none of that wrapper's body checks. The defect that surfaced on the babysit lane is a property of the shared upsert shape, not of one lane: an `@path` passed as a body VALUE is transmitted as literal text (`gh` expands a - leading `@` only for `--body-file` / `-F field=@file`). Both blocks now carry two halves. A + leading `@` only for `--body-file` / `-F field=@file`). Both blocks now carry three checks. A **pre-write gate** rejects a `$BODY_FILE` that is empty, opens with a literal `@`, is not - sentinel-prefixed, or holds under 16 bytes of payload — no POST, no PATCH. A **post-write - read-back** then re-reads the comment and reports the cycle UNREPORTED unless the persisted body - still opens with the sentinel and clears the same floor; this is the half that catches the actual - #943 shape, where the composed file is fine and the defect is the invocation (`-f body=@FILE` - instead of `-F body=@FILE`) — a file-only check cannot see it. The create path is covered by the same - cycle's PATCH, and a degraded POST leaves no sentinel-prefixed comment to re-read, so that branch - now reports UNREPORTED too instead of falling through silently. The 16-byte floor is measured on the - payload beneath the sentinel, matching the wrapper's `MIN_BODY_BYTES`; prefix comparison is + sentinel-prefixed, or holds under 16 bytes of payload — no POST, no PATCH. The **write's own exit + status** is then checked, because a failed PATCH leaves the previous cycle's body in place and a + read-back running regardless would accept it. A **post-write read-back** re-reads what the write + stored and reports the cycle UNREPORTED unless that body still opens with the sentinel and clears + the same floor; this is the check that would have caught the actual #943 shape, where the composed + file is fine and the defect is the invocation (`-f body=@FILE` instead of `-F body=@FILE`) — a + file-only check cannot see it. Every branch that ends without a verified body — including a + degraded create, which leaves no sentinel-prefixed comment to re-read — reports UNREPORTED and + skips the duplicate-supersede pass, so a cycle whose own write is unproven never tombstones a + racing session's comment. The 16-byte floor is measured on everything below the sentinel LINE, so + it matches the wrapper's `MIN_BODY_BYTES` byte-for-byte on LF and CRLF alike; prefix comparison is byte-wise, so a CRLF body is not false-rejected. `work-loop` additionally records a refusal or - failed verification in durable loop state, since stderr does not survive the session and a cycle - that did not report must stay visible to the next one. The `$BODY_FILE` sentinel-first-line contract - is now stated in prose. Not replicated from the wrapper: the 64 KiB cap and containment checks. + failed verification in durable loop state; `attend-queue` has none, so it carries the same fact in + the cycle's own summary — either way stderr does not survive the session and a cycle that did not + report must stay visible to the next one. The `$BODY_FILE` sentinel-first-line contract is now + stated in prose. Two wrapper limits are inherited rather than fixed: a PATCH that succeeds while + storing the previous body still verifies, and the read-back proves *some* well-formed telemetry is + present, not *this* cycle's. Not replicated at all: the 64 KiB cap, the containment checks, + retries, and the wrapper's distinct non-zero exits — every inline branch exits 0. ## [0.30.3] diff --git a/plugins/work-items/skills/attend-queue/SKILL.md b/plugins/work-items/skills/attend-queue/SKILL.md index 952f4ba30..6818f7087 100644 --- a/plugins/work-items/skills/attend-queue/SKILL.md +++ b/plugins/work-items/skills/attend-queue/SKILL.md @@ -122,9 +122,9 @@ MARKER="work-items:attend-queue" SENT="" # $BODY_FILE MUST open with this line LOOKUP() { gh api --paginate "repos/$REPO/issues/$ISSUE/comments" \ --jq ".[] | select(.body | startswith(\"$SENT\")) | .id"; } -SENTINEL_OK() { # $1 = text; true iff it opens with $SENT and carries a payload under it +SENTINEL_OK() { # $1 = text; true iff line 1 is exactly $SENT and >=16 payload bytes follow [ "$(printf '%s' "$1" | head -c ${#SENT})" = "$SENT" ] && - [ "$(printf '%s' "$1" | wc -c | tr -d ' ')" -ge $((${#SENT} + 17)) ] + [ "$(printf '%s' "$1" | tail -n +2 | wc -c | tr -d ' ')" -ge 16 ] } VERIFY() { # $1 = comment id; re-read what LANDED, whatever form the write took BACK="$(gh api "repos/$REPO/issues/comments/$1" --jq '.body' 2>/dev/null | tr -d '\r')" && @@ -142,16 +142,17 @@ else LIST=$(LOOKUP) || LIST="" # re-list; a failure here converges next cycle fi CANON=$(printf '%s\n' "$LIST" | sort -n | head -n1) - if [ -n "$CANON" ]; then - gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" - VERIFY "$CANON" || - echo "telemetry: comment $CANON does NOT carry a well-formed telemetry body after the write - treat the lane as UNREPORTED and record it in durable state; do not trust the timestamp" >&2 + if [ -z "$CANON" ]; then + echo "telemetry: no sentinel-prefixed comment to write to - treat the lane as UNREPORTED and carry that forward to the next cycle" >&2 + elif ! gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" >/dev/null; then + echo "telemetry: the PATCH of comment $CANON failed - treat the lane as UNREPORTED and carry that forward to the next cycle; the comment still holds an earlier cycle" >&2 + elif ! VERIFY "$CANON"; then + echo "telemetry: comment $CANON does NOT carry a well-formed telemetry body after the write - treat the lane as UNREPORTED and carry that forward to the next cycle; do not trust the timestamp" >&2 + else for DUP in $(printf '%s\n' "$LIST" | sort -n | tail -n +2); do gh api -X PATCH "repos/$REPO/issues/comments/$DUP" \ -f body="Superseded duplicate - canonical telemetry comment: $CANON" || true done - else - echo "telemetry: the write left no sentinel-prefixed comment to verify - treat the lane as UNREPORTED and record it in durable state; the next cycle retries the create" >&2 fi fi ``` @@ -161,22 +162,32 @@ it. The lookup matches on that prefix, so a body composed without it is not mere would never be found again, and the next pass would post a second comment. Compose the sentinel into the file; do not rely on anything downstream to add it. -**Body gate and post-write verification (encoded above, #943).** Two halves, because they catch +**Body gate, write check, and read-back (encoded above, #943).** Three checks, because they catch different failures. The **pre-write** assertions run before any API call and reject a `$BODY_FILE` -that is empty, opens with a literal `@`, is not sentinel-prefixed, or carries under 16 bytes of -payload — the mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes skill -("Never pass a body as an `@path` string"). The **post-write** `VERIFY` re-reads what actually landed, -which is what catches the failure the pre-write half structurally cannot: a correctly composed file -sent through a body-value flag (`-f body=@"$BODY_FILE"`) rather than `-F body=@`, where `gh` transmits -the literal path and the file itself was never at fault. -On the first-ever upsert the same cycle's PATCH is what verifies the create, so a POST that lands -degraded carries no sentinel, the re-lookup finds nothing, and there is no comment to re-read. That -path reports UNREPORTED as well rather than passing silently, and the next cycle retries the create. - -A degraded body that lands still moves the comment's timestamp, so any check keying on `updatedAt` -reads the lane as **fresh** while it carries nothing. Refusing, or reporting the write UNREPORTED, is -what keeps that from passing silently. Not replicated from the wrapper: the 64 KiB cap and the -body-file containment checks. +that is empty, opens with a literal `@`, is not sentinel-prefixed, or carries under 16 payload bytes +below the sentinel — the mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes +skill ("Never pass a body as an `@path` string"). The floor is measured on everything below line 1, +so it matches the wrapper's `MIN_BODY_BYTES` byte-for-byte whether that line ends in LF or CRLF. The +**write's own exit status** is checked next: a PATCH that fails leaves the previous cycle's body in +place, which a read-back running regardless would happily accept. The **post-write** `VERIFY` then +re-reads what the write stored — the only check that sees a write which reported success and stored +something else: a mangled body, a concurrent overwrite, a deleted comment. It is also the half that +would have caught #943 itself, where the composed file was correct and the defect was the invocation +(`-f body=@FILE` transmits the literal path; this block only ever uses `-F body=@`). + +Every branch that ends without a verified body says so and skips the duplicate-supersede pass, so a +cycle whose own write is unproven never tombstones a racing session's comment. A degraded body that +does land still moves the comment's timestamp, so any consumer keying on that timestamp rather than +on the body reads the lane as **fresh** while it carries nothing — which is why a refusal, a failed +write, and a failed verification all have to be carried forward: stderr does not survive the session, +and the next cycle must see that this one did not report. A lane with durable loop state records it +there; a lane without one carries it in the cycle's own summary. + +Known limits, inherited from the wrapper: a PATCH that succeeds while storing the previous body still +verifies, and `VERIFY` asserts that *some* well-formed telemetry is present, not that *this* cycle's +write is what is present. Not replicated at all: the 64 KiB cap, the body-file containment checks, +retries, and the wrapper's distinct non-zero exit codes — every branch here exits 0 and reports +through stderr alone. **Creation race reconcile (encoded above).** Two sessions racing the first-ever upsert can both see an empty lookup and both POST, forking the singleton. The upsert converges every cycle diff --git a/plugins/work-items/skills/work-loop/SKILL.md b/plugins/work-items/skills/work-loop/SKILL.md index d06642f06..f7f9b8d76 100644 --- a/plugins/work-items/skills/work-loop/SKILL.md +++ b/plugins/work-items/skills/work-loop/SKILL.md @@ -77,9 +77,9 @@ MARKER="work-items:work-loop" SENT="" # $BODY_FILE MUST open with this line LOOKUP() { gh api --paginate "repos/$REPO/issues/$ISSUE/comments" \ --jq ".[] | select(.body | startswith(\"$SENT\")) | .id"; } -SENTINEL_OK() { # $1 = text; true iff it opens with $SENT and carries a payload under it +SENTINEL_OK() { # $1 = text; true iff line 1 is exactly $SENT and >=16 payload bytes follow [ "$(printf '%s' "$1" | head -c ${#SENT})" = "$SENT" ] && - [ "$(printf '%s' "$1" | wc -c | tr -d ' ')" -ge $((${#SENT} + 17)) ] + [ "$(printf '%s' "$1" | tail -n +2 | wc -c | tr -d ' ')" -ge 16 ] } VERIFY() { # $1 = comment id; re-read what LANDED, whatever form the write took BACK="$(gh api "repos/$REPO/issues/comments/$1" --jq '.body' 2>/dev/null | tr -d '\r')" && @@ -97,16 +97,17 @@ else LIST=$(LOOKUP) || LIST="" # re-list; a failure here converges next cycle fi CANON=$(printf '%s\n' "$LIST" | sort -n | head -n1) - if [ -n "$CANON" ]; then - gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" - VERIFY "$CANON" || - echo "telemetry: comment $CANON does NOT carry a well-formed telemetry body after the write - treat the lane as UNREPORTED and record it in durable state; do not trust the timestamp" >&2 + if [ -z "$CANON" ]; then + echo "telemetry: no sentinel-prefixed comment to write to - treat the lane as UNREPORTED and carry that forward to the next cycle" >&2 + elif ! gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" >/dev/null; then + echo "telemetry: the PATCH of comment $CANON failed - treat the lane as UNREPORTED and carry that forward to the next cycle; the comment still holds an earlier cycle" >&2 + elif ! VERIFY "$CANON"; then + echo "telemetry: comment $CANON does NOT carry a well-formed telemetry body after the write - treat the lane as UNREPORTED and carry that forward to the next cycle; do not trust the timestamp" >&2 + else for DUP in $(printf '%s\n' "$LIST" | sort -n | tail -n +2); do gh api -X PATCH "repos/$REPO/issues/comments/$DUP" \ -f body="Superseded duplicate - canonical telemetry comment: $CANON" || true done - else - echo "telemetry: the write left no sentinel-prefixed comment to verify - treat the lane as UNREPORTED and record it in durable state; the next cycle retries the create" >&2 fi fi ``` @@ -116,23 +117,32 @@ below it. The lookup matches on that prefix, so a body composed without it is no — it would never be found again, and the next cycle would post a second comment. Compose the sentinel into the file; do not rely on anything downstream to add it. -**Body gate and post-write verification (encoded above, #943).** Two halves, because they catch +**Body gate, write check, and read-back (encoded above, #943).** Three checks, because they catch different failures. The **pre-write** assertions run before any API call and reject a `$BODY_FILE` -that is empty, opens with a literal `@`, is not sentinel-prefixed, or carries under 16 bytes of -payload — the mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes skill -("Never pass a body as an `@path` string"). The **post-write** `VERIFY` re-reads what actually landed, -which is what catches the failure the pre-write half structurally cannot: a correctly composed file -sent through a body-value flag (`-f body=@"$BODY_FILE"`) rather than `-F body=@`, where `gh` transmits -the literal path and the file itself was never at fault. -On the first-ever upsert the same cycle's PATCH is what verifies the create, so a POST that lands -degraded carries no sentinel, the re-lookup finds nothing, and there is no comment to re-read. That -path reports UNREPORTED as well rather than passing silently, and the next cycle retries the create. - -A degraded body that lands still moves the comment's timestamp, so any check keying on `updatedAt` -reads the lane as **fresh** while it carries nothing. Refusing, or reporting the write UNREPORTED, -is what keeps that from passing silently. A refusal or a failed verification is a cycle that did not -report: record it in durable state so the next cycle sees it, since stderr does not survive the -session. Not replicated from the wrapper: the 64 KiB cap and the body-file containment checks. +that is empty, opens with a literal `@`, is not sentinel-prefixed, or carries under 16 payload bytes +below the sentinel — the mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes +skill ("Never pass a body as an `@path` string"). The floor is measured on everything below line 1, +so it matches the wrapper's `MIN_BODY_BYTES` byte-for-byte whether that line ends in LF or CRLF. The +**write's own exit status** is checked next: a PATCH that fails leaves the previous cycle's body in +place, which a read-back running regardless would happily accept. The **post-write** `VERIFY` then +re-reads what the write stored — the only check that sees a write which reported success and stored +something else: a mangled body, a concurrent overwrite, a deleted comment. It is also the half that +would have caught #943 itself, where the composed file was correct and the defect was the invocation +(`-f body=@FILE` transmits the literal path; this block only ever uses `-F body=@`). + +Every branch that ends without a verified body says so and skips the duplicate-supersede pass, so a +cycle whose own write is unproven never tombstones a racing session's comment. A degraded body that +does land still moves the comment's timestamp, so any consumer keying on that timestamp rather than +on the body reads the lane as **fresh** while it carries nothing — which is why a refusal, a failed +write, and a failed verification all have to be carried forward: stderr does not survive the session, +and the next cycle must see that this one did not report. A lane with durable loop state records it +there; a lane without one carries it in the cycle's own summary. + +Known limits, inherited from the wrapper: a PATCH that succeeds while storing the previous body still +verifies, and `VERIFY` asserts that *some* well-formed telemetry is present, not that *this* cycle's +write is what is present. Not replicated at all: the 64 KiB cap, the body-file containment checks, +retries, and the wrapper's distinct non-zero exit codes — every branch here exits 0 and reports +through stderr alone. **Creation race reconcile (encoded above).** Two sessions racing the first-ever upsert can both see an empty lookup and both POST, forking the singleton. The upsert converges every cycle From ddb50a58a9c148f3b96f0156899f5661c06f5454 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 31 Jul 2026 01:01:48 -0400 Subject: [PATCH 5/7] refactor(work-items): move work-loop's telemetry upsert to a reference spoke `work-loop`'s SKILL.md sat at 499 of its 500-line hard cap on main, so the #943 gate, write-status check, read-back, and their prose pushed it to 548 and the skill-quality gate failed. The upsert moves to `reference/telemetry-upsert.md`, mirroring how the sibling `babysit-loop` lane already carries the identical block, with SKILL.md keeping the telemetry home and durable-state contract and pointing at the spoke for the mechanism. SKILL.md is 437 lines; the block, its lane-instance validation, the body gate, and the race reconcile move verbatim. The rationale for inlining rather than calling `claude-ops`'s wrapper is now stated once, in the spoke, instead of in both places. Co-Authored-By: Claude Opus 5 (1M context) --- plugins/work-items/skills/work-loop/SKILL.md | 117 +---------------- .../work-loop/reference/telemetry-upsert.md | 121 ++++++++++++++++++ 2 files changed, 124 insertions(+), 114 deletions(-) create mode 100644 plugins/work-items/skills/work-loop/reference/telemetry-upsert.md diff --git a/plugins/work-items/skills/work-loop/SKILL.md b/plugins/work-items/skills/work-loop/SKILL.md index 5207c0a6c..7b9eaf1f3 100644 --- a/plugins/work-items/skills/work-loop/SKILL.md +++ b/plugins/work-items/skills/work-loop/SKILL.md @@ -68,120 +68,9 @@ The telemetry home is a **per-lane tracking issue in the target repository**, re config; default: the open issue titled `Lane telemetry: work-loop` (exact match), created through the seam `create-item` verb when absent (announce the creation). Maintain exactly ONE status comment on it **per lane instance**, sentinel-identified and edited in place (the `claude-ops` -lane-telemetry contract; one writer identity owns a marker). The upsert is inlined here because an -installed plugin cannot invoke a sibling plugin's scripts. - -**Resolve the lane instance first (#1295).** The marker names the *writer*, not the lane type — per -the convention's lane-instance identity rule. The id is `${user_config.lane_instance}`; a surviving -literal `${user_config.…}` placeholder means the key is unset, so fall back to the sanitized -lowercased hostname (headless-config floor: log the assumption). It is operator-supplied text about -to be interpolated into a shell string and a `jq` program, so it is validated and **rejected**, -never sanitized-and-continued. Substitute the resolved value for `` below; the check -runs **before** `MARKER` is built, because a lane that validates only in prose has documented a -guard that does not run: - -```bash -INSTANCE="" # ${user_config.lane_instance}, else `hostname` sanitized -[ -n "$INSTANCE" ] || INSTANCE="$(hostname | tr '[:upper:]' '[:lower:]' | tr -c 'a-z0-9-' '-')" -# ^[a-z0-9][a-z0-9-]{0,31}$ — empty, a leading hyphen, any other character, or -# over 32 chars is REJECTED, never trimmed into something that looks valid. -case "$INSTANCE" in -"" | -* | *[!a-z0-9-]*) - echo "telemetry: lane_instance '$INSTANCE' is not ^[a-z0-9][a-z0-9-]{0,31}\$; refusing to build a marker" >&2 - exit 1 - ;; -esac -[ "${#INSTANCE}" -le 32 ] || { - echo "telemetry: lane_instance '$INSTANCE' exceeds 32 characters; refusing to build a marker" >&2 - exit 1 -} -``` - -The hostname fallback is a *default*, not a sanitizer: the same gate validates it, so a hostname -that cannot produce a conforming id stops the lane rather than yielding a marker nobody chose. - -```bash -MARKER="work-items:work-loop@$INSTANCE" -SENT="" # $BODY_FILE MUST open with this line -LOOKUP() { gh api --paginate "repos/$REPO/issues/$ISSUE/comments" \ - --jq ".[] | select(.body | startswith(\"$SENT\")) | .id"; } -SENTINEL_OK() { # $1 = text; true iff line 1 is exactly $SENT and >=16 payload bytes follow - [ "$(printf '%s' "$1" | head -c ${#SENT})" = "$SENT" ] && - [ "$(printf '%s' "$1" | tail -n +2 | wc -c | tr -d ' ')" -ge 16 ] -} -VERIFY() { # $1 = comment id; re-read what LANDED, whatever form the write took - BACK="$(gh api "repos/$REPO/issues/comments/$1" --jq '.body' 2>/dev/null | tr -d '\r')" && - SENTINEL_OK "$BACK" -} -if [ ! -s "$BODY_FILE" ] || [ "$(head -c 1 "$BODY_FILE")" = "@" ]; then - echo "telemetry: body is empty or a literal @path - nothing written; fix the body composition, do not re-run blind" >&2 -elif ! SENTINEL_OK "$(cat "$BODY_FILE")"; then - echo "telemetry: body is not sentinel-prefixed or carries no payload - nothing written; fix the body composition, do not re-run blind" >&2 -elif ! LIST=$(LOOKUP); then - echo "telemetry: comment lookup failed; skipping upsert this cycle (fail closed)" >&2 -else - if [ -z "$LIST" ]; then - gh api -X POST "repos/$REPO/issues/$ISSUE/comments" -F body=@"$BODY_FILE" >/dev/null - LIST=$(LOOKUP) || LIST="" # re-list; a failure here converges next cycle - fi - CANON=$(printf '%s\n' "$LIST" | sort -n | head -n1) - if [ -z "$CANON" ]; then - echo "telemetry: no sentinel-prefixed comment to write to - treat the lane as UNREPORTED and carry that forward to the next cycle" >&2 - elif ! gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" >/dev/null; then - echo "telemetry: the PATCH of comment $CANON failed - treat the lane as UNREPORTED and carry that forward to the next cycle; the comment still holds an earlier cycle" >&2 - elif ! VERIFY "$CANON"; then - echo "telemetry: comment $CANON does NOT carry a well-formed telemetry body after the write - treat the lane as UNREPORTED and carry that forward to the next cycle; do not trust the timestamp" >&2 - else - for DUP in $(printf '%s\n' "$LIST" | sort -n | tail -n +2); do - gh api -X PATCH "repos/$REPO/issues/comments/$DUP" \ - -f body="Superseded duplicate - canonical telemetry comment: $CANON" || true - done - fi -fi -``` - -**`$BODY_FILE` contract.** The file's FIRST line must be exactly `$SENT`, with the cycle's telemetry -below it. The lookup matches on that prefix, so a body composed without it is not merely rejected here -— it would never be found again, and the next cycle would post a second comment. Compose the sentinel -into the file; do not rely on anything downstream to add it. - -**Body gate, write check, and read-back (encoded above, #943).** Three checks, because they catch -different failures. The **pre-write** assertions run before any API call and reject a `$BODY_FILE` -that is empty, opens with a literal `@`, is not sentinel-prefixed, or carries under 16 payload bytes -below the sentinel — the mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes -skill ("Never pass a body as an `@path` string"). The floor is measured on everything below line 1, -so it matches the wrapper's `MIN_BODY_BYTES` byte-for-byte whether that line ends in LF or CRLF. The -**write's own exit status** is checked next: a PATCH that fails leaves the previous cycle's body in -place, which a read-back running regardless would happily accept. The **post-write** `VERIFY` then -re-reads what the write stored — the only check that sees a write which reported success and stored -something else: a mangled body, a concurrent overwrite, a deleted comment. It is also the half that -would have caught #943 itself, where the composed file was correct and the defect was the invocation -(`-f body=@FILE` transmits the literal path; this block only ever uses `-F body=@`). - -Every branch that ends without a verified body says so and skips the duplicate-supersede pass, so a -cycle whose own write is unproven never tombstones a racing session's comment. A degraded body that -does land still moves the comment's timestamp, so any consumer keying on that timestamp rather than -on the body reads the lane as **fresh** while it carries nothing — which is why a refusal, a failed -write, and a failed verification all have to be carried forward: stderr does not survive the session, -and the next cycle must see that this one did not report. A lane with durable loop state records it -there; a lane without one carries it in the cycle's own summary. - -Known limits, inherited from the wrapper: a PATCH that succeeds while storing the previous body still -verifies, and `VERIFY` asserts that *some* well-formed telemetry is present, not that *this* cycle's -write is what is present. Not replicated at all: the 64 KiB cap, the body-file containment checks, -retries, and the wrapper's distinct non-zero exit codes — every branch here exits 0 and reports -through stderr alone. - -**Creation race reconcile (encoded above).** Two sessions racing the first-ever upsert can both -see an empty lookup and both POST, forking the singleton. The upsert converges every cycle -duplicates are visible: the LOWEST comment id is canonical (numeric sort, deterministic for -every session), the canonical comment receives the current cycle's full state, and every other -sentinel comment is edited to a one-line tombstone so it never matches a lookup again — this -covers a racer that died between its POST and its own re-list, because the NEXT session's -ordinary upsert performs the same reconcile. A crashed racer's unmerged counters are an -accepted loss (durable state re-derives over a cycle); nothing is deleted. The reconcile converges -duplicates **within one instance's own sentinel set** — a sibling instance's comment carries a -different marker and never enters `$LIST`, so it is neither canonical nor tombstoned. +lane-telemetry contract; one writer identity owns a marker). The upsert itself — lane-instance resolution and validation, the singleton lookup, the body gate, the +write-status check and read-back, the POST/PATCH, and the creation-race reconcile — is owned by +[reference/telemetry-upsert.md](reference/telemetry-upsert.md). When the bound provider is not `github`, this upsert is unavailable: carry the same telemetry content — state block included — in the lane's cycle report/log, noting the comment surface is diff --git a/plugins/work-items/skills/work-loop/reference/telemetry-upsert.md b/plugins/work-items/skills/work-loop/reference/telemetry-upsert.md new file mode 100644 index 000000000..d73b86e10 --- /dev/null +++ b/plugins/work-items/skills/work-loop/reference/telemetry-upsert.md @@ -0,0 +1,121 @@ +# Telemetry comment upsert (per-instance singleton, race-converging) + +The exact upsert this lane runs to maintain its ONE sentinel-identified status comment **for this +lane instance**. `SKILL.md`'s "Telemetry and durable loop state" owns where the comment lives and +what goes in it; this file owns how the singleton is maintained, how its body is gated and verified, +and how a creation race converges. + +The upsert is inlined in this plugin rather than invoked from `claude-ops` because an installed +plugin cannot invoke a sibling plugin's scripts. + +**Resolve the lane instance first (#1295).** The marker names the *writer*, not the lane type — per +the convention's lane-instance identity rule. The id is `${user_config.lane_instance}`; a surviving +literal `${user_config.…}` placeholder means the key is unset, so fall back to the sanitized +lowercased hostname (headless-config floor: log the assumption). It is operator-supplied text about +to be interpolated into a shell string and a `jq` program, so it is validated and **rejected**, +never sanitized-and-continued. Substitute the resolved value for `` below; the check +runs **before** `MARKER` is built, because a lane that validates only in prose has documented a +guard that does not run: + +```bash +INSTANCE="" # ${user_config.lane_instance}, else `hostname` sanitized +[ -n "$INSTANCE" ] || INSTANCE="$(hostname | tr '[:upper:]' '[:lower:]' | tr -c 'a-z0-9-' '-')" +# ^[a-z0-9][a-z0-9-]{0,31}$ — empty, a leading hyphen, any other character, or +# over 32 chars is REJECTED, never trimmed into something that looks valid. +case "$INSTANCE" in +"" | -* | *[!a-z0-9-]*) + echo "telemetry: lane_instance '$INSTANCE' is not ^[a-z0-9][a-z0-9-]{0,31}\$; refusing to build a marker" >&2 + exit 1 + ;; +esac +[ "${#INSTANCE}" -le 32 ] || { + echo "telemetry: lane_instance '$INSTANCE' exceeds 32 characters; refusing to build a marker" >&2 + exit 1 +} +``` + +The hostname fallback is a *default*, not a sanitizer: the same gate validates it, so a hostname +that cannot produce a conforming id stops the lane rather than yielding a marker nobody chose. + +```bash +MARKER="work-items:work-loop@$INSTANCE" +SENT="" # $BODY_FILE MUST open with this line +LOOKUP() { gh api --paginate "repos/$REPO/issues/$ISSUE/comments" \ + --jq ".[] | select(.body | startswith(\"$SENT\")) | .id"; } +SENTINEL_OK() { # $1 = text; true iff line 1 is exactly $SENT and >=16 payload bytes follow + [ "$(printf '%s' "$1" | head -c ${#SENT})" = "$SENT" ] && + [ "$(printf '%s' "$1" | tail -n +2 | wc -c | tr -d ' ')" -ge 16 ] +} +VERIFY() { # $1 = comment id; re-read what LANDED, whatever form the write took + BACK="$(gh api "repos/$REPO/issues/comments/$1" --jq '.body' 2>/dev/null | tr -d '\r')" && + SENTINEL_OK "$BACK" +} +if [ ! -s "$BODY_FILE" ] || [ "$(head -c 1 "$BODY_FILE")" = "@" ]; then + echo "telemetry: body is empty or a literal @path - nothing written; fix the body composition, do not re-run blind" >&2 +elif ! SENTINEL_OK "$(cat "$BODY_FILE")"; then + echo "telemetry: body is not sentinel-prefixed or carries no payload - nothing written; fix the body composition, do not re-run blind" >&2 +elif ! LIST=$(LOOKUP); then + echo "telemetry: comment lookup failed; skipping upsert this cycle (fail closed)" >&2 +else + if [ -z "$LIST" ]; then + gh api -X POST "repos/$REPO/issues/$ISSUE/comments" -F body=@"$BODY_FILE" >/dev/null + LIST=$(LOOKUP) || LIST="" # re-list; a failure here converges next cycle + fi + CANON=$(printf '%s\n' "$LIST" | sort -n | head -n1) + if [ -z "$CANON" ]; then + echo "telemetry: no sentinel-prefixed comment to write to - treat the lane as UNREPORTED and carry that forward to the next cycle" >&2 + elif ! gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" >/dev/null; then + echo "telemetry: the PATCH of comment $CANON failed - treat the lane as UNREPORTED and carry that forward to the next cycle; the comment still holds an earlier cycle" >&2 + elif ! VERIFY "$CANON"; then + echo "telemetry: comment $CANON does NOT carry a well-formed telemetry body after the write - treat the lane as UNREPORTED and carry that forward to the next cycle; do not trust the timestamp" >&2 + else + for DUP in $(printf '%s\n' "$LIST" | sort -n | tail -n +2); do + gh api -X PATCH "repos/$REPO/issues/comments/$DUP" \ + -f body="Superseded duplicate - canonical telemetry comment: $CANON" || true + done + fi +fi +``` + +**`$BODY_FILE` contract.** The file's FIRST line must be exactly `$SENT`, with the cycle's telemetry +below it. The lookup matches on that prefix, so a body composed without it is not merely rejected here +— it would never be found again, and the next cycle would post a second comment. Compose the sentinel +into the file; do not rely on anything downstream to add it. + +**Body gate, write check, and read-back (encoded above, #943).** Three checks, because they catch +different failures. The **pre-write** assertions run before any API call and reject a `$BODY_FILE` +that is empty, opens with a literal `@`, is not sentinel-prefixed, or carries under 16 payload bytes +below the sentinel — the mechanical form of the `@path`-as-body rule owned by the `claude-ops` lanes +skill ("Never pass a body as an `@path` string"). The floor is measured on everything below line 1, +so it matches the wrapper's `MIN_BODY_BYTES` byte-for-byte whether that line ends in LF or CRLF. The +**write's own exit status** is checked next: a PATCH that fails leaves the previous cycle's body in +place, which a read-back running regardless would happily accept. The **post-write** `VERIFY` then +re-reads what the write stored — the only check that sees a write which reported success and stored +something else: a mangled body, a concurrent overwrite, a deleted comment. It is also the half that +would have caught #943 itself, where the composed file was correct and the defect was the invocation +(`-f body=@FILE` transmits the literal path; this block only ever uses `-F body=@`). + +Every branch that ends without a verified body says so and skips the duplicate-supersede pass, so a +cycle whose own write is unproven never tombstones a racing session's comment. A degraded body that +does land still moves the comment's timestamp, so any consumer keying on that timestamp rather than +on the body reads the lane as **fresh** while it carries nothing — which is why a refusal, a failed +write, and a failed verification all have to be carried forward: stderr does not survive the session, +and the next cycle must see that this one did not report. A lane with durable loop state records it +there; a lane without one carries it in the cycle's own summary. + +Known limits, inherited from the wrapper: a PATCH that succeeds while storing the previous body still +verifies, and `VERIFY` asserts that *some* well-formed telemetry is present, not that *this* cycle's +write is what is present. Not replicated at all: the 64 KiB cap, the body-file containment checks, +retries, and the wrapper's distinct non-zero exit codes — every branch here exits 0 and reports +through stderr alone. + +**Creation race reconcile (encoded above).** Two sessions racing the first-ever upsert can both +see an empty lookup and both POST, forking the singleton. The upsert converges every cycle +duplicates are visible: the LOWEST comment id is canonical (numeric sort, deterministic for +every session), the canonical comment receives the current cycle's full state, and every other +sentinel comment is edited to a one-line tombstone so it never matches a lookup again — this +covers a racer that died between its POST and its own re-list, because the NEXT session's +ordinary upsert performs the same reconcile. A crashed racer's unmerged counters are an +accepted loss (durable state re-derives over a cycle); nothing is deleted. The reconcile converges +duplicates **within one instance's own sentinel set** — a sibling instance's comment carries a +different marker and never enters `$LIST`, so it is neither canonical nor tombstoned. From 80eec2a1f6c4555924095b82a10e85b4cff88485 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 31 Jul 2026 01:03:24 -0400 Subject: [PATCH 6/7] docs(work-items): record the work-loop telemetry spoke move in the changelog The file move is a visible change to the skill's shape, not just an internal tidy: a reader who knows where the upsert lived needs to be told it moved. Co-Authored-By: Claude Opus 5 (1M context) --- plugins/work-items/CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/plugins/work-items/CHANGELOG.md b/plugins/work-items/CHANGELOG.md index bd050179c..9acec7042 100644 --- a/plugins/work-items/CHANGELOG.md +++ b/plugins/work-items/CHANGELOG.md @@ -33,6 +33,14 @@ All notable changes to the `work-items` plugin are documented here. Format follo storing the previous body still verifies, and the read-back proves *some* well-formed telemetry is present, not *this* cycle's. Not replicated at all: the 64 KiB cap, the containment checks, retries, and the wrapper's distinct non-zero exits — every inline branch exits 0. +- **`work-loop`'s telemetry upsert moves to `reference/telemetry-upsert.md`.** SKILL.md sat at 499 of + its 500-line hard cap, so the checks above did not fit. The upsert — lane-instance resolution and + validation, the singleton lookup, the body gate, the write-status check and read-back, the + POST/PATCH, and the creation-race reconcile — moves verbatim into a spoke, the same shape the + sibling `source-control:babysit-loop` lane already uses for the identical block. SKILL.md keeps the + telemetry home and the durable-state contract and points at the spoke for the mechanism; the + rationale for inlining rather than calling `claude-ops`'s wrapper is now stated once instead of + twice. ## [0.31.0] From 60d6860b9a42c1a4406464010a7debddbf281ecc Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 31 Jul 2026 01:07:54 -0400 Subject: [PATCH 7/7] fix(loop-lane): make the notices and the changelogs true of the branches they describe Follow-ups from the re-review, all wording or fail-safety, no behavior change on the happy path: - The claude-ops changelog still described two checks and two not-replicated items while the lanes skill it summarizes had moved to three and four. A reader of the changelog alone could not learn that an inline branch always exits 0, which is the deferral most likely to bite a caller. Mirrored, with the inherited limits named. - source-control's entry still said "both halves". - Two notices were false on a reachable sub-branch. An empty $CANON can also mean the create landed and the re-list missed it, so the notice no longer claims there is nothing there. A failed PATCH leaves a body that may be several cycles old, not "an earlier cycle". - The bare POST gets `|| true`. Under a `set -e` wrapper a failed create aborted the block before any notice, which would have made the "every branch exits 0 and reports" claim false exactly when it matters. - The race-reconcile paragraph now says tombstoning waits on the canonical write verifying, which is what the code does. Co-Authored-By: Claude Opus 5 (1M context) --- plugins/claude-ops/CHANGELOG.md | 12 ++++++++---- plugins/source-control/CHANGELOG.md | 2 +- .../babysit-loop/reference/telemetry-upsert.md | 9 +++++---- plugins/work-items/skills/attend-queue/SKILL.md | 9 +++++---- .../skills/work-loop/reference/telemetry-upsert.md | 9 +++++---- 5 files changed, 24 insertions(+), 17 deletions(-) diff --git a/plugins/claude-ops/CHANGELOG.md b/plugins/claude-ops/CHANGELOG.md index e17c255ca..0ae18829d 100644 --- a/plugins/claude-ops/CHANGELOG.md +++ b/plugins/claude-ops/CHANGELOG.md @@ -11,10 +11,14 @@ All notable changes to the `claude-ops` plugin are documented here. Format follo trust — and corrects which consumer the failure actually deceives (#943).** The rule's closing paragraph claimed the prose was "the only thing standing between" an inlined upsert and a silent observability fail-open. That is no longer true: every lane that inlines the `gh api` upsert — - `source-control:babysit-loop`, `work-items:work-loop`, `work-items:attend-queue` — now carries both - a pre-write body gate and a post-write read-back in its own block. The paragraph states which - guarantees travel inline (both halves) and which do not (the 64 KiB cap, the body-file containment - checks). + `source-control:babysit-loop`, `work-items:work-loop`, `work-items:attend-queue` — now carries three + checks in its own block: a pre-write body gate, a check of the write's own exit status, and a + post-write read-back of what the write stored. The paragraph states which guarantees travel inline + (those three) and which do not: the 64 KiB cap, the body-file containment checks, retries, and this + script's distinct non-zero exit codes — an inline branch always exits 0 and reports through stderr, + so a caller cannot detect a failed cycle from its exit status. It also names the limits an inline + block inherits rather than fixes: a PATCH that succeeds while storing the previous body still + verifies, and the read-back proves *some* well-formed telemetry is present, not *this* cycle's. - **Corrected: `morning-brief` is not the check a degraded telemetry body deceives.** The rule said a freshness check "passes over a blind lane". Verified against `morning-brief.sh`'s `print_telemetry`: it parses `lane:` and `last-cycle:` out of the comment BODY, so an `@path` body carries no `lane:` diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index 6700315a3..2c06805d3 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -13,7 +13,7 @@ All notable changes to the `source-control` plugin are documented here. Format f field=@file`, so an `@path` passed as a body VALUE is transmitted as text. `claude-ops`'s `telemetry-upsert.sh` refuses such a body and re-reads what it wrote, but an installed plugin cannot invoke a sibling plugin's script, so this lane inlines its own upsert and inherited neither - protection. The block now carries both halves, which catch different failures. A **pre-write gate** + protection. The block now carries three checks, which catch different failures. A **pre-write gate** rejects a `$BODY_FILE` that is empty, opens with a literal `@`, is not sentinel-prefixed, or holds under 16 bytes of payload — no POST, no PATCH. The **write's own exit status** is then checked, because a failed PATCH leaves the previous cycle's body in place and a read-back running regardless would diff --git a/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md b/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md index 23eb89fa1..ce9eb2ddc 100644 --- a/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md +++ b/plugins/source-control/skills/babysit-loop/reference/telemetry-upsert.md @@ -54,14 +54,14 @@ elif ! LIST=$(LOOKUP); then echo "telemetry: comment lookup failed; skipping upsert this cycle (fail closed)" >&2 else if [ -z "$LIST" ]; then - gh api -X POST "repos/$REPO/issues/$ISSUE/comments" -F body=@"$BODY_FILE" >/dev/null + gh api -X POST "repos/$REPO/issues/$ISSUE/comments" -F body=@"$BODY_FILE" >/dev/null || true LIST=$(LOOKUP) || LIST="" # re-list; a failure here converges next cycle fi CANON=$(printf '%s\n' "$LIST" | sort -n | head -n1) if [ -z "$CANON" ]; then - echo "telemetry: no sentinel-prefixed comment to write to - treat the lane as UNREPORTED and carry that forward to the next cycle" >&2 + echo "telemetry: no comment available to write to (a create may have landed but was not re-found) - treat the lane as UNREPORTED and carry that forward to the next cycle" >&2 elif ! gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" >/dev/null; then - echo "telemetry: the PATCH of comment $CANON failed - treat the lane as UNREPORTED and carry that forward to the next cycle; the comment still holds an earlier cycle" >&2 + echo "telemetry: the PATCH of comment $CANON failed - treat the lane as UNREPORTED and carry that forward to the next cycle; the comment holds an earlier body, not this cycle's write" >&2 elif ! VERIFY "$CANON"; then echo "telemetry: comment $CANON does NOT carry a well-formed telemetry body after the write - treat the lane as UNREPORTED and carry that forward to the next cycle; do not trust the timestamp" >&2 else @@ -109,7 +109,8 @@ through stderr alone. see an empty lookup and both POST, forking the singleton. The upsert converges every cycle duplicates are visible: the LOWEST comment id is canonical (numeric sort, deterministic for every session), the canonical comment receives the current cycle's full state, and every other -sentinel comment is edited to a one-line tombstone so it never matches a lookup again — this +sentinel comment is edited to a one-line tombstone — only once the canonical write verifies — so +it never matches a lookup again — this covers a racer that died between its POST and its own re-list, because the NEXT session's ordinary upsert performs the same reconcile. A crashed racer's unmerged counters are an accepted loss (durable state re-derives over a cycle); nothing is deleted. The reconcile converges diff --git a/plugins/work-items/skills/attend-queue/SKILL.md b/plugins/work-items/skills/attend-queue/SKILL.md index a20373112..db954468b 100644 --- a/plugins/work-items/skills/attend-queue/SKILL.md +++ b/plugins/work-items/skills/attend-queue/SKILL.md @@ -153,14 +153,14 @@ elif ! LIST=$(LOOKUP); then echo "telemetry: comment lookup failed; skipping upsert this cycle (fail closed)" >&2 else if [ -z "$LIST" ]; then - gh api -X POST "repos/$REPO/issues/$ISSUE/comments" -F body=@"$BODY_FILE" >/dev/null + gh api -X POST "repos/$REPO/issues/$ISSUE/comments" -F body=@"$BODY_FILE" >/dev/null || true LIST=$(LOOKUP) || LIST="" # re-list; a failure here converges next cycle fi CANON=$(printf '%s\n' "$LIST" | sort -n | head -n1) if [ -z "$CANON" ]; then - echo "telemetry: no sentinel-prefixed comment to write to - treat the lane as UNREPORTED and carry that forward to the next cycle" >&2 + echo "telemetry: no comment available to write to (a create may have landed but was not re-found) - treat the lane as UNREPORTED and carry that forward to the next cycle" >&2 elif ! gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" >/dev/null; then - echo "telemetry: the PATCH of comment $CANON failed - treat the lane as UNREPORTED and carry that forward to the next cycle; the comment still holds an earlier cycle" >&2 + echo "telemetry: the PATCH of comment $CANON failed - treat the lane as UNREPORTED and carry that forward to the next cycle; the comment holds an earlier body, not this cycle's write" >&2 elif ! VERIFY "$CANON"; then echo "telemetry: comment $CANON does NOT carry a well-formed telemetry body after the write - treat the lane as UNREPORTED and carry that forward to the next cycle; do not trust the timestamp" >&2 else @@ -208,7 +208,8 @@ through stderr alone. see an empty lookup and both POST, forking the singleton. The upsert converges every cycle duplicates are visible: the LOWEST comment id is canonical (numeric sort, deterministic for every session), the canonical comment receives the current cycle's full state, and every other -sentinel comment is edited to a one-line tombstone so it never matches a lookup again — this +sentinel comment is edited to a one-line tombstone — only once the canonical write verifies — so +it never matches a lookup again — this covers a racer that died between its POST and its own re-list, because the NEXT session's ordinary upsert performs the same reconcile. A crashed racer's unmerged counters are an accepted loss (durable state re-derives over a cycle); nothing is deleted. The reconcile converges diff --git a/plugins/work-items/skills/work-loop/reference/telemetry-upsert.md b/plugins/work-items/skills/work-loop/reference/telemetry-upsert.md index d73b86e10..832c315cf 100644 --- a/plugins/work-items/skills/work-loop/reference/telemetry-upsert.md +++ b/plugins/work-items/skills/work-loop/reference/telemetry-upsert.md @@ -58,14 +58,14 @@ elif ! LIST=$(LOOKUP); then echo "telemetry: comment lookup failed; skipping upsert this cycle (fail closed)" >&2 else if [ -z "$LIST" ]; then - gh api -X POST "repos/$REPO/issues/$ISSUE/comments" -F body=@"$BODY_FILE" >/dev/null + gh api -X POST "repos/$REPO/issues/$ISSUE/comments" -F body=@"$BODY_FILE" >/dev/null || true LIST=$(LOOKUP) || LIST="" # re-list; a failure here converges next cycle fi CANON=$(printf '%s\n' "$LIST" | sort -n | head -n1) if [ -z "$CANON" ]; then - echo "telemetry: no sentinel-prefixed comment to write to - treat the lane as UNREPORTED and carry that forward to the next cycle" >&2 + echo "telemetry: no comment available to write to (a create may have landed but was not re-found) - treat the lane as UNREPORTED and carry that forward to the next cycle" >&2 elif ! gh api -X PATCH "repos/$REPO/issues/comments/$CANON" -F body=@"$BODY_FILE" >/dev/null; then - echo "telemetry: the PATCH of comment $CANON failed - treat the lane as UNREPORTED and carry that forward to the next cycle; the comment still holds an earlier cycle" >&2 + echo "telemetry: the PATCH of comment $CANON failed - treat the lane as UNREPORTED and carry that forward to the next cycle; the comment holds an earlier body, not this cycle's write" >&2 elif ! VERIFY "$CANON"; then echo "telemetry: comment $CANON does NOT carry a well-formed telemetry body after the write - treat the lane as UNREPORTED and carry that forward to the next cycle; do not trust the timestamp" >&2 else @@ -113,7 +113,8 @@ through stderr alone. see an empty lookup and both POST, forking the singleton. The upsert converges every cycle duplicates are visible: the LOWEST comment id is canonical (numeric sort, deterministic for every session), the canonical comment receives the current cycle's full state, and every other -sentinel comment is edited to a one-line tombstone so it never matches a lookup again — this +sentinel comment is edited to a one-line tombstone — only once the canonical write verifies — so +it never matches a lookup again — this covers a racer that died between its POST and its own re-list, because the NEXT session's ordinary upsert performs the same reconcile. A crashed racer's unmerged counters are an accepted loss (durable state re-derives over a cycle); nothing is deleted. The reconcile converges