Skip to content

docs(demos): promoted-workflow demo, and the live gate it made runnable - #215

Merged
pdettori merged 5 commits into
rossoctl:mainfrom
pdettori:demo/promoted-workflow
Sep 3, 2026
Merged

docs(demos): promoted-workflow demo, and the live gate it made runnable#215
pdettori merged 5 commits into
rossoctl:mainfrom
pdettori:demo/promoted-workflow

Conversation

@pdettori

@pdettori pdettori commented Sep 3, 2026

Copy link
Copy Markdown
Member

What this does

Adds the guided demo for workflow promotion (#214), plus its asserting sibling and a cluster-free
test for the fixture it depends on.

The claim: a Claude Code workflow authored in a minimal local sandbox — one skill, a
CLAUDE.md, one memory file, one slash command — runs unchanged in the harness. The payoff is an
A/B: the same prompt dispatched twice to the same cluster, differing only by configRef.

A (bare):       "It looks like the workspace is mostly empty. Could you provide more context?
                 1. What project is this for?  2. What is a 'ship note' in your context?…"

B (+configRef):  SHIP NOTE · auth-timeout-v2
                 WHAT:   Fixed idle reaper measuring time from connection open…
                 TICKET: KAG-4471                          <- promoted memory
                 RISK:   low — behind auth.idle_reaper_v2   <- promoted CLAUDE.md
                 TOKEN:  SHIPNOTE-7F3A-SANDBOX-OK           <- read in the sandbox pod

HOME=$SANDBOX is the demo rather than a trick to skip work: it makes promote read the sandbox as
user scope, so 1 skill travels in 12 KB with zero preflight findings, against 56 travelling
skills and ~8.6 MB for a real ~/.claude. That is the sandbox-first authoring §11 recommends,
enforced by one env var. It is also what makes the slash command travel at all — promote reads
prompts from user scope only, so a project-scope .claude/commands/ file is invisible without it.

Shape

File Role
docs/demos/promoted-workflow-demo.md The walkthrough, in acts, per docs/demos/README.md conventions
deploy/knative/demo-promoted-workflow.sh make demo-promoted-workflow — same claims, asserted
deploy/knative/fixtures/promoted-demo/ The seeded workflow (skill + CLAUDE.md + memory + command)
deploy/knative/tests/demo-promoted-workflow.test.sh Cluster-free; pins the fixture invariants the A/B rests on

Three findings from making it actually run

Each is now defended in the script, because each produced a green run that proved nothing.

1. The shared config cache is readable by other leaves. The overlay materialises the bundle into
/workspace/.sh-config/<digest>/ in the shared pool sandbox — world-readable, holding
context/agents/0-CLAUDE.md and memory/ — and it outlives the leaf. A later bare leaf leasing
that sandbox can explore the filesystem and answer from another run's promoted workflow. Measured:
the second run's bare arm emitted TICKET: KAG-4471 and TOKEN: SHIPNOTE-7F3A-SANDBOX-OK, opening
with "following the house rules", having been told none of it. The A/B still looked plausible; it had
stopped being a control. The script now purges the digest from every pool sandbox before the control
run and asserts the purge.

Filed as #216, deliberately narrowed: cross-leaf reading is an accepted non-goal (P2 §9, one trust
domain; Kata isolation is P3/#48), so the news is lifetime — the cache outlives the leaf that
created it, which makes spec §2 goal 6 ("absent a promoted bundle, harness behavior is unchanged")
true in the harness process, where run-turn.ts:382 asserts it, but not observably true. Checked
against SECURITY.md first; the reasoning for filing publicly rather than as an advisory is written
into the issue so a maintainer can overrule it.

2. Probing for a listener on 6379 uploads to the wrong Redis. This repo's own test container
publishes 0.0.0.0:6379, so a "something already listens, reuse it" shortcut made promote upload
into it, print uploaded, and then the harness — reading the cluster's Redis — failed with
config bundle not found for the digest sitting in the log. The script binds its own port (16379),
refuses to run if it is taken, and reads the key back through the cluster's own client
(kubectl exec deploy/redis -- redis-cli EXISTS), which is the only check that distinguishes the
two — and does it before the two model calls are paid for.

3. A warm cluster silently serves a pre-#214 image. kind load on a mutable tag rolls no new
Revision, so the feature reads as absent. Claim 0 gates on an unknown digest failing loudly and
prints the forced-roll commands rather than emitting a green run on the wrong image.

The sandbox half is asserted twice

Once in the model's words — a token readable only by a read executed in the sandbox pod, resolving
a relative sibling path against the injected absolute skills root — and once on the filesystem,
which holds even on a run where the model declines to read. pi puts only a skill's
name/description in the system prompt and tells the model to read the body on demand
(core/skills.ts), so the format appearing at all already implies the sandbox got the bundle.

Verification

  • 14/14 claims pass on three consecutive runs against kind, including two where the sandbox
    cache was warm from the previous run — the condition that broke run 2.
  • Every command in the walkthrough was driven by hand as written; the outputs shown are real,
    and the digest was byte-identical across two different sandbox paths, confirming the canonical-tar
    property the doc claims.
  • make lint (all 9 hooks) and make test-deploy (94 checks) pass.
  • The new test pins what the A/B rests on: the ticket appears only in the memory file, the token
    only in references/, the skill's frontmatter name and description are intact, MEMORY.md
    links resolve, and --teardown touches no cluster (mirroring demo-teardown-scope.test.sh).

Records the measurement §8 left owed

§8 said the end-to-end in-cluster cold-start comparison "could not be taken during this
implementation: the deployed image predates this branch, and taking it needs a full image build,
kind load, and a forced new Revision." Building this demo required exactly that, so the second
commit takes it and drops (cold-start measurement owed) from the spec's status.

arm median min max
baseline (no configRef) 6.38 s 6.18 s 7.43 s
promoted (configRef set) 7.61 s 6.23 s 15.31 s

N=5 per arm, alternating, each sample preceded by a wait for Knative's own scale-to-zero.
Median delta +1234 ms against a within-arm spread of 9082 ms, so what §8 now records is
"no cold-start regression observable at this sample size" — not a cost figure. Container start
and one model call swamp the ~114 ms of fetch/verify/unpack measured in isolation; §8 says so and
names the in-pod measurement anyone wanting a resolvable number should take instead.

Both false starts are recorded too, because each produced a confident wrong number rather than an
error: reusing one curl output path let a dead port-forward report 0.0005 s responded for ten
straight samples (curl never wrote the file, jq read the previous body), and forcing
kubectl scale --replicas=0 fights the KPA, which re-scales after each served request — that
skipped every baseline sample while running every promoted one, a one-armed "comparison" that still
printed five tidy timings.

The reproduction snippet in §8 is replaced rather than annotated: as written it forced
--replicas=0, slept 5 s instead of waiting for zero, and selected
serving.knative.dev/service=harness instead of serverless-harness, so it could not have produced
a cold start on this cluster.

Also fires the live gate that had never fired, and fixes what stopped it

promote-live-smoke.test.ts (SH_PROMOTE_LIVE_SMOKE) is the gate #214 recorded as never having run
in a cluster. Run against kind, it could not have passed: both session ids used the
<run>/<item> shape, and the slash is rejected by the session-id validator, so the first test failed
in 597 ms with

Session id must be non-empty, contain only alphanumeric characters, '-', '_', and '.', and
start and end with an alphanumeric character

— before any model call, never reaching the sibling-read assertion it exists to make. The second test
was passing for a reason unrelated to its assertion, which is why nothing looked wrong: an unknown
digest fails before session-id validation is reached, so its toContain('not found') held while
its id was equally invalid.

Both ids now use -, with a comment recording why the <run>/<item> shape is right elsewhere
(leaf-smoke.sh and the leaf fixtures go through an item envelope, where leafSessionId derives a
sanitized id) but wrong here (kind: 'prompt' uses sessionId verbatim as the session key).

SH_PROMOTE_LIVE_SMOKE=1 … pnpm exec vitest run test/promote-live-smoke.test.ts
✓ test/promote-live-smoke.test.ts (2 tests) 8420ms   →   leaf returns PROMOTED-SIBLING-OK

So spec §4.5 path translation is now demonstrated by the gate written for it, not only by this
demo's HTTP equivalent. Unset the env and both tests skip as before; the full harness suite is
348 passed / 5 skipped / 0 failed.

🤖 Generated with Claude Code

Shows PR rossoctl#214 end to end: a Claude Code workflow authored in a minimal local sandbox -- one
skill, a CLAUDE.md, one memory file, one slash command -- running unchanged in the harness.
The payoff is an A/B: the same prompt dispatched twice to the same cluster, differing only by
`configRef`. The bare arm asks what a ship note is; the promoted arm emits the house format
and cites an incident id that exists nowhere but the memory directory.

`HOME=$SANDBOX` is the demo rather than a shortcut. It makes promote read the sandbox as USER
scope, so 1 skill travels in 12 KB with zero preflight findings, against 56 skills and ~8.6 MB
for a real ~/.claude -- the sandbox-first authoring the design recommends (spec §11). It is
also what makes the slash command travel at all, since promote reads prompts from user scope
only.

Three findings from making it actually run, each now defended in the script:

1. The shared config cache leaks across leaves. The overlay materialises the bundle into
   /workspace/.sh-config/<digest>/ in the SHARED pool sandbox, world-readable, holding
   context/agents/0-CLAUDE.md and memory/, and it outlives the leaf. A later bare leaf leasing
   that sandbox can explore the filesystem and answer from another run's promoted workflow --
   measured: the second run's bare arm produced the ticket AND the token, having been told
   neither, so the A/B looked fine while proving nothing. The script purges the digest before
   the control run and asserts the purge; the demo's notes name it as a confidentiality
   question on a multi-tenant pool, since ADR-0031 covers write-protection, not visibility.

2. Probing for an existing listener on 6379 uploads to the wrong Redis. This repo's own test
   container publishes 0.0.0.0:6379, so promote reported `uploaded` and the harness then failed
   with `config bundle not found` for that exact digest. The script binds its own port (16379)
   and reads the key back through the cluster's own client, before the two model calls are paid.

3. A warm cluster silently serves a pre-rossoctl#214 image. `kind load` on a mutable tag rolls no new
   Revision, so the feature appears absent. Claim 0 gates on an unknown digest failing loudly
   and prints the forced-roll commands rather than emitting a green run that proves nothing.

The sandbox half is asserted twice: in the model's words, via a token readable only by a `read`
executed in the sandbox pod against a translated sibling path, and on the filesystem, which
holds even when the model declines to read.

Verification: 14/14 claims pass on three consecutive runs against kind, including two where the
cache was warm from the previous run. Every command in the walkthrough was driven by hand as
written. `make lint` (9 hooks) and `make test-deploy` (94 checks) pass. The new cluster-free test
pins the fixture invariants the A/B depends on -- the ticket must appear only in the memory file
and the token only in references/ -- plus that `--teardown` touches no cluster.

This is also the in-cluster end-to-end exercise rossoctl#214 recorded as owed.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
@pdettori
pdettori force-pushed the demo/promoted-workflow branch from c6d5599 to 429e4e0 Compare September 3, 2026 16:10
§8 said the end-to-end comparison "could not be taken during this implementation: the deployed
image predates this branch, and taking it needs a full image build, `kind load`, and a forced
new Revision." Building the promoted-workflow demo required exactly that, so the measurement is
now taken against a Revision built from main with the feature in it. Status drops
"(cold-start measurement owed)".

N=5 per arm, alternating, each sample preceded by a wait for Knative's own scale-to-zero:
baseline median 6.38 s (6.18-7.43), promoted median 7.61 s (6.23-15.31). Median delta +1234 ms
against a within-arm spread of 9082 ms, so the recorded conclusion is "no cold-start regression
observable at this sample size" rather than a cost figure -- container start and one model call
swamp the ~114 ms of fetch/verify/unpack measured in isolation. §8 says so explicitly and points
at the in-pod measurement anyone wanting a resolvable number should take instead.

Both false starts are recorded, because both produced confident wrong numbers rather than
errors: reusing one curl output path let a dead port-forward report `0.0005 s responded` ten
times (curl never wrote; jq read the previous body), and forcing `kubectl scale --replicas=0`
fights the KPA, which skipped every baseline sample while running every promoted one -- a
one-armed comparison that still printed five tidy timings.

The reproduction snippet is replaced rather than annotated: as written it forced --replicas=0,
slept 5 s instead of waiting for zero, and selected `serving.knative.dev/service=harness`
instead of `serverless-harness`, so it could not have produced a cold start on this cluster.
Leaving it in place next to a note saying it does not work would be worse than either fixing or
deleting it.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
Filed as rossoctl#216. The demo's Act 3a purge and its "Notes and limits" entry now say what the issue
narrows the claim to, so a reader is not left thinking cross-leaf reading is itself the news:
that part is an accepted non-goal (P2 §9, one trust domain; Kata isolation is P3/rossoctl#48). The new
part is lifetime — the digest cache outlives the leaf that created it, so a configRef-less leaf
can answer from it, which makes spec §2 goal 6 true in the harness process (run-turn.ts:382
asserts exactly that) but not observably true.

Both notes name the condition for their own removal: once the cache no longer outlives its leaf,
the purge step and the caveat go.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>

@pdettori pdettori left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well-evidenced demo PR. The three "green run that proved nothing" defenses are the substance here, and each is verifiable rather than asserted: the purge and materialisation asserts cross-check, so a wrong CACHE_DIR cannot produce a false green (the purge would no-op and the landed assert would fail); and the in-cluster redis-cli EXISTS is the only check that distinguishes the two Redises, run before the two model calls are paid for.

Spot-checked against source rather than taken on trust: CACHE_DIR's sha256-<hex> shape matches digestDirName (packages/config-bundle/src/tar.ts:137) via configCacheDir (harness/src/config-overlay.ts:10); KSVC/PORT are provided by lib.sh:15-16, so set -u is satisfied; mapfile has precedent in verify-sandbox-inventory.sh and measure-swebench-runtimes.sh; $(DEMO_ARGS) matches the existing demo-remote-sandbox target; make test-deploy globs tests/*.test.sh, so the new test is auto-discovered and the green deploy-scripts job covers it; the 14 ok calls match the claimed 14/14; and the spec's figures are internally consistent (15.31 − 6.23 = 9.08 s within-arm spread). SC2086 on the unquoted $CURL_OPTS is not applicable — the gate runs at -S warning and SC2086 is info.

The new test is genuinely assertive: it pins the two invariants the A/B rests on (ticket only in the memory file, token only in references/), and the unmarked-directory case actually proves the marker guard rather than just exercising it.

Findings below are docs-drift and script-robustness only; nothing blocking. The most consequential is the teardown marker gap — make demo-promoted-workflow-teardown does not clean up the sandbox the walkthrough builds by hand.

One item outside the diff, for a maintainer rather than a reviewer: #216 was filed publicly on the reasoning that cross-leaf reading is an accepted non-goal (P2 §9) and the news is cache lifetime. That reasoning is written into the issue for override, which is the right call, but it is a judgment I cannot settle from the PR.

Review posted as COMMENT rather than APPROVE only because GitHub rejects self-approval.

Assisted-By: Claude Code

Comment thread docs/demos/promoted-workflow-demo.md Outdated
# Cleanup

```bash
make demo-promoted-workflow-teardown # removes the sandbox (refuses any dir it did not create)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not remove the sandbox the walkthrough built. --teardown gates every destructive touch on the .sh-demo-sandbox marker, and only reset_sandbox creates it — Act 1a (125-134) builds the sandbox by hand, and the marker string appears nowhere in this doc. So a reader who followed the walkthrough gets nothing to remove at /tmp/sh-demo (no .sh-demo-sandbox marker) and keeps the directory.

The parenthetical "refuses any dir it did not create" is exactly why it fails. Either add touch $SANDBOX/.sh-demo-sandbox to Act 1a (which also makes the marker's purpose concrete at the point it matters), or tell the reader to rm -rf $SANDBOX here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b5fd240 — this was the real one. Act 1a now does touch $SH_DEMO_SANDBOX/.sh-demo-sandbox, with a comment saying why at the point it matters. Verified both directions: a marked dir is removed, an unmarked one still gets nothing to remove. The Cleanup parenthetical now reads "needs the Act 1a marker" rather than advertising the refusal that was causing the failure.

Comment thread docs/demos/promoted-workflow-demo.md Outdated
### 2c. Re-promotion is free

```bash
HOME=$SANDBOX pnpm --dir harness promote --entry ship-note --project $SANDBOX | tail -3

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--dir harness resolves to /tmp/sh-demo/harness at this point: line 153 is cd $SANDBOX && claude, and the callout at 160-161 tells the reader to drive the rest of the demo from that shell.

Line 170 writes the same command as --dir ~/path/to/serverless-harness/harness, so the doc gives two different forms for one invocation. Suggest exporting it once alongside the other vars at 68-72 — e.g. export HARNESS=$HOME/src/serverless-harness/harness — and using --dir $HARNESS in both places. That also retires the ~/path/to/... placeholder, which mixes a real ~ with a fake path.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b5fd240. export HARNESS=$(pwd)/harness joins the Act 0 vars, and both invocations use --dir $HARNESS, so the ~/path/to/... placeholder is gone. You were right about the cwd too — from Act 1b onward the shell is in the sandbox, so --dir harness was resolving to $SANDBOX/harness; the new comment on the export says exactly that.

Comment thread docs/demos/promoted-workflow-demo.md Outdated
holds even on a run where the model declines to read:

```bash
kubectl exec -n $NS sandbox-0 -- \

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded sandbox-0, and this is the load-bearing filesystem proof — the one that holds when the model declines to read.

The script cannot assume the pod: it enumerates POOL_PODS by selector and breaks on the first one holding release-token.md, precisely because the leaf may have leased any pool sandbox. This doc's own purge loop at 261 uses the selector too. If the leaf leased something other than sandbox-0, this cat fails and reads as a broken feature rather than a missed guess.

Suggest the same for p in $(kubectl get pods -n $NS -l 'sh.kagenti.io/sandbox-pool=default' -o name | sed 's|pod/||') loop used at 261. Same applies to the exploratory exec at 246, though the stakes there are lower.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b5fd240. $POOL is derived once by selector in Act 3a and both the exploratory find (246) and the load-bearing cat (327) now loop over it, breaking on the first hit like the script does. Added a callout that a hardcoded pod fails as "broken feature" when it was only a wrong guess.

# --- Claim 2: re-promotion of unchanged config uploads nothing --------------------------------
claim 2 "re-promoting unchanged configuration uploads nothing"
repromote_log="$(mktemp)"
HOME="$SANDBOX" pnpm --dir "$REPO_ROOT/harness" promote \

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This promote is unguarded, unlike the identical call at 207. Under set -euo pipefail a non-zero exit here terminates the script immediately: no ko, no cat "$repromote_log", and no === Results === line — the one failure mode in this script that looks like a crash rather than a test failure.

Wrapping it the way Claim 1 does would also let you move $promote_log/$repromote_log into cleanup(). Today the rm -f at 346 is only reached on the happy path, so both temp files leak on every exit 1 above it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b5fd240. Wrapped, but reporting and continuing rather than exiting: Claim 3 needs only $DIGEST from Claim 1, so aborting here would hide the A/B behind a re-promotion failure. Both temp logs are now initialised before the EXIT trap and removed in cleanup(), so they no longer leak on any early exit.

Comment thread docs/demos/promoted-workflow-demo.md Outdated
export NS=default KSVC=serverless-harness
export HOSTHDR='Host: serverless-harness.default.example.com'
export BASE=http://localhost:8080
export SANDBOX=/tmp/sh-demo

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These export SANDBOX and MODEL, but the script and the make targets read SH_DEMO_SANDBOX, SH_MODEL and SH_DEMO_REDIS_PORT. It works here only because the values happen to equal the script's defaults.

A reader who overrides them (say export SANDBOX=/tmp/mine) and then runs make demo-promoted-workflow at 373 silently gets the defaults instead — and the hand-driven half of the walkthrough would be operating on a different directory than the scripted half. Worth using the SH_* names here, or noting them next to the make targets.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b5fd240. The walkthrough now exports and uses SH_DEMO_SANDBOX, SH_DEMO_REDIS_PORT and SH_MODEL throughout, with a comment saying these are the names the script and make targets read. Your override scenario is exactly the failure — it worked only because the values matched the defaults.

--keep-sandbox) KEEP_SANDBOX=1 ;;
--teardown) TEARDOWN=1 ;;
-h | --help)
sed -n '2,32p' "$0"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2,32p includes line 32, which is set -euo pipefail — so --help ends with a stray line of shell. The last comment line is 31 (--teardown # remove the sandbox and exit), so 2,31p.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b5fd240, and it uncovered more than the range. Correcting to 2,31p made me run --help, which failed with a sed error: cd "$(dirname "$0")" happens before arg parsing, so the documented relative invocation no longer resolves argv[0]. $SELF is now resolved before the cd. Verified from the repo root, via an absolute path, and from an unrelated cwd.

kubectl wait ksvc/$KSVC -n $NS --for=condition=Ready --timeout=180s

EOF
echo "=== Results: $PASS passed, $((FAIL)) failed ==="

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: $((FAIL)) arithmetic-expands a plain variable read. $FAIL, matching the final results line at 348.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b5fd240$FAIL, matching the results line.

Comment thread docs/demos/promoted-workflow-demo.md Outdated

---

# Cleanup

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: # Cleanup here and # Notes and limits at 395 are H1, while every other top-level section is H2 (## Act 0## What just happened). That gives the page three H1 titles. Demote both to ##.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b5fd240. Worth noting remote-sandbox-demo.md uses H1 for every section including its Acts — self-consistent, just a different choice — so demoting these two removes the inconsistency inside this page rather than creating one against its sibling. Happy to align both if you would rather the folder converge.

scaled-to-zero Revision) could not be taken during this implementation: the deployed image
predates this branch, and taking it needs a full image build, `kind load`, and a forced new
Revision. It remains owed; reproduction, once such a cluster is available:
Revision. It has since been taken (see **In-cluster result**, below); the reproduction is:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "…the reproduction is:" promises a code block, but the paragraph at 472-474 intervenes and ends with its own lead-in ("What was actually run:"), leaving this colon dangling. Dropping it — "It has since been taken (see In-cluster result, below)." — leaves 474 as the single lead-in.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b5fd240 — dropped to "It has since been taken (see In-cluster result, below)." so line 474 is the single lead-in.

The gate had never fired, and it could not have: both session ids used the `<run>/<item>` shape,
whose slash the session-id validator rejects. Run against a cluster, the first test failed in
597 ms with

  Session id must be non-empty, contain only alphanumeric characters, '-', '_', and '.', and
  start and end with an alphanumeric character

so it died before any model call and never reached the sibling-read assertion it exists to make.
Both ids now use '-'.

The second test was passing for a reason unrelated to what it asserts, which is why nothing looked
wrong: an unknown digest fails before session-id validation is reached, so `toContain('not found')`
held while its id was equally invalid. Fixed too, so it asserts on the path it names.

The `<run>/<item>` shape is correct where it appears elsewhere -- leaf-smoke.sh and the leaf
fixtures go through an item envelope, where `leafSessionId` derives a sanitized id. These tests call
runLeaf with `kind: 'prompt'`, whose sessionId is the session key verbatim. A comment records that
distinction so the ids do not get "tidied" back.

Verification, against kind with a pool and a real model:

  SH_PROMOTE_LIVE_SMOKE=1 ... pnpm exec vitest run test/promote-live-smoke.test.ts
  -> 2 passed (8.4s); the leaf returns PROMOTED-SIBLING-OK

That string is only producible by resolving a relative sibling path from a promoted skill's
instructions against the absolute skills directory the leaf injects, inside the sandbox pod -- so
spec §4.5 path translation is now demonstrated by the gate written for it, not only by the demo's
HTTP equivalent. Unset the env and both tests still skip; the full harness suite is 348 passed /
5 skipped / 0 failed.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
@pdettori pdettori changed the title docs(demos): promoted-workflow demo, driven and scripted docs(demos): promoted-workflow demo, and the live gate it made runnable Sep 3, 2026
…tion, unguarded promote

All nine review findings, plus one the review pointed near but did not name.

Substantive:

- **The scripted teardown could not clean the hand-built sandbox.** `--teardown` gates every
  destructive touch on the `.sh-demo-sandbox` marker, and only `reset_sandbox` wrote it, so a reader
  who followed Act 1a got "nothing to remove (no .sh-demo-sandbox marker)" and kept the directory.
  Act 1a now creates the marker, at the point where its purpose is concrete. Verified: teardown
  removes a marked dir and still refuses an unmarked one.
- **The load-bearing filesystem proof hardcoded `sandbox-0`.** The leaf leases whichever pool
  sandbox is free -- which is exactly why the script enumerates by selector and breaks on the first
  hit -- so a fixed pod reads as a broken feature when it was a wrong guess. Act 3a and 3d now loop
  over `$POOL`, derived once by selector.
- **The re-promote was unguarded** under `set -euo pipefail`: a non-zero exit killed the run with no
  `ko`, no log dump and no results line, the one failure that looked like a crash. Now reported and
  survived, because Claim 3 needs only `$DIGEST` from Claim 1 and aborting would hide the A/B behind
  it. Both temp logs moved into `cleanup()`, so they no longer leak on early exit.
- **`--help` was broken for the documented invocation.** Not in the review: the range fix (`2,32p`
  spilled `set -euo pipefail`) exposed that `cd "$(dirname "$0")"` runs before arg parsing, so a
  relative argv[0] no longer resolves and `--help` printed a sed error. `$SELF` is resolved before
  the cd; verified from the repo root, via an absolute path, and from an unrelated cwd.

Docs drift:

- Act 0 exported `SANDBOX`/`MODEL` while the script and make targets read `SH_DEMO_SANDBOX`/
  `SH_MODEL`/`SH_DEMO_REDIS_PORT`; it only worked because the values matched the defaults, and an
  override would have split the hand-driven and scripted halves onto different directories. The
  walkthrough now uses the `SH_*` names throughout.
- `--dir harness` and `--dir ~/path/to/serverless-harness/harness` were two forms of one command,
  and the first resolved to `$SANDBOX/harness` because Act 1b moves the reader's cwd. Both now use
  `$HARNESS`, exported once in Act 0, retiring the `~`-plus-fake-path placeholder.
- Two stray H1s (`# Cleanup`, `# Notes and limits`) among H2 sections, demoted. Note
  `remote-sandbox-demo.md` is consistently H1 for every section -- a different but self-consistent
  choice -- so this only removes the inconsistency inside this page.
- `$((FAIL))` -> `$FAIL`; the spec's "the reproduction is:" no longer dangles, since a paragraph
  with its own lead-in intervenes before the block.

Verification: `make lint` (9 hooks) and `make test-deploy` (94 checks) pass, and the demo still
returns 14 passed / 0 failed against kind after all of it.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
@pdettori

pdettori commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

All nine findings addressed in b5fd240, with a reply on each thread. Summary, since the review's
"most consequential" call was right:

The teardown gap was a genuine defect in the walkthrough, not drift: a reader who followed Act 1a
and then ran make demo-promoted-workflow-teardown kept the directory, and the parenthetical
advertising the refusal was the reason. Act 1a now creates the marker. Verified both directions — a
marked dir is removed, an unmarked one still refuses.

Two others were also behavioural, not cosmetic. The hardcoded sandbox-0 in the load-bearing
filesystem proof would have failed as "broken feature" whenever the leaf leased a different pool
sandbox; both execs now loop over a $POOL derived by selector. And the unguarded re-promote is
wrapped — reporting and continuing rather than exiting, because Claim 3 needs only $DIGEST from
Claim 1, so aborting would have hidden the A/B behind a re-promotion failure. Both temp logs moved
into cleanup().

One finding uncovered a bug it did not name. Correcting 2,32p2,31p made me actually run
--help, which failed with a sed error: cd "$(dirname "$0")" runs before arg parsing, so the
documented relative invocation no longer resolved argv[0]. $SELF is resolved before the cd now, and
--help is verified from the repo root, via an absolute path, and from an unrelated cwd.

On the H1 nit, one thing worth a maintainer's call rather than my guess: remote-sandbox-demo.md
uses H1 for every section, including its Acts. That is self-consistent, just a different choice, so
demoting my two removes an inconsistency inside this page rather than creating one against its
sibling. Say the word if you would rather the folder converge on one style.

Verification after all nine: make lint (9 hooks) and make test-deploy (94 checks) pass, and the
demo still returns 14 passed / 0 failed against kind.

On #216 — agreed that the public-vs-advisory judgment is yours, not mine. The reasoning is written
into the issue precisely so it can be overruled; happy to convert it to an advisory if you read P2 §9
differently.

@pdettori
pdettori merged commit ce7bcda into rossoctl:main Sep 3, 2026
12 checks passed
@pdettori
pdettori deleted the demo/promoted-workflow branch September 3, 2026 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant