fix(core): artifact provenance, store-corruption guard, an opaque payload channel for run_task, and a runtime-free core

[jaruesink]

Five commits. The first was already reviewed; the rest landed on top. Every
one of them is the same defect class in a different place — a failed
operation rendering as an ordinary state of the world — which is why they
travel together.

	Commit	What
1	c4b3625	run_task artifact provenance — stop inventing commit SHAs out of agent prose
2	ce9607c	A failed store read must not destroy the store
3	57d3e72	An opaque payload channel for run_task
4	b0c65d5	Get the concrete runtime out of packages/core
5	b44ce7f	A payload write failure must fail the dispatch, not degrade silently

Important

The run_task declaration changed (change 3 adds a payload argument),
so toolsetVersion shifts and every connector holding an approved snapshot
needs a Refresh on its action configuration. ChatGPT freezes the catalog
approved at setup — restarting the service is not enough, and a newly added
action arrives disabled by default. Confirm with get_connection_info: a
client whose cached catalog disagrees with the server's toolsetVersion is
holding a stale snapshot rather than talking to a broken server.

---

1 — run_task artifact provenance (c4b3625)

The defect

extractPatternsFromSummary infers engineering identity by regex over a
free-form agent summary, and presents the guess as a fact. Three terminal job
records were observed carrying identifiers for work that never happened.

commitSha came from /\b([0-9a-f]{7,40})\b/ — the first hex-shaped word
anywhere in the summary. That matches:

what got recorded	what it actually was
an 8-char SHA	a precondition the summary restated — "expected head ``"
a 32-char hex run	the middle of a hyphenated session slug; - is a word boundary, so \b does not exclude it
an 11-digit number	a CI run id inside a pasted URL — 0-9 is a subset of the hex class, so any 7+ digit number is eligible

This is not cosmetic. deriveNextStep branches on artifacts.commitSha, so a
fabricated SHA suppressed the correct next step ("Review changes and commit.")
on a job that had changed files and committed nothing.

branchName came back as `feature-x`, — the capture class excluded
quotes but not backticks or sentence punctuation, so markdown rode along.

The fix

A SHA now requires a commit-ish claim. Accepted only from a GitHub commit
URL, or from a hex run that clears three checks: it stands alone (the
characters either side may not be -, _, /, or alphanumeric, which also
rejects runs longer than 40); it is introduced by an adjacent commit cue
(commit/committed/sha/HEAD, reachable across markdown and a linking
word); and it is not adjacent to a marker that makes it an input rather than
an output (expected, base, from, was, previous, parent).

The input-marker check is anchored to the hex run rather than scanning a fixed
character window, so an ordinary sentence — "the change was reviewed and
committed as `a1b2c3d4`" — still reads as a real commit even though it
contains "was".

branchName stops capturing markdown, and trailing .,;:)]} is stripped.

An inferred value now says it is inferred. Artifacts.provenance records
how each identifier was obtained:

export type ArtifactProvenance = "summary-text" | "command";

"command" means it was read off a command the agent actually ran (a
checkout -b in commandsRun); "summary-text" means it was scraped from
prose, which is a guess about what the prose meant. Command evidence now wins
where both exist — previously prose was consulted first and the command branch
never got a chance. The field is optional, so job records persisted before it
existed deserialize unchanged; no migration, no shim.

Readers surface it. The MCP and CLI JSON paths pass artifacts wholesale,
so provenance rides along. The one place these are rendered for a human —
packages/cli/src/output.ts — now marks a prose-derived value rather than
printing it beside command-derived ones as equally established.

---

2 — A failed store read must not destroy the store (ce9607c)

The defect

JsonFileJobStore.load() and JsonFileAttachmentStore.load() returned [] on
any read or parse error, logging one line to stderr. That is a lie when the
file exists but cannot be read, and a self-erasing one:
SessionManager.rehydrateFromStore rehydrates nothing, then
persistActiveJobs does a whole-map overwrite — so the first save after a
failed load writes the truncated set over the only file that could have shown
what was lost.

The store holds in-flight jobs for restart recovery, so the consequence was
that every in-flight job at restart was orphaned and silently unrecoverable,
and the only trace was a stderr line nobody reads. The attachment store has
the identical shape and is worse in one way: it persists every session that has
ever had an attachment, so a failed load discards lineage, not just active work.

The fix

Distinguish "not there" (empty is a fact — unchanged) from "could not be
read" (empty is a lie).

• An unreadable file is renamed aside as <file>.corrupt-<timestamp> before
  anything can overwrite it. The subsequent save is then harmless: it writes
  a fresh file and the preserved copy is untouched.
• If the preservation itself fails, the store refuses to save at all. Not
  persisting is recoverable; shredding the only copy is not.
• A file that parses but is not an array counts as unreadable too. It is
  something other than this store's contents, and answering [] for it erases
  it exactly the way a syntax error did.

Where it becomes visible. Nothing existing was suitable — TelemetryEvent
is structurally scoped to the task-contract tools and has no shape for this —
so the store takes an onDegraded sink, GatewayPool collects what it
reports, and get_connection_info serves it as degradedStores. That is the
tool a supervisor already calls when something looks inconsistent, and its
description now says so. The field is omitted entirely when nothing
degraded, so its presence is the signal.

Both stores are kept identical. No conflict with how SessionManager uses
them surfaced: it only ever calls load() once at construction and save()
with the current set, and neither contract changed.

---

3 — An opaque payload channel for run_task (57d3e72)

The defect, reproduced twice on real dispatches

task and context are delivered to the agent as one conversational
message. There is no way to say "this part is not addressed to you."

A brief of the form "you are the manager; write the context to a file; then
launch the worker" therefore reaches the manager AND — because the manager
faithfully passes the whole brief onward — reaches the worker, which reads
the same manager instructions, concludes it is the manager, and launches
another worker. Observed on two independent dispatches: the worker's first
assistant message was "I'll write the prompt file, then launch...", followed
by a large file write and a shell launch. Manager-ness is contagious down the
chain, and every status surface above the worker looked healthy throughout.

The fix — content never enters the instruction stream

run_task gains an optional payload. The server materialises it to a file
and tells the agent only the path. The bytes never appear in the
conversation, which is what makes this structural rather than a wording fix.

• Opaque. Never parsed, interpreted, templated, truncated, or echoed.
• On disk, mode 0600, under a dedicated directory (default
  ~/.clawconnect/payloads/, overridable via payloadDir / CLAWCONNECT_PAYLOAD_DIR,
  following how the job/attachment store directories are already configured).
  Named by job id.
• The delivered message gains only a short fixed note naming the path and
  saying plainly that the contents are opaque, not addressed to the agent, and
  not to be acted on. Two sentences — it is a delivery note, not instructions.
• payloadPath is recorded on the job and returned by get_task, so a
  supervisor can see a payload existed and where it went. No read tool returns
  the contents, including get_task detail="prompt".
• Retention is TTL-based (24h), never terminal-based. The downstream worker
  routinely outlives the job that launched it — that is what a delegated
  handoff is, and it is the case that motivated this — so deleting on job
  completion would pull the file out from under a live reader. The sweep runs
  at startup and opportunistically on write, is rate-limited, and can never
  fail a dispatch.

Deliberately the generic shape: ClawConnect does not launch the downstream
worker and must not know what will consume the payload. It provides a side
channel and a path; who reads it, and how, is the caller's and the agent's
business.

payload is optional, so every existing caller keeps a byte-identical message.
It is declared once in the shared capability surface, so both transports serve
it — test/surface-parity.test.ts covers that, and now also asserts the
argument's presence and shape on both.

---

4 — Get the concrete runtime out of packages/core (b0c65d5)

Why

packages/core/src/runtime-modules.ts states the design intent plainly: the
seam exists "without teaching ClawConnect anything about any particular
runtime… A host's own runtime bridge is one such module; ClawConnect neither
ships it nor knows it exists."

That is not what the code did. packages/core shipped LocalTmuxFleetAdapter
— shelling out to tmux, hard-coding the ~/.claude-fleet/<handle>/meta.json
convention — plus CLAUDE_FLEET_RUNTIME_ID, a FleetAdapter interface
threaded through GatewayPool, a "fleet-transcript" literal in the core
ResultSource type, and a "claude-fleet" default for any attach directive
that named no runtime. Both entrypoints constructed the adapter by default.
So core knew about exactly one runtime while claiming to know about none.

Verified safe: no tmux sessions on the live deployment,
CLAWCONNECT_AGENT_SESSION_RUNTIME_MODULES unset, and the only attachment
records in the live store are August smoke tests already status: "detached".

What changed

The adapter moves to examples/local-tmux-runtime/ and reaches a
deployment through CLAWCONNECT_AGENT_SESSION_RUNTIME_MODULES like any host
module. Core keeps only the neutral registry, the attachment model, and the
callback seam. Both entrypoints now register nothing by default — asserted
directly, since an entrypoint quietly constructing a runtime is precisely what
made the old claim false.

No shims, no flags, no runtime guards.

Judgement call 1 — the legacy transcript path collapses into "agent-session"

ResultSource loses "fleet-transcript". That value existed because reading a
Claude Code transcript off disk is a stronger provenance claim than an
arbitrary runtime's reply.

That gate is not lost — it was never in core. It lived inside the adapter
(the tmux pane must have ended, and the transcript entry must carry its own
timestamp) and it still lives inside the module, which is where the evidence
is. Core keeps every check it can actually make: the turn must be a
completed one, the answer must be datable, it must post-date the job it would
answer, and it must survive the compare-and-set.

What core cannot do is verify a runtime's evidentiary claim, and a fixed enum
restating an unverifiable claim is worse than not making it — it reads as
established where it is hearsay. A reader who wants to know what answered reads
agentSession.runtime on the same snapshot, which names the actual runtime
rather than a category. That is strictly more precise than the enum it replaces.

Judgement call 2 — where the adapter lives: an example, not a package or a deletion

Git history is a sufficient record of code, but not of the seam being
usable. The repo documents CLAWCONNECT_AGENT_SESSION_RUNTIME_MODULES and
shipped no worked example, and this repo's own rule is that a documented
integration the shipped binary cannot perform is the exact bug that mechanism
exists to fix. Deleting outright would have left that gap wider.

A package was the wrong weight: build config, exports and workspace
membership for something core explicitly says it does not ship. So it is an
example — and deliberately plain JavaScript with no build step and no
import from @clawconnect/core. An operator points the env var straight at
runtime.mjs and it loads. That is the honest demonstration: a runtime module
needs nothing from ClawConnect but the registry object it is handed.

Its tests came along (converted to drive inspect through the seam) and live
in test/, because examples/ is not a workspace package and a test file
outside every project is not picked up by the default vp test run — a check
nobody runs is a check that does not exist.

One extra change this forced

An attach/replace directive must now name its runtime. It defaulted to
"claude-fleet", which is how a hardcoded runtime id survived in the layer
whose whole claim is that it knows none — and with no built-in adapter, that
default would attach a session to something nobody registered. The neutral
<agent-session> marker has always required runtime, so this makes the
two agree rather than inventing a rule. An id nobody registered still reads
back as a normalized unknown_runtime result, never an error.

GatewayPool and the neutral runtime path are otherwise unchanged, and their
tests still pass. The recovery tests that drove core's rules through the
adapter now drive the same rules through a registered runtime, which is the
only path there is.

---

5 — A payload write failure must fail the dispatch (b44ce7f)

The defect

Change 3 above shipped FilePayloadStore.write() returning undefined on
failure and logging only to stderr. submitTask then dispatched the task with
no payload note at all — indistinguishable from a task that never had a
payload. The task text routinely names the file, so the agent was handed a
brief referencing data it could not find, and the only trace was a line in a
log nobody reads.

The comment defended this as best-effort. That conflated two opposite cases:

	Load-bearing?	Correct behaviour
sweep	No — nobody asked for it, nothing reads its result	Silent, rate-limited, must never block a dispatch
write	Yes, by construction — the caller explicitly passed a payload	Fail the dispatch

Which makes it the same defect as the other four: artifacts.ts stopped
presenting a guess as a fact, the store guard stopped a failed read reading as
"empty", and this one still let a failed write read as "no payload was passed".
Failing is recoverable — the caller retries. Silently degrading is not.

The fix

write returns string and throws. The string | undefined return type
was itself the invitation; there is no longer a "returned nothing" branch to
carry on from. A job id failing SAFE_ID_RE throws too — ids are minted
internally, so that is an invariant violation in this process, and returning
undefined hid a bug in id minting behind a merely-absent payload. A failed
write also unlinks whatever reached disk: the dispatch is being refused, so the
file is unreferenced garbage, and a chmod that failed would otherwise leave a
payload with looser permissions than one may keep.

submitTask refuses through the same rejection path a "session busy"
collision already used, so both transports surface it identically — run_task
throws at the tool boundary and the capability layer renders an isError
result. A caller can tell "your payload could not be stored" from "that agent
is busy" by the message, and the error states that nothing is running so a
retry is not a duplicate submit.

That path is now one implementation rather than two: rejectSubmit, which
deliberately touches neither the session's latest-job pointer, nor its history,
nor the attachment directive, nor persistActiveJobs. Nothing is dispatched
and no job is left half-created.

One case the brief did not name, fixed for the same reason: a payload
passed with no payload store configured at all previously produced no file,
silently. That is reachable — createMcpServer deliberately does not default a
payload directory — and it is the identical silent degradation, so it gets the
identical answer. The stale comment in server.ts promising the old behaviour
is corrected.

sweep is untouched. No fallback, no flag, no opt-in degrade mode.

---

Verification

$ pnpm run ready
✔ Build complete   (all 5 packages)

$ ./node_modules/.bin/vp test --run
 Test Files  34 passed (34)
      Tests  657 passed (657)

$ ./node_modules/.bin/vp test --run test/surface-parity.test.ts
 Test Files  1 passed (1)
      Tests  12 passed (12)

Lint is byte-identical to the pre-change baseline — 22 findings, of which 7
are the documented pre-existing TS2322 errors in
apps/chatgpt/src/widget/state.test.ts (collectFollowUpWakes' Map/Set
literals) and 15 are pre-existing warnings. Nothing new was introduced and
nothing pre-existing was absorbed.

New coverage

File	Covers
packages/core/src/payload-channel.test.ts	Payload written 0600; its contents absent from the actual text handed to the gateway; the path and not-addressed-to-you note present; no payload ⇒ byte-identical message; payloadPath on the job and in get_task; contents returned by no read tool; TTL sweep removes an old file and keeps a fresh one; a sweep failure never fails a dispatch. Plus (change 5): an unwritable payload dir refuses the task naming the payload and the underlying errno, dispatches nothing, and leaves no half-created job or persisted state; a missing payload store refuses identically; run_task throws rather than returning a jobId; an id failing the filename guard throws; and the same broken dir still dispatches an ordinary no-payload task
packages/core/src/store-health.test.ts	A degraded store reaches get_connection_info as degradedStores, and is omitted when healthy
packages/core/src/job-store.test.ts, attachment-store.test.ts	Missing ⇒ empty with no side effects; corrupt ⇒ preserved under a new name and a later save cannot destroy it; non-array treated as unreadable; valid file unchanged; save refused when preservation itself failed
test/example-local-tmux-runtime.test.ts	The example module through the seam: registers one runtime, offers inspect only, reports liveness with no state while the pane is up, a dated completed turn once it is gone, path-traversal containment, and abort behaviour
test/surface-parity.test.ts	payload served identically by both transports, and still optional

Not done, deliberately

• No deploy, no production change, no credential change. Not merged.
• The job store still writes into apps/chatgpt/.job-store, inside the repo
  working tree. Out of scope here — moving it needs a data migration on a live
  deployment.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your current included review allowance is based on your included PR review attempts over the past 7 days.

Next review available in: 28 minutes

Limit details: You’ve used the included review currently available. Your 68 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2f7a3a9f-17ee-445b-999d-2cbad50f5ab8

📥 Commits

Reviewing files that changed from the base of the PR and between 06c7771 and b44ce7f.

📒 Files selected for processing (38)

• AGENTS.md
• README.md
• apps/chatgpt/src/app.test.ts
• apps/chatgpt/src/app.ts
• docs/architecture/runtime-boundary.md
• docs/architecture/runtime-registration.md
• examples/local-tmux-runtime/README.md
• examples/local-tmux-runtime/runtime.mjs
• packages/cli/src/output.ts
• packages/core/README.md
• packages/core/src/agent-session-attachment.test.ts
• packages/core/src/artifacts.test.ts
• packages/core/src/artifacts.ts
• packages/core/src/attachment-store.test.ts
• packages/core/src/attachment-store.ts
• packages/core/src/capability.ts
• packages/core/src/fleet-adapter.test.ts
• packages/core/src/fleet-adapter.ts
• packages/core/src/gateway-pool.ts
• packages/core/src/index.ts
• packages/core/src/job-store.test.ts
• packages/core/src/job-store.ts
• packages/core/src/payload-channel.test.ts
• packages/core/src/payload-store.ts
• packages/core/src/recovery-liveness.test.ts
• packages/core/src/session-handoff.test.ts
• packages/core/src/session-handoff.ts
• packages/core/src/session.ts
• packages/core/src/store-health.test.ts
• packages/core/src/store-health.ts
• packages/core/src/structured-content.ts
• packages/core/src/tools.ts
• packages/core/src/types.ts
• packages/mcp/src/bin.ts
• packages/mcp/src/server.test.ts
• packages/mcp/src/server.ts
• test/example-local-tmux-runtime.test.ts
• test/surface-parity.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[jaruesink]

@coderabbitai review

The earlier review attempt hit a rate limit and the branch has advanced since. Current head is b44ce7f. Five commits: artifact provenance, store-corruption guard, opaque payload channel for run_task, runtime-free core, and a payload write-failure fix.

Note for reviewers: run_task gains an optional payload parameter, so the tool declaration changes and toolsetVersion will shift — connectors need a refresh after deploy.

[coderabbitai]

@jaruesink I will review the current changes at b44ce7f, including the run_task payload surface change and connector refresh impact.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.