Repository navigation
unprovable_effects warns once per step and fires on nearly every real spec, so the warning channel carries no signal - #621
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
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 |
…are-garden-83d7850d Session-Id: 0e078a4c-f89d-4e74-9919-00e1e9a08879
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Session-Id: 0e078a4c-f89d-4e74-9919-00e1e9a08879
|
Finishing pass. No Garden adversarial review or review threads were recorded for this PR, so I reviewed it myself:
bugbot run |
|
@coderabbitai review |
|
bugbot run |
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e08cc12. Configure here.
…are-garden-83d7850d # Conflicts: # packages/sdk/src/cli.ts Session-Id: 0e078a4c-f89d-4e74-9919-00e1e9a08879
Combine both flows check options in cli.ts, cli-commands.ts, cli-watch.ts (the watch child forwards the daemon mode and --explain-warnings), and the CLI surface samples. #621's JSON byte-identity test now builds its expected report in the CLI's daemon validation mode, since check --json carries the validation field. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Session-Id: 0e078a4c-f89d-4e74-9919-00e1e9a08879
1 participant
flows checknow collapses repeatedunprovable_effectswarnings into one line,so a suspicious command remains visible.
--explain-warningsrestores everyoriginal per-step diagnostic, including in watch mode. JSON stdout retains the
original report; only its accompanying stderr is summarised.
The change follows
reviewed-plan.md: folding is an explicit renderer opt-in,starts at two affected steps, counts distinct step IDs, and preserves other
kinds in order before appending the summary. The denominator includes every step.
The ladder examples consequently read
2 of 2,2 of 3, and2 of 3.Scope: fixes
checkon YAML/JSON specs. The external 58-step spec's format hasnot been confirmed. Authored TypeScript checks do not currently produce these
step diagnostics.
run/resume/deployretain their current rendering, with afollow-up recorded in
ops/BACKLOG.md. Stdout still prints eachGATEline.No effect proving, spec-schema changes, kernel changes, or workflow edits.
Regression coverage includes opt-in isolation, single-step and duplicate-step
cases, missing step IDs, exact diagnostic ordering, full per-step explanation,
JSON serialization equality, invalid flags, and real watch-child forwarding.
The run-renderer regression uses a deliberate daemon-unreachable invocation
with
--no-spawn; it verifies warning rendering, not live kernel execution.The explicit per-step explanation assertion supplies coverage that the existing
ladder prefix assertions no longer provide after folding.
The
relay-cli-surface.test.tschange only adds an INVOCATIONS sample for thenewly declared flag. Existing gate assertions remain intact; no covenant-2
assertion or preflight implementation was edited.
Verification commands and captured output follow. All final commands below
completed successfully. During implementation, the first build was mistakenly
launched at the repository root (no package.json); the first SDK build found an
import inserted ahead of the CLI shebang (TS18026/TS1005). The import was moved
below the shebang before the successful build and checks captured here.
The ten-step demonstration fixture (outside the checkout) was:
Its sibling
/tmp/foldcheck/flows.jsoncontains{}.JSON stderr captured by the command above:
Checks
The checks fail on the base commit too, so these failures were not introduced by this change: they come from the repository itself or from the environment the checks ran in. This pull request is a draft until someone looks.
What ran (.relayflow/check.sh)
Output on this branch (last 80 lines)
Output on the base commit (last 80 lines)
What the repair agent found
Repair notes —
relayflow/flows-software-garden-83d7850d.relayflow/check.shfailed on this branch with 31 failed tests + 1 failedsuite. None of them trace to the branch's change (
flows checkwarningfolding). Three distinct root causes, all in the machine rather than in the
code:
tests/authored-node-runtime.test.ts)$HOME/package.jsondeclares"type": "commonjs"tests/live-kernel.test.ts)kernel.apparmor_restrict_unprivileged_userns=1and the sysctl is not writable here1. bun version (fixed as setup)
packages/sdk/tests/authored-node-runtime.test.ts:18asserts the bun versionbefore building a standalone CLI, because the standalone target is a bun
single-file executable. All four workflows check.sh mirrors pin it:
check.sh had no bun step at all — it used whatever
bunPATH offered, which onthis machine is nvm's 1.3.6. It now fetches the pinned release into
$HOME/.relayflows-toolchain/bun-1.4.0(the same private toolchain homeops/cargo.shuses), prepends it to PATH, exportsFLOWS_BUILD_BUN, and failsclosed if the result is not 1.4.0.
2.
$HOME/package.jsonwith"type": "commonjs"(fixed as setup)Seven
tests/live-kernel.test.tscases failed with the agent step'sverification reading
{gate: 'execution', verdict: 'fail'}andoutput: null,e.g.
The fixtures those steps execute (
testdata/preflight/analyze-story-stub-cliand friends) are extensionless executables whose bodies are ESM:
Node decides an extensionless file's module kind from the nearest ancestor
package.json. This checkout lives at/home/daytona/.relayflow-v2-supervisor/durable/repository, and the harnessimage ships an unrelated
/home/daytona/package.jsoncontaining"type": "commonjs". That turns module detection off, the fixture is loaded asCommonJS, and node then exits 0 having produced nothing — no output, no
error:
Reduced to first principles, outside the repo entirely:
Not a node-version artifact — node 22 (what CI pins) behaves identically:
A CI checkout has no ancestor
package.json, so CI never sees this. check.shnow writes
testdata/preflight/package.json={"type":"module"}during setupif it is absent. That is deliberately not committed and not tracked: the
repo has no root
package.jsonby design, and this is environment repair, nota change to the branch under test. With it in place all 32 live-kernel cases
pass (see §4).
Same cause, visible outside the test suite —
flows checkon an authored.tsflow also breaks here, which is worth knowing when hand-verifying anything:
check.sh does not paper over that one, because no test in the suite depends on
it. If a future run needs it, the fix is the same one line in
examples/.Durable fix worth a human's call (not done here, out of this task's scope):
commit
{"type":"module"}package.json files next to the extensionless ESMfixtures, or a root one. That makes the fixtures independent of wherever the
tree happens to be checked out, instead of relying on no ancestor declaring a
type.
3. bubblewrap cannot create a network namespace (outside this run's control)
24 failures, all one error:
bubblewrap is installed and new enough; it simply cannot unshare a namespace
here:
CI's remedy is
sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0.This machine has passwordless sudo, and the sysctl is still refused — it is
not namespaced, so a container cannot write it:
So the hosted-extension sandbox suites cannot run on this machine at all. This
is exactly the case check.sh's own header already anticipates ("CI does that on
an ephemeral VM it owns; this machine's kernel is not ours to retune … fail
loudly — which is the honest outcome, not a pass"). Left failing; no check.sh
change, no code change. These 24 cases are unverified by this run and need
a machine that can unshare a user namespace.
Affected files:
tests/hosted-extension-isolation.test.ts(13),tests/hosted-extension-protocol.test.ts(8),tests/software-garden-babysitter-composition.test.ts(2),tests/babysitter-native-extension.test.ts(1).Unrelated note: node version
CI pins
actions/setup-node@v4node-version: "22"; this machine runs nodev25.6.0 and no other version is installed. No failure in this run traces to
that (§2 reproduces identically under node 22), so check.sh was left using the
PATH node rather than growing a second toolchain bootstrap.
Fixes #510
Summary by cubic
flows checknow folds repeatedunprovable_effectswarnings into a single trailing stderr line, so rarer diagnostics likecommand_unresolvedstay visible. Use the new--explain-warningsflag (also with--watch) to list each warning per step;--jsonstdout always retains the full original report, and only the stderr rendering is summarized.llmandagentsteps.checkon YAML/JSON specs:run,resume, anddeploykeep their current rendering, with a follow-up noted inops/BACKLOG.md.$HOME/package.json, bubblewrap without a writable userns sysctl) and reproduce identically on the base commit.Written for commit 0fa9174. Summary will update on new commits.
Note
Low Risk
CLI presentation and flag parsing only; preflight diagnostics and JSON reports are unchanged aside from stderr folding on check.
Overview
flows checknow collapses repeatedunprovable_effectswarnings on YAML/JSON specs into a single trailing line when two or more distinct steps are affected (denominator is total step count). Other warning kinds stay separate and in order, so warnings likecommand_unresolvedremain visible.Adds
--explain-warnings(including with--watch) to print every per-step warning again.--jsonstdout still carries the full diagnostic list; only human stderr is summarized.run,resume, anddeployare unchanged.Implementation is presentation-only via
foldUnprovableEffectsindiagnostic-fold.ts, wired throughemitCheckReport/emitDiagnostics. Docs and CLI surface metadata are updated; a backlog item notes extending the same pattern to other verbs later.Reviewed by Cursor Bugbot for commit 0fa9174. Bugbot is set up for automated code reviews on this repo. Configure here.