Software factory change - #545
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
`flows status --cloud` and `flows logs` were built for a run that had already finished. This adds the two live forms and the live step rows the watched page needs. - `flows status --cloud` renders `running`, `backoff`, `waiting` and `needs_human` rows in the local view's grammar, with the number of the attempt now running and the time since its start. Only what Cloud's step snapshot establishes is printed: no maximum-attempt denominator, no wait id, no attempt for a row that was never dispatched, and no advancing of a finished step whose end timestamp is missing. A running run with no rows says so rather than claiming the run has no steps. - `flows status --cloud --watch` redraws every two seconds until the run is terminal and leaves the final page up. Each frame is rendered, checked against the abort, then written in one call, so an interrupt cannot leave a torn page and a failed poll leaves the previous page alone. - `flows logs <run> --follow` appends new runner output until the run ends and Cloud marks the log complete. Lines are buffered across polls and redacted whole, so a secret split on a chunk boundary cannot escape. - Both exit with the run's outcome, not the read's, through the validator `flows run --cloud --wait` already blocks on (extracted to `cloud-run-record.ts` and shared): 0 only on an attested completed/success, 1 on an attested failure or cancellation, and `cloud_invalid_response` on a terminal record that attests neither. Ctrl-C is `observation_aborted`, exit 1, and does not cancel the run. The runner log is re-read whole on each poll and only the new suffix is printed: the route's `offset` is a byte count and the content is a string, and mixing them loses text on the first non-ASCII character. A snapshot that no longer extends what was printed refuses with `cloud_log_rewritten` rather than guessing. `--follow --step` is refused before any request; a step transcript is not an append-only stream. `cli/cloud-read.ts` was over the 500-line smell threshold, so the shared formatting, refusals and status view moved to dependency-only modules that the one-shot and live entry points both import. 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3c613af. Configure here.
Two review findings on #545. F1 (P1, review.md): `flows logs --follow` split the new content into lines before redacting each one, while the one-shot read redacts the whole content first. A secret env value spanning more than one line — a PEM private key is the usual one — matches no single line, so every line of key material reached stdout. The un-released remainder is now redacted as one block: a cut is released only where no secret value has begun and not ended (`openSecretStart`, which also covers a value whose halves arrive in two different polls) and only where redacting the block alone gives the same text as the front of the whole redacted remainder, so a cut that fell inside something the redactor would have caught moves back a line instead. A line can therefore appear one poll late; none of it can appear unredacted. Cursor Bugbot (inline, medium): `flows status --cloud` preferred a step row's `duration_ms` over the derived elapsed time. A row still in flight can carry the `wallclockMs` of the attempt that already ended, so a watched frame froze its clock at that attempt's duration. The derived time comes first now; `liveElapsed` is null for every row that is not demonstrably in flight, so a finished row still prints what Cloud reported. 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 #545Reviewed head: Changes requested. One credential-disclosure finding remains. P1 — Preserve unfinished credential-header context between pollsLocation: When a running log ends with The assumption that header shapes cannot span lines is false: the named-value patterns in Retain incomplete named-credential context across polls (or use an incremental redactor with equivalent semantics to whole-log redaction). Add regression coverage where the first snapshot ends immediately after each header and the next completes its value; assert that no intermediate or final stdout write contains the credential. ReproductionAfter the package build, the following command uses only fake responses, transitioning running → completed. All credential strings are synthetic. cat > /tmp/flows-review-current-probe.mjs <<'EOF'
import { runCloudLogsFollow } from '/home/daytona/.relayflow-v2-supervisor/durable/repository/packages/sdk/dist/cli/cloud-live.js';
import { redact } from '/home/daytona/.relayflow-v2-supervisor/durable/repository/packages/sdk/dist/redact.js';
for (const prefix of ['x-callback-token:\n', 'Bearer\n', 'authorization:\n']) {
const full = prefix + 'opaque-sensitive-value\n';
let tick = -1;
globalThis.fetch = async input => {
const logs = String(input).endsWith('/logs');
if (!logs) tick++;
const content = tick === 0 ? prefix : full;
return new Response(JSON.stringify(logs ? {content, offset: Buffer.byteLength(content), totalSize: Buffer.byteLength(content), done: tick > 0} : {runId:'review-run', relayflowVersion:'v2', status: tick === 0 ? 'running' : 'completed', result: tick === 0 ? undefined : {ok:true, status:'completed', completionReason:'success'}}));
};
const stdout = [], stderr = [];
const exit = await runCloudLogsFollow({runId:'review-run', step:undefined, json:false}, {stdout: x=>stdout.push(x), stderr:x=>stderr.push(x)}, {apiUrl:'https://fake.example', token:'fake-test-token',env:{},sleep:async()=>{}});
console.log(JSON.stringify({prefix, oneShot:redact(full,{}),exit,stdout,stderr}));
}
EOF
node /tmp/flows-review-current-probe.mjs > /tmp/flows-review-current-probe.txt
cat /tmp/flows-review-current-probe.txtCaptured output (exit 0): Prior findings and PR commentsRead the PR description, issue comment, submitted review, and inline comment. CodeRabbit skipped review. Cursor's inline finding concerns snapshot duration freezing live elapsed time; current Commands used to capture the comments: gh api --paginate repos/AgentWorkforce/flows/pulls/545/comments --jq '.[] | {path,line,body}' > /tmp/flows-review-current-comments.json
gh pr view 545 --json headRefOid,comments,reviews > /tmp/flows-review-current-pr.jsonThe complete captured responses are in those files; no comment was posted or changed. Affected checksFrom npm run typecheck && npm run build && npm run typecheck:tests && npx vitest run tests/cloud-live.test.ts tests/cloud-read.test.ts tests/cloud-run.test.ts tests/cli-status.test.ts tests/relay-cli-surface.test.ts tests/cli.test.ts tests/redact.test.ts > /tmp/flows-review-current-focused.txt 2>&1Captured typecheck/build output: Captured focused test output: Scope and limitsReviewed the diff, new tests, shared outcome validator, transport mapping, signal ownership, rendering and redaction paths. No live hosted run was exercised; server compatibility is not attested by this review. No production code or existing test was edited. No mutation-verification claim is made. Kernel code is unchanged; no standalone kernel test suite was run. Full SDK package suiteFrom npm test > /tmp/flows-review-current-package.txt 2>&1Exit 1: 8 files failed, 154 passed, 3 skipped; 41 tests failed, 2563 passed, 25 skipped; 1 unhandled error. Failures include missing daemon paths ( Complete captured package output |
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.
flows status --cloud --watchandflows logs --followCloses the ticket's three items: live step rows on the hosted status page, a
watched page, and a followed runner log.
What changed
Live step rows (
flows status --cloud, one-shot and watched).runningandbackoffrender↻,waitingandneeds_humanrender⏸,matching
cli/status.ts's glyphs. A row in flight shows the number of theattempt now running and the time since its
startTime:Only what Cloud's snapshot establishes is printed:
retryCount + 1, notattempts.length—those entries are completion records, so during a second attempt the array
still holds one and would print
attempt 1. It is printed only when the rowalso carries a
startTime; a row with no dispatch evidence shows no attemptat all rather than
attempt 1./maxdenominator, noawaiting <kind>: <id>, nobackoff until …:Cloud's step rows carry no maximum, no wait id and no backoff deadline.
finished step whose
endTimeis missing keeps the duration Cloud reportedinstead of being advanced to now. A start in the future clamps to zero.
steps 0thenNo step snapshot available yet.— a fact about the snapshot, where a baresteps 0would be a claim about the run.flows status --cloud --watch. Polls every two seconds; each tick readsthe run record, then the step rows, renders the whole page, checks the abort,
and emits it in one
io.stdoutcall. ANSI clear only for a TTY (threadedthrough
CliIo.tty); redirected output appends complete pages. A failed pollleaves the previous page untouched. The terminal page is printed once and no
further tick is scheduled.
flows logs <run> --follow. Reads the run record, then the log, each tick;prints only whole lines, holding a partial line across polls so a secret split
on a chunk boundary is redacted as one line. Stops only when the run is
terminal and the envelope says
doneand the served bytes covertotalSize;donewith an active run keeps following, and a terminal run withdone: falsekeeps draining. Prints the header once and the run's footer once(
COMPLETED <run> completionReason: success).Exit codes are the run's, not the read's. The raw-record validator behind
getCloudFlowRunmoved tocloud-run-record.tsand is now shared, so--watch/--followblock on exactly whatflows run --cloud --waitblockson: 0 only on an attested
completed/success, 1 on an attested failure orcancellation, and
cloud_invalid_response(exit 1) on a terminal record thatattests neither, or on a run status this client does not know.
getCloudRunDetailLivevalidates and maps the same response, so no secondGET per poll. Ctrl-C is
observation_aborted, exit 1, and says the hosted runwas not cancelled. Existing read refusals keep their codes and exits.
Deliberate decisions
offsetis a byte count andcontentis aJavaScript string; subtracting one from the other silently loses text the
moment the log contains a non-ASCII character — which the runner's own
transition lines do (
▶,✓,·). The Cloud-side contract could not bechecked from this repo, so
--followre-reads the log whole each poll andprints the new suffix, refusing with
cloud_log_rewrittenif what wasprinted is no longer a prefix. Cost: a full read per poll, and the log held
in memory. Benefit: no line can be duplicated or skipped. Documented in
docs/CLOUD.md.--follow --stepis refused (invalid_invocation, before any request).A retry replaces a transcript and the rendered form is built from the whole
JSONL; following it is a separate feature.
--jsonemits one final document, not a frame stream — the one theone-shot form would have printed, with
--followcarrying the whole redactedlog. Deliberately unlike
check --watch's report-per-check.ok: truemeansthe read succeeded; the run's outcome is the exit code. Called out in the
docs as a choice, not a repository rule.
projections; a step row can lag its header by a poll. Documented rather than
papered over.
donekeeps draining. Stoppingafter N identical polls would be an arbitrary truncation; Ctrl-C ends it.
Documented.
Module layout
cli/cloud-read.tswas 518 lines, past the AGENTS §1 threshold. The sharedpieces moved to dependency-only modules, so the one-shot and live entry points
import them without depending on each other (
cli.tsdispatches both):cli/cloud-format.tssafe,thousands,dollars,instant,ago,errorLinescli/cloud-refusal.tsrefusalFor,fail,RUN_ID_REQUIRED,isTransientReadcli/cloud-status-view.tscli/cloud-live.tscli/cloud-read.tscloud-run-record.tscloudRunState,isCloudRunActiveVerification
All commands run from
packages/sdk.npm run typecheck && npm run build && npm run typecheck:tests— clean:npx vitest run tests/cloud-live.test.ts tests/cloud-read.test.ts tests/cloud-run.test.ts tests/cli-status.test.ts tests/relay-cli-surface.test.ts tests/cli.test.ts:tests/cloud-live.test.tsis new: 43 cases over live rows (all four states,retry in progress, unknown attempt, absent/invalid/future timestamps, elapsed
growing across frames, empty snapshot), watch lifecycle (running → completed,
already terminal, failed, cancelled, exactly one final frame and no extra poll,
TTY vs redirected), outcome integrity (completed with no valid result,
contradictory reason, unknown status, mismatched run id,
needs_humanstep onan active run), follow content (growth, no-op poll, blank and repeated lines,
CRLF, partial lines, unterminated final line, empty log, multibyte log),
draining (
done+ active, terminal + notdone, transient failure on the finalread, finished failed run), security (secret split across polls), cancellation
(pre-aborted, between the two reads, during the sleep, aborted fetch, no
process handler installed), refusals and retries (404/401, 503 backoff growth,
cap and reset, zero requests on the refused combination),
--jsonfor bothverbs, and the argv/
runCliwiring.Mutation verification
Two behaviours, each reverted, failed, restored byte-for-byte (md5 compared),
and re-passed.
1. Attempt number from
retryCount + 1, notattempts.length. Changedcli/cloud-status-view.ts:85to`attempt ${step.attempts.length}`:Restored (
md5sumof the file and the backup bothc736206b848f19d523c4353d2c8cd071) and re-run:2. The abort check before a frame is emitted. Deleted the
if (isAborted(signal)) return aborted(...)line between rendering and writingthe page in
cli/cloud-live.ts:Restored (both
53d54ee5994b3ecfb687eb9ceb638c42) and re-run:Full package suite: 7 files fail, for environment reasons, before this change
npm testinpackages/sdk:The failures are
tests/live-kernel.test.ts,tests/webhook-live.test.ts,tests/mcp.test.ts,tests/provider-trigger-executor.test.ts,tests/authored-node-runtime.test.ts,tests/canonical-software-factory.test.tsandtests/stuck-run-triage.test.ts, with two causes, neither touched by thischange:
spawn …/kernel/target/debug/relayflowd ENOENT—ops/cargo.shputs buildoutput under
$HOME/.relayflows-toolchain/target/<hash>/, notkernel/target/, so the live-kernel suites find no binary in this sandbox.expected an @relayflows/surface flow handle— this sandbox had nonode_modulesat all; installing pulled the published@relayflows/surfacerather than linking
packages/surface, so two copies of the surface areloaded.
Confirmed pre-existing by stashing the change (
git stash push -- docs packages) and re-running exactly those seven files on the unmodified tree:Same seven files, same 39 tests. The stash was popped and the working tree
restored before committing.
No kernel code was touched and no kernel test was run. The change is
TypeScript CLI and transport only.
Not done
(cloud#3918) could not be verified from this repo — both issue URLs were
unreachable. Item 1 is therefore exercised against fixtures shaped like the
documented step-row contract, not against a live hosted run.
--follow --step(rendered or raw) is refused rather than implemented.--watchfor the localflows status; it stays a one-file offline read.Note
Medium Risk
Changes CLI exit semantics, polling, and streaming secret redaction for Cloud reads—important for scripts and operators, but scoped to read-only hosted observation with no auth or execution-path changes.
Overview
Adds live observation for hosted runs:
flows status --cloud --watchredraws the Cloud status page every two seconds until the run is terminal, andflows logs <run-id> --followstreams new runner log lines on the same cadence. Both use the run’s exit code (aligned withflows run --cloud --waitvia sharedcloud-run-record.tsvalidation), support Ctrl-C asobservation_abortedwithout cancelling the run, and emit a single--jsondocument at the end.flows status --cloudnow renders in-flight steps like the local view (↻/⏸, attempt fromretryCount + 1, elapsed fromstartTime, empty snapshot messaging).--followre-reads the full log each poll (no offset paging), refuses--stepandcloud_log_rewrittenon non-prefix updates, and uses block-wise redaction with newopenSecretStartso secrets split across polls or lines are not leaked.Refactors oversized
cloud-read.tsinto shared format, refusal, status view, and live modules; wiresCliIo.ttyfor TTY vs redirected redraw. Docs (CLOUD.md,SURFACE.md) and extensivecloud-live.test.tscover the behavior.Reviewed by Cursor Bugbot for commit e191e3d. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds live reads for hosted runs:
flows status --cloudnow renders running steps with elapsed time,--watchredraws the page every two seconds until the run ends, andflows logs <run> --followappends new runner output until the run is terminal.flows run --cloud --waitalready uses (extracted tocloud-run-record.tsand shared): 0 only on an attestedcompleted/success, 1 on failure or cancellation,cloud_invalid_responseon a terminal record that attests neither.observation_aborted, exit 1) without cancelling the hosted run.startTimefor in-flight rows and is never overridden by a staleduration_ms, so a watched frame's clock keeps running.--followre-reads the whole log each poll and prints only the new suffix, redacting the un-released remainder as one block rather than line by line, so a multi-line secret such as a PEM key cannot leak; a line can appear one poll late, and a snapshot that no longer extends printed output refuses withcloud_log_rewritten. Offset paging was avoided because the route'soffsetis a byte count and the content is a string, which would lose text on the first non-ASCII character.--follow --stepis refused before any request.Refactors
cli/cloud-read.ts(518 lines, past the project's threshold) was split into dependency-only modules —cloud-format.ts,cloud-refusal.ts,cloud-status-view.ts,cloud-live.ts— so the one-shot and live entry points share formatting, refusals, and the status view without import cycles.cloud-run-record.ts(cloudRunState,isCloudRunActive), now the single source of truth for what a run record attests.tests/cloud-live.test.tscovers 43 cases across live rows, watch lifecycle, draining, follow content, redaction, cancellation, and refusals.Written for commit e191e3d. Summary will update on new commits.