Software factory change - #547
Draft
agent-relay-code[bot] wants to merge 2 commits into
Draft
agent-relay-code[bot] wants to merge 2 commits into
agent-relay-code[bot] wants to merge 2 commits into
Conversation
`ctx.done(reason)` took only the reason, so every authored `step_failed`
landed on one generic sentence — "No step failed, so there is no step-level
evidence to inspect". Cloud run f92bf832 finished that way after twenty
successful steps and an opened PR; the reviewer's actual finding ("one P2
remains: `review.clean` was not created") existed and reached no reader.
Finding it took opening the Codex reviewer's raw transcript.
`done()` now takes an optional `{ detail }`. It is normalized once, at
`done()`: redacted with the SDK's existing redactor, trimmed, and bounded to
2,000 Unicode code points including a fixed `… (truncated)` suffix. Redaction
runs BEFORE truncation, because cutting a secret in half leaves nothing for an
exact-match redaction to find — a truncation that causes the leak.
The detail travels inside the terminal marker's deterministic command, so it
is a durable journal fact and the IPC verifier's existing command comparison
attests it for free. It is carried on the authored root's output, read back by
a completed-root resume without recomputing it, and reported as
`completionDetail` plus the diagnostic's `detail`; for `step_failed` it
replaces the generic sentence. `flows status` prints it as a labelled
`authored done("...")` line beside — never instead of — the kernel's own
account, which stays `completed / success` because that is what happened.
No detail means no change: the marker command, the lowered spec, its canonical
hash, the report message and every JSON shape are byte-identical, pinned
against fixtures captured from the pre-change tree.
Kernel: no production change. The detail is opaque data in a shell string. A
regression pins the three kernel properties the SDK relies on — the marker
command survives journal persistence and reopen verbatim, re-admitting it
under the same admission key returns the same run, and changed marker data is
refused as `run_admission_conflict`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…rogates Two P2 findings from the review of #547. **F1 — a committed detail is recovered, not recomputed.** The terminal marker's command embeds the detail and the marker run is opened under a stable admission key, but normalization redacts against `process.env`. A credential rotated or removed while the process was down made the SAME authored sentence normalize differently, so a resumed body retried the marker's key with a drifted spec and the kernel refused it as `run_admission_conflict` — losing an explanation the root had already journaled, for a flow whose every step succeeded. The verdict is now appended to a stream on the root BEFORE the marker run is opened, and a later attempt reaching the same terminal step reuses what was recorded. That is the durability the executor already gives a predicate gate: record the decision first, so the spec built from it is identical on every attempt. A `done()` with no detail commits nothing — its marker command is a function of the reason alone, nothing can drift, and that flow's journal is byte-for-byte the one it always wrote. **F2 — a lone surrogate never reaches a durable write.** `(prose + '\u{1F642}').slice(0, -1)` is ordinary JS trimming of an agent's output and produces a `string` that is not text. `JSON.stringify` escapes it, so the marker command was admitted; the raw detail then went out in the root's `step.complete` output, where the kernel's JSON decoder answered `bad_request` with a null request id — which resolves no pending request, so the CLI produced no report at all. Normalization now substitutes U+FFFD, one code unit for one, and `isDurableCompletionDetail` enforces the same invariant at every read boundary. Substituting rather than refusing: the flow has already reached its verdict, and losing the whole explanation over one broken code unit destroys more than it protects. Docs: `docs/CLOUD.md` no longer states the server's report-to-`error` projection as established fact. What this repo pins is the client half — the message this CLI produces and the rendering `flows status --cloud` gives an `error` that holds it; the hosted end to end is named as unconfirmed. Kernel: still no production change. One test pins the decoder property the SDK's normalization exists for — a lone surrogate is refused at the line, and the refusal carries `"id": null`, so no pending request can be matched to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Relayflow: the adversarial review did not pass. This branch is not approved: the flow stopped here and did not mark it ready to merge. Review of PR #547: changes requested — one P2 finding remains. Reviewed head: [P2] Recover a committed verdict before taking the no-detail shortcut. Location:
The real-daemon probe below uses the same flow, root identity, and reason on both executions. Its first attempt journals
Move the root-stream lookup ahead of the current-attempt no-detail shortcut. When a record exists, recover it; when none exists and this attempt has no detail, return without appending. This preserves no-detail marker bytes, spec hashes, and journal writes while honoring committed verdicts. Add regressions for both windows, including undefined/blank retry detail. The probe changes only the optional source, not the flow code or root identity; it is executor-seam failure injection, not a process-kill test. The prior review's lone-surrogate bug and its original same-text credential-rotation case are addressed by the follow-up code and the passing regressions below. The remaining finding is a different branch in that recovery logic. Cloud acceptance is still limited to the client contract:
Verification evidence The surface suite, 22 affected SDK suites, SDK build/typechecks, and kernel library suite were run. This is not a claim of running the full SDK suite. Commands and captured outputs follow. Working directory: npm test > /tmp/review547-surface.log 2>&1; result=$?; cat /tmp/review547-surface.log; exit "$result"Exit code: 0. Working directory: npm run build && npm run typecheck && npm run typecheck:testsExit code: 0. npx vitest run tests/authored-completion-detail.test.ts tests/authored-completion-recovery.test.ts tests/authored-status-detail.test.ts tests/authored-detail-live.test.ts tests/authored-root.test.ts tests/authored-node-result.test.ts tests/cloud-read.test.ts tests/authored-step-failed.test.ts tests/authored-declined.test.ts tests/authored-declined-report.test.ts tests/authored-declined-live.test.ts tests/authored-step-failed-exit.test.ts tests/cli-status.test.ts tests/run-state.test.ts tests/cloud-run.test.ts tests/spec-parity.test.ts tests/authored-flow.test.ts tests/authored-human.test.ts tests/authored-admission.test.ts tests/cli.test.ts tests/direct-run-failure.test.ts tests/authored-run-failure-evidence.test.ts > /tmp/review547-sdk.log 2>&1; result=$?; cat /tmp/review547-sdk.log; exit "$result"Exit code: 0. Working directory: sh ../ops/cargo.sh test -p relayflowd --lib > /tmp/review547-kernel.log 2>&1; result=$?; cat /tmp/review547-kernel.log; exit "$result"Exit code: 0. Remaining finding: reproduction and captured failure Save the following as import { expect, it } from 'vitest';
import { flow } from '@relayflows/surface';
import { executeAuthoredFlow } from '../src/authored-flow-executor.js';
import { readAuthoredVerdict } from '../src/authored-completion-record.js';
import { chainFixture } from './flow-chain-fixture.js';
it.each([false, true])('recovers a committed explanation when the optional source disappears; marker admitted=%s', async (admitMarker) => {
const fixture = chainFixture();
const envName = 'REVIEW_SUMMARY';
const previous = process.env[envName];
try {
const journal = await fixture.connect();
const root = await journal.runStart({ name: 'recovery-root', steps: [{
id: 'authored-root', type: 'agent', instruction: '{}',
surfaces: { streams: [{ stream: 'unheld-root-stream' }] }, recovery_mode: 'reset', max_iterations: 8,
}] } as never);
const handle = flow('optional-review-summary', async f => {
f.done('step_failed', { detail: process.env[envName] });
});
const run = () => executeAuthoredFlow(handle, journal, undefined, { rootRunId: root.run_id, dataDir: fixture.data });
process.env[envName] = 'review found 1 P2: cleanup remains ambiguous';
const start = journal.runStart.bind(journal);
journal.runStart = async (...args) => {
if (admitMarker) await start(...args);
throw new Error('injected marker response loss');
};
await expect(run()).rejects.toThrow('injected marker response loss');
journal.runStart = start;
const committed = await readAuthoredVerdict(journal, root.run_id, 'complete-1');
console.log('committed before retry', JSON.stringify(committed));
delete process.env[envName];
await expect(run()).resolves.toMatchObject({ completionReason: 'step_failed', completionDetail: committed!.detail });
} finally {
if (previous === undefined) delete process.env[envName]; else process.env[envName] = previous;
await fixture.close();
}
}, 60000);Working directory: npx vitest run tests/zz-review547-recovery.test.ts > /tmp/review547-probe.log 2>&1; result=$?; cat /tmp/review547-probe.log; exit "$result"Exit code: 1. PR comments inspected gh api --paginate repos/AgentWorkforce/flows/pulls/547/comments > /tmp/review547-inline-comments.json
gh api --paginate repos/AgentWorkforce/flows/issues/547/comments > /tmp/review547-issue-comments.json
gh api --paginate repos/AgentWorkforce/flows/pulls/547/reviews > /tmp/review547-reviews.jsonCaptured inline-comment response: []Captured submitted-review response: []Captured issue-comment response: [{"url":"https://api.github.com/repos/AgentWorkforce/flows/issues/comments/5768503296","html_url":"https://github.com/AgentWorkforce/flows/pull/547#issuecomment-5768503296","issue_url":"https://api.github.com/repos/AgentWorkforce/flows/issues/547","id":5768503296,"node_id":"IC_kwDOUF0ysM8AAAABV9RgAA","user":{"login":"coderabbitai[bot]","id":136622811,"node_id":"BOT_kgDOCCSy2w","avatar_url":"https://avatars.githubusercontent.com/in/347564?v=4","gravatar_id":"","url":"https://api.github.com/users/coderabbitai%5Bbot%5D","html_url":"https://github.com/apps/coderabbitai","followers_url":"https://api.github.com/users/coderabbitai%5Bbot%5D/followers","following_url":"https://api.github.com/users/coderabbitai%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/coderabbitai%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/coderabbitai%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/coderabbitai%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/coderabbitai%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/coderabbitai%5Bbot%5D/repos","events_url":"https://api.github.com/users/coderabbitai%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/coderabbitai%5Bbot%5D/received_events","type":"Bot","user_view_type":"public","site_admin":false},"created_at":"2026-09-21T22:39:53Z","updated_at":"2026-09-21T23:00:20Z","body":"<!-- This is an auto-generated comment: summarize by coderabbit.ai -->\n<!-- This is an auto-generated comment: skip review by coderabbit.ai -->\n\n> [!IMPORTANT]\n> ## Review skipped\n> \n> Bot user detected.\n> \n> To trigger a single review, invoke the `@coderabbitai review` command.\n> \n> <details>\n> <summary>⚙️ Run configuration</summary>\n> \n> **Configuration used**: Organization UI\n> \n> **Review profile**: CHILL\n> \n> **Plan**: Advanced\n> \n> **Run ID**: `69616f3a-8180-4eca-ad2d-e20e5f0fe56d`\n> \n> </details>\n> \n> You can disable this status message by setting the `reviews.review_status` to `false` in the CodeRabbit configuration file.\n> \n> Use the checkbox below for a quick retry:\n> - [ ] <!-- {\"checkboxId\":\"e9bb8d72-00e8-4f67-9cb2-caf3b22574fe\"} --> 🔍 Trigger review\n\n<!-- end of auto-generated comment: skip review by coderabbit.ai -->\n\n<!-- tips_start -->\n\n---\n\nThanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=AgentWorkforce/flows&utm_content=547)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.\n\n<details>\n<summary>❤️ Share</summary>\n\n- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai)\n- [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai)\n- [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai)\n- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)\n\n</details>\n\n\n<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>\n\n<!-- tips_end -->","author_association":"NONE","reactions":{"url":"https://api.github.com/repos/AgentWorkforce/flows/issues/comments/5768503296/reactions","total_count":0,"+1":0,"-1":0,"laugh":0,"hooray":0,"confused":0,"heart":0,"rocket":0,"eyes":0},"performed_via_github_app":{"id":347564,"client_id":"Iv1.6aaafe4fe882736b","slug":"coderabbitai","node_id":"A_kwHOB96YWc4ABU2s","owner":{"login":"coderabbitai","id":132028505,"node_id":"O_kgDOB96YWQ","avatar_url":"https://avatars.githubusercontent.com/u/132028505?v=4","gravatar_id":"","url":"https://api.github.com/users/coderabbitai","html_url":"https://github.com/coderabbitai","followers_url":"https://api.github.com/users/coderabbitai/followers","following_url":"https://api.github.com/users/coderabbitai/following{/other_user}","gists_url":"https://api.github.com/users/coderabbitai/gists{/gist_id}","starred_url":"https://api.github.com/users/coderabbitai/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/coderabbitai/subscriptions","organizations_url":"https://api.github.com/users/coderabbitai/orgs","repos_url":"https://api.github.com/users/coderabbitai/repos","events_url":"https://api.github.com/users/coderabbitai/events{/privacy}","received_events_url":"https://api.github.com/users/coderabbitai/received_events","type":"Organization","user_view_type":"public","site_admin":false},"name":"coderabbitai","description":"# Transforming Code Reviews with AI\r\n\r\n## Features\r\n\r\n**Automated Reviews**: Continuous reviews of the pull requests including incremental commits. \r\n\r\n**Summarization**: Generates high-level summary and a technical walkthrough of the PR changes. \r\n\r\n**Line-by-line review**: Provides line-by-line suggestions committable with one click.\r\n\r\n**Codebase verification**: Verifies the impact on the overall codebase and identifies missing changes.\r\n\r\n**Insights into your code**: Ask any questions on your codebase within the pull request \r\n\r\n**Chat about your code** : Chat with the bot around your code. The more you chat, the smarter it gets.\r\n\r\n**Issue Validation**: Validates the PR against the linked issues and identifies other related issues \r\n\r\n\r\n\r\n","external_url":"https://coderabbit.ai?utm_source=cr_app&utm_medium=github","html_url":"https://github.com/apps/coderabbitai","created_at":"2023-06-14T15:47:27Z","updated_at":"2026-09-20T03:49:19Z","permissions":{"actions":"read","checks":"write","contents":"write","discussions":"read","issues":"write","members":"read","merge_queues":"read","metadata":"read","pull_requests":"write","statuses":"write"},"events":["issues","issue_comment","label","membership","merge_group","organization","pull_request","pull_request_review","pull_request_review_comment","pull_request_review_thread","release","repository","team"]},"minimized":null}] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
done(reason, { detail }): let a flow say why it failedCloses the gap in the ticket:
ctx.done(reason)accepted only the reason, soevery authored
step_failedlanded on one generic sentence. Cloud runf92bf832-7848-58d8-b5ca-da8e3b849f1c(cloud#3919) ended that way after twentysuccessful steps and an opened PR. The reviewer's actual finding — "one P2
remains: cleanup can report success while an ambiguous allocation stays
invisible through all three sweeps …
review.cleanwas not created" — existedand reached no reader. Finding it took opening the Codex reviewer's raw
transcript.
Implemented against
reviewed-plan.md.What changed
Surface (
packages/surface)done(reason: FlowCompletionReason, options?: DoneOptions): void, withDoneOptions { detail?: string }. The one-argument form is untouched.COMPLETION_DETAIL_MAX_CODE_POINTS = 2000exported so an author can measurea detail before passing one.
SDK (
packages/sdk)src/authored-completion.ts(167 lines) — the authored verdictvocabulary (
LOWERED_COMPLETIONS,isLoweredCompletion,completionMarker), moved out of the 704-line executor, plusnormalizeCompletionDetail,isDurableCompletionDetailandsingleLineCompletionDetail. The executor shrank to 686 lines; the CLIreport and
flows statusnow read the vocabulary without importing theexecutor and everything it pulls in.
src/authored-verdict.ts(87 lines) — a small projection from a rootjournal to
{ reason, detail? }, kept out of the genericfoldRunState.authored-flow-executor.ts—done()validates and normalizes the optionsbefore
markCompletion(), and the terminal marker carries the detail.authored-root.ts—completionDetailon the root step's output;isCompletedRootOutputvalidates it;completedRootResultreturns thestored value without recomputing it.
authored-node-runner.ts— the IPC frame's detail is type- andbound-checked, then attested by the existing marker comparison.
cli/run.ts—RunReport.completionDetail, and the diagnostic's existingdetailkey. Forstep_failedthe detail replaces the generic sentence.cli/status.ts— a labelledauthored done("...")line and anauthored_completionJSON key, beside the kernel facts, which are unchanged.Kernel — no production change. A regression in
relayflowd/src/server/tests.rspins the three kernel properties the SDK nowrelies on.
Docs —
docs/SURFACE.mdgains a Saying why section (bound, redaction,absence semantics, marker shape, the
flows statusline, the one-line messageencoding) and an exit-code table note;
docs/CLOUD.mddocuments theerrorrendering and the one-line encoding.
Design decisions worth reviewing
The detail rides inside the terminal marker.
completionMarker(reason)with no detail returns the exact strings it always did; with one it returns
printf '%s' '<JSON>'where the JSON is{"completionReason":"<reason>","detail":"<detail>"}, shell-quoted the wayf.hookalready quotes its record. This makes the detail a durable journalfact, and the IPC verifier's existing command comparison attests it for free —
a frame cannot claim a detail the journal does not hold, drop one it does, or
alter a character of it.
Redact, then truncate — and this is not cosmetic. A secret environment
value is redacted by exact
replaceAll. Cut it in half first and there isnothing left for that match to find, so the truncation is what journals a live
fragment. Mutation-verified below.
The bound is 2,000 Unicode code points in the final string, suffix
included. Deliberately not the kernel's
worker_failure_detailbound,which takes 2,000 chars and then appends its suffix. Code points, not
String.length, which counts UTF-16 units. Over-long details truncate with avisible
… (truncated)rather than being refused.Absence has four spellings and they all mean the same thing. No argument,
undefined,{},{ detail: undefined }— and whitespace-only joins them.Nonempty prose is preserved, trimmed. No empty-string refusal was added: that
would be a new runtime failure for a type-valid string.
Local status keeps the kernel's account. An authored
step_failedrundoes complete with a root step that succeeded, and
status,completion_reasonand every step line stay exactly that. The authored verdictis a separate labelled line. Recognition is by the root's own
relayflows.authored-root.v1metadata, not the step id — an ordinary flow mayname a step
authored-root— and only a terminalstep_done/successcompletion with a complete, in-bounds output attests a verdict.
The diagnostic message is one line. Cloud renders a run's
errorthrougherrorLines, which elides the middle of a long multi-line error. A forty-linedetail rendered as forty lines would lose exactly the finding it carries, so
the message escapes breaks as
\n/\r/\t/\uXXXX. The unescapednormalized text stays in
completionDetailand the diagnostic'sdetail.Verification
Every claim below carries the literal command and its captured output.
Environment:
node v25.6.0, cargo bootstrapped byops/cargo.sh(
cargo 1.98.1).packages/surfacebuilt and installed intopackages/sdk/node_moduleswith the repo's existing local-surface pattern(
npm install ./../surface --prefix packages/sdk --no-save --ignore-scripts)before any SDK test was interpreted.
Surface
SDK — every affected suite
Typechecking (both configs, plus the new test files added to
tsconfig.tests.json's include list):(No diagnostics; both exited 0.)
Kernel
The new test asserts, through the protocol verbs: the marker command with JSON
survives admission and a journal reopen character for character; re-admitting
the identical spec under the same admission key returns the same
run_idandone
run.spawned; changing one word of the detail under that key is refusedwith
run_admission_conflict.Spec-hash stability against pre-change fixtures
The four no-detail lowered specs were captured from the tree before this
change (
git stash, capture,git stash pop; base commit16237b6) and arehardcoded in
tests/authored-completion-detail.test.tsasPRE_CHANGE_SPEC.The pre-change capture, truncated to fit:
The test re-derives the complete canonical spec and its sha256 for each of the
four reasons, for each of
done(r),done(r, {}),done(r, {detail: undefined})anddone(r, {detail: ' '})— sixteen cases — and comparesagainst these bytes. The capture file was removed after use; it is not part of
the commit.
Mutation verification
Two claims, each reverted, failed, restored byte-for-byte, and re-passed.
M1 — redaction runs before truncation. Reverted to
redact(bound(detail)):Restored (
diffagainst the pre-mutation copy produced no output) and re-run:M2 — the detail replaces the generic
step_failedsentence. Reverted themessage branch to the unconditional generic string, rebuilt, and ran both the
unit and the live end-to-end suite:
Restored (
diffproduced no output), rebuilt, re-run:git status --shortafter both mutations shows only the two untracked planfiles, so the committed tree is the restored one.
Test coverage, against the reviewed plan's regression matrix
packages/surface/tests/done-detail.test.ts(compiler: one-argument,{}, explicitundefined, all four lowered reasons,@ts-expect-erroron a numeric detail, a bare-string options, an unknown option, a third argument, and a step-only reason)authored-completion-detail.test.ts— five absence spellings, seven malformed shapes, type-named refusals with no value echoed, env-secret and token redaction, 1,999 / 2,000 / 2,001 code points, astral-plane counting, suffix inside the bound, straddling-secret ordering/bin/sh -cand parsed back (single quotes, double quotes, backslashes, newlines,$(id)/backticks, a'; rm …; echo 'terminator, unicode); the command is asserted to be one linePRE_CHANGE_SPEC(full canonical JSON and sha256)completionDetailabsent when there is none, no fabricated step evidence, multiline-safe message with the original indetail,needs_human/declined/successexit codes and kinds unchangedauthored-root.test.ts— journaled output with and without the key, completed-root resume returning the stored detail with the body never run, legacy output, three malformed details failing closed.authored-node-result.test.ts— altered / added / dropped detail all refused; non-string, over-long and empty details refused at the frameauthored-status-detail.test.ts— direct projection tests (nine negative cases including a similarly named ordinary step, retry, park, non-success root, malformed output), then CLI text and--json, full 2,000-code-point detail past the 1,024 gate limit, multiline folding, redaction in both renderings, and no line / no key for a one-argumentdone()cloud-read.test.ts— the real report message through a fake Clouderrorto text and--json, a token redacted, and a finding in the middle of a 40-line detail survivingerrorLines' elisionauthored-detail-live.test.ts— a realrelayflowd:flows run --jsonexit 1 and message, the root's journaled output, the marker child'sstdout_tail,flows statustext and--json,flows resumereturning the same detail with the journal byte-identical; plus the no-detail counterpartrelayflowd/src/server/tests.rs— persistence/reopen, idempotent admission, spec-drift refusal with a detail-bearing markerWhat is not verified
Cloud's server-side projection. Issue item 3 asks to confirm that Cloud
stores the diagnostic message as the run
error. This repository can show thatCloud stores the CLI's JSON run report as the run
result(
cloud-run.tsvalidatesresult.ok/.status/.completionReason) andthat
flows status --cloudrendersrun.error(cli/cloud-read.ts). The stepthat derives
errorfrom the report lives in agentrelay.com and is not inthis tree. The cloud tests here are labelled in the file as client contract
only. No Cloud credential was available in this environment, so neither
flows status --cloud f92bf832-…nor a fresh hosted run on a rebuilt artifactwas attempted. The
flows status --cloudacceptance criterion is thereforenot claimed as met end to end — only that the message reaches a reader once
errorholds it.Pre-existing failures in this environment, unrelated to the change. The
full
npm testinpackages/sdkreports 7 failed files / 39 failed tests.Every one of them was reproduced on the unmodified base commit:
— identical counts to the run with this change applied. Two causes, both
environmental: several suites hardcode
kernel/target/debug/relayflowdinsteadof reading
RELAYFLOWD_BIN(whichops/cargo.shrelocates outside the repo,and the file is
ENOENThere), andexamples/*.flow.tsresolves@relayflows/surfaceto a stale2.0.24copy in anode_modulesabove therepository root, which refuses the
versionheader.npm run typecheck:examplesinpackages/surfacelikewise fails onworkflows/stuck-run-triage.flow.ts(Cannot find name 'URL') identically onthe base commit.
Follow-up (out of scope, per the ticket)
Issue item 4: the
software-factorypreset in agentrelay.comweb/lib/flow-workflows.tsshould pass the reviewer's remaining findings — thereview.mdsummary or the P-level list — as the detail. Left unchanged here,as were the two in-repo
examples/software-factorycallers:tests/canonical-software-factory.test.tspins that example's commandsequence, and feeding
review.mdinto a detail would mean adding a journaledf.runto a canonical example.Rails
Committed on
relayflow/flows-software-garden-abe21cfb, on top of16237b6(the merge of #544). Nothing is committed to
main; a human merges. No gatethat judges this work was touched.
🤖 Generated with Claude Code
Note
Medium Risk
Touches authored execution, journal durability, IPC verification, and user-visible run/status reporting; behavior is heavily tested but resume/redaction ordering is subtle.
Overview
Adds
done(reason, { detail })so authored flows can explain verdicts (especiallystep_failed) instead of only generic failure text.The SDK normalizes optional detail at
done()(redact → bound to 2,000 Unicode code points with truncation suffix → strip lone surrogates), embeds it in the terminal marker JSON, and surfaces it ascompletionDetailon run reports, diagnostics, andflows status(authored done("…")beside unchanged kernelcompleted/successfacts). Verdicts with detail are committed to a rootauthored-verdictstream before the marker run opens so resume cannot drift redaction and hitrun_admission_conflict.One-argument
done()stays byte-identical for markers and spec hashes. Cloud docs/tests cover one-line error rendering whenrun.errorcarries the diagnostic message; server projection is explicitly out of repo scope. Kernel adds regression tests only (marker persistence, surrogatebad_request).Reviewed by Cursor Bugbot for commit 43a3b22. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Lets an authored flow call
done(reason, { detail })so astep_failedverdict carries the actual finding instead of a generic sentence.ctx.done(reason)still works unchanged; a missing, empty, or whitespace-onlydetailproduces byte-identical markers, specs, hashes, reports, and JSON shapes.done(): redacted, trimmed, bounded to 2,000 Unicode code points including a… (truncated)suffix, and lone surrogates substituted with U+FFFD. Redaction runs before truncation, so cutting cannot split a secret into a fragment no pattern matches.process.envchanged while the process was down. A one-argumentdone()commits nothing.completionDetailand on the diagnostic'sdetail;flows statusprints a labelledauthored done("...")line beside the kernel facts, which staycompleted / success.To review
errorlives in agentrelay.com and is not in this tree, so theflows status --cloudacceptance criterion is only verified client-side.Written for commit 43a3b22. Summary will update on new commits.