Skip to content

feat(task): a task's resolved values reach a step's process environment as DATA - #47

Merged
github-actions[bot] merged 1 commit into
mainfrom
feat/stepenv-data-channel
Oct 24, 2026
Merged

github-actions[bot] merged 1 commit into
mainfrom
feat/stepenv-data-channel

Conversation

@aitrawog

Copy link
Copy Markdown
Contributor

Summary

A task's params: reach a plan step only as text: the expander substitutes the value into the step's command, and the shell then PARSES that text — so a value that is DATA becomes SOURCE, and a $(…) inside a value EXECUTES with the step's privileges. This change opens the second channel. The task's resolved values — its vars, its env, its resolved params and TASK_DIR — are handed to the SDK as kit.RunnerConfig.StepEnv, so they reach every step as the step's process environment, where the shell never re-parses them. The change is 7 files, +401 −45 (Head 253120997a33e1c021fb7c88609a78ef84f8b1de).

Closes #34.

Three rules bound the channel, and each is a rule the SDK's own merge already owns, so the plugin adds no second implementation of any of them:

  • the step's OWN env: WINS a name collision — the author spoke last about their own step, and the task's value is a default;
  • a name the shell cannot export (outside [A-Za-z_][A-Za-z0-9_]*) is REJECTED and RETURNED, and the SDK renders it as a NOTE on the step's message ("not exported (not a valid shell identifier): …") — disclosed, never dropped silently;
  • a piped stdin is not a resolved value, so it is NOT exported. The map is CLONED before captureStdin mutates it, because the command verb renders an exported value into the command TEXT, and a NUL byte (a piped binary stream carries one) would make the exec un-runnable. The stream keeps its own channel: TASK_STDIN / TASK_STDIN_FILE stay reachable through the text expansion they have always had.

A lowercase param is a second, independent win: the expander's grammar is uppercase-only, so a lowercase param was previously unreachable from a step at all. The environment is what makes it reachable BY NAME.

RCA

  • Mechanism: the task engine had exactly ONE route from a resolved value to a step — the expander, which substitutes the value into the command TEXT. A shell parses the text it is given, so the value is re-parsed on the way: a $(…) inside it EXECUTES, and a ; or a space splits it into words. The value changed from DATA into SOURCE at the moment it crossed that channel.
  • Missed control: nothing asserted that a value stays DATA. The expander's contract was "the value's TEXT is substituted", and there was no second channel, so the only way to reach a step was the unsafe one; and the suite asserted what a step's command RECEIVED, never what a param's value was DELIVERED as. A value that arrived as text and a value that arrived as data were indistinguishable to it.
  • Blast radius: every task that passes a value into a step — a param carrying a path with a space, a command substitution, a shell metacharacter, or a lower-case name.
  • Root fix: the environment is the second channel, and it is the SDK's channel, not a plugin-local one: kit.RunnerConfig.StepEnv + kit.MergeStepEnv (opencharly/sdk#475, released as sdk v0.2026297.1851). The plugin hands over the resolved map and lets the SDK own the merge rules; the plugin's own contribution is the clone before the stdin capture, because only the plugin knows that the map it hands over is about to be mutated with a runtime stream.

Producer-first

The DATA channel is an SDK capability, so this module's require rises to sdk v0.2026297.1851. This repo's own coherence control (scripts/check-coherence.sh, run by the coherence CI job on every PR and by the live gate before its build) asserts that charly's sdk/spec pins LEAD this module's requires, so the producer leg had to land first — and it has: opencharly/charly#1062 merged as 87f6302456f9c821877e719cbe90615368cd85bc with the merge-time tag v2026.297.1957, and charly's main now pins sdk v0.2026297.1851 / spec v0.2026296.1036. The coherence control against main is pasted under How tested, and the live gate ran against main — the configuration that ships.

The legs that follow this one:

  1. opencharly/charly#1062 — charly adopts sdk v0.2026297.1851 (MERGED, v2026.297.1957);
  2. this PR — this repo raises its own require and implements the consumer;
  3. charly adopts the new plugin-task tag this PR produces;
  4. the umbrella advances its charly and plugin-task gitlinks.

The files this diff touches

file what this diff does to it
README.md the user-facing section that documents the new channel: what reaches a step, the three rules that bound it, and the lower-case-param win
candy/plugin-task/go.mod the sdk require rises to v0.2026297.1851, the release that brings the DATA channel; the spec require and the go directive move with it
candy/plugin-task/go.sum the sums for the new sdk release, and the transitive bumps go mod tidy resolved alongside it
candy/plugin-task/runner.go the resolved map is CLONED before the stdin capture and handed to the SDK as kit.RunnerConfig.StepEnv
candy/plugin-task/stepenv_test.go new: the channel's contract at unit level — the export, the collision rule, the disclosure of a non-exportable name, the stdin exclusion
scripts/check-coherence.sh one comment clause: the historical measurement named "this module's v0.2026276.1822", a pin this module no longer carries, so it now names the pin the module carried at the time of that measurement
scripts/gate-task.sh the live arm: a param carrying $(touch <marker>) runs against a real charly binary, and the arm asserts the value arrives VERBATIM and that the marker file does not exist

How tested

Every command below ran on the committed tree this PR pushes — git rev-parse HEAD prints the pushed commit, and git status --porcelain printed nothing on it.

$ git rev-parse HEAD
253120997a33e1c021fb7c88609a78ef84f8b1de
$ git status --porcelain
STATUS_RC=0

The module gate, in candy/plugin-task/, under the toolchain the module's own go.mod DECLARES (go 1.26.8) — which is the toolchain CI installs, via go-version-file: candy/plugin-task/go.mod. The linter is the version CI pins, and it runs under that toolchain because golangci-lint cannot read the export data of a newer GOROOT.

== go build ./... ==
BUILD_RC=0
== go test -count=1 ./... ==
ok  	github.com/opencharly/plugin-task/candy/plugin-task	3.817s
?   	github.com/opencharly/plugin-task/candy/plugin-task/cmd/serve	[no test files]
?   	github.com/opencharly/plugin-task/candy/plugin-task/params	[no test files]
TEST_RC=0
== gofmt -l . ==
GOFMT_LIST=[]
GOFMT_RC=0
== go vet ./... ==
VET_RC=0
== golangci-lint (the module's own toolchain) ==
0 issues.
LINT_RC=0

The candy manifest, validated at the repo root with the worktree-local charly build:

charly box validate: OK — checked 1 candy, 7 distros, 7 builders; 0 warnings, 0 errors
BOXVALIDATE_RC=0

The two steps of the CI coherence job — the SAME control, the same two callers — one against the control's own synthetic fixtures, one against charly main's live go.mod over HTTP:

== scripts/check-coherence.sh --self-test (the CI coherence job, step 1) ==
self-test OK: leading passes, lagging fails, absent pin fails
COHERENCE_SELFTEST_RC=0
== charly main's pins lead this module's requires (the CI coherence job, step 2) ==
CURL_RC=0
coherence OK: charly main's sdk/spec pins lead this module's requires
COHERENCE_MAIN_RC=0

The live gate — the repo's own scripts/gate-task.sh, which clones charly at main, compiles this candy IN, and drives command:task plus the five maintenance verbs against real git fixtures. This is the ONLY live execution of those verbs, and it is where the DATA channel is proven end to end: the data arm's param carries a command substitution, and the arm asserts BOTH outcomes — the value arrives VERBATIM, and the marker file the substitution would have created does NOT exist.

== fetching charly main (compiles candy/plugin-task in) ==
Cloning into '/tmp/plugin-task-gate.0wr6iF/charly'...
coherence OK: charly main's sdk/spec pins lead this module's requires
== building charly with candy/plugin-task compiled in from the working tree ==
== charly task list ==
task failtask: 1 step(s), 1 failed, 0 skipped
  [fail] run — this fails
      exit=1, want 0 (stderr: )
charly: error: command "task": task "failtask" failed (1 step(s) failed)
== a step that ran nothing is counted in its own bucket ==
   | task skipped-step: 1 step(s), 0 failed, 1 skipped
   |   [skip] run — this step ran nothing
   |       skip: true
   a step that ran nothing is counted, disclosed, and non-fatal
== a SILENT task still discloses the step that ran nothing ==
   | task silent-skip: 2 step(s), 0 failed, 1 skipped
   |   [skip] run — the silent step that ran nothing
   |       skip: true
   the silent path keeps its meaning and still discloses the skip
== verb:task nested hop (a composite must carry the leaf step's own reason) ==
   | task composite: 1 step(s), 1 failed, 0 skipped
   |   [fail] run — the outer step (a nested task step)
   |       task "leafreason" failed (1 step(s) failed, 0 skipped
   |         [fail] run — the leaf step (its Message is the reason)
   |             exit=3, want 0 (stderr: leaf-reason: boom on stderr))
   | charly: error: command "task": task "composite" failed (1 step(s) failed)
   the composite carried the leaf step's reason AND its path across the hop
== a param reaches a step as DATA (the shell must not parse it) ==
   | task datachannel: 1 step(s), 0 failed, 0 skipped
   |   [pass] run — the param arrives in the step's process environment, verbatim
   |       exit=0
   the param arrived as data: verbatim, and unexecuted
== verb:git-submodules verify (stale -> must FAIL) ==
task pins-verify: 1 step(s), 1 failed, 0 skipped
  [fail] check — the pin holds
      verify: 1 failure(s): pin mismatch: target (260dd68fab9537d90ad37fc007fc05f9a13e6571) != ../src's target (6cabf62ce5c80b1973c22f1b42c3d7277d69ee41)
charly: error: command "task": task "pins-verify" failed (1 step(s) failed)
== verb:git-submodules bump (must move the pin to src's twin) ==
== verb:git-submodules verify (now current -> must PASS) ==
== verb:task nested hop (a composite must carry the nested run's OWN report) ==
   | task pins-verify-nested: 1 step(s), 0 failed, 0 skipped
   |   [pass] run — the outer step (a nested task step)
   |       verify: 1 policy pin(s) hold across 1 submodule(s); 1 checkout(s) clean at their gitlink
   the composite carried the nested pin report across the hop
== verb:git-submodules verify (missing twin -> must FAIL loudly, never vacuous) ==
task missing-pin: 1 step(s), 1 failed, 0 skipped
  [fail] check — the missing twin fails
      verify: 1 failure(s): no gitlink for "nope/absent" in "../src"
charly: error: command "task": task "missing-pin" failed (1 step(s) failed)
== verb:git-submodules bump (unmaterialized submodule -> the pin advances IN THE INDEX, no checkout) ==
   | task bump-all: 1 step(s), 0 failed, 0 skipped
   |   [pass] run — bump
   |       pinned 0 pin_map entr(y|ies) () after rolling 0 submodule(s) to their default-branch HEAD and advancing 1 pin(s) in the index
   unmaterialized pin advanced in the index (260dd68fab9537d90ad37fc007fc05f9a13e6571 -> 6cabf62ce); no checkout created; umbrella untouched (main @ 47a839024)
== verb:git-submodules status (uninitialized submodule -> '- (uninitialized)', never the umbrella HEAD) ==
   status reported '- (uninitialized)'; umbrella HEAD (47a8390) not leaked as a pin
== verb:git-submodules bump (pinned_from with NO CHECKOUT and no 'branch=' -> the refusal names the missing checkout) ==
   | task bump-from-source: 1 step(s), 1 failed, 0 skipped
   |   [fail] run — bump
   |       1 pin(s) FAILED: refused to pin 1 pin_map entr(y|ies): pinned_from "src" has no checkout, so its twin gitlinks cannot be read — materialize it with `git submodule update --init src`; rolled 0 submodule(s) to their default-branch HEAD; advanced 0 pin(s) in the index; pinned 0 pin_map entr(y|ies): 
   | charly: error: command "task": task "bump-from-source" failed (1 step(s) failed)
   the refusal named the missing checkout and the materialize command; the stale diagnosis is gone
== verb:git-submodules bump ORDER (pinned_from is itself rolled first) ==
== verb:git-submodules currency (after the ordered bump every pin must be CURRENT) ==
== verb:git-submodules currency (an edge over an edge must be REFUSED, never answered) ==
== verb:git-submodules currency (an EXPLICIT edge over an edge must be REFUSED too) ==
== verb:git-submodules currency (stale_after: a pin NEGLECTED past the bound must FAIL) ==
   | task currency-stale: 1 step(s), 1 failed, 0 skipped
   |   [fail] check — no pin is neglected past the bound
   |       currency: 1 of 1 pin(s) NOT current [1 behind]; intents: 1 upstream leaf/leaves (1 implicit), 0 derived-from edge(s); NOT CURRENT: gap: behind; intent upstream:main (implicit: .gitmodules branch=); recorded 260dd68fab; served 6cabf62ce5 (branch); 1 STALE pin(s) (neglected for more than 168h): gap: behind, and this repo has not advanced it for 59732h (bound 168h) — advance it: mode=bump advances every pin to the ref its intent names; FAILED on the STALE pin(s) above; producer-first legs: gap (260dd68fab -> 6cabf62ce5)
   | charly: error: command "task": task "currency-stale" failed (1 step(s) failed)
== verb:git-submodules currency (stale_after: the SAME pin, advanced recently, must PASS) ==
   the same behind pin, advanced recently, passed the bound
== verb:git-submodules currency (stale_after: a NON-POSITIVE bound must be REFUSED) ==
   | task currency-nonpositive: 1 step(s), 1 failed, 0 skipped
   |   [fail] check — no pin is neglected past the bound
   |       git-submodules: stale_after "0s" is not a POSITIVE duration — a bound of zero or less marks every not-current pin stale, which is the permanently-red gate this bound exists to avoid
   | charly: error: command "task": task "currency-nonpositive" failed (1 step(s) failed)
   a bound of zero was refused, and no staleness was judged
== verb:git-submodules currency (an UNREADABLE history is not neglect) ==
   the fixture's history read genuinely fails, and the healthy fixture's succeeds
   | task currency-unreadable: 1 step(s), 1 failed, 0 skipped
   |   [fail] check — no pin is neglected past the bound
   |       currency: 1 of 1 pin(s) NOT current [1 behind]; intents: 1 upstream leaf/leaves (1 implicit), 0 derived-from edge(s); NOT CURRENT: gap: behind; intent upstream:main (implicit: .gitmodules branch=); recorded 260dd68fab; served 6cabf62ce5 (branch); 1 STRUCTURAL failure(s): gap: the age of this repo's own last advance of it could not be read (fatal: bad object refs/heads/main) — the staleness of this pin cannot be judged; producer-first legs: gap (260dd68fab -> 6cabf62ce5)
   | charly: error: command "task": task "currency-unreadable" failed (1 step(s) failed)
   an unreadable history was reported as unreadable, not as neglect
== verb:file-parity (identical must PASS, drift must FAIL) ==
task parity: 1 step(s), 1 failed, 0 skipped
  [fail] check — the pair is identical
      parity drift in 1 pair(s): r != r.copy
charly: error: command "task": task "parity" failed (1 step(s) failed)
== verb:splice-region (stale -> check must FAIL, sync -> current) ==
task splice-check: 1 step(s), 1 failed, 0 skipped
  [fail] check — the region is current
      splice-region: TARGET is stale (the marked region differs)
charly: error: command "task": task "splice-check" failed (1 step(s) failed)
== verb:module-pins (stale -> must FAIL, adopt -> must PASS) ==
task mods-check: 1 step(s), 1 failed, 0 skipped
  [fail] check — the pins hold
      stale pins: tools/m1: github.com/opencharly/sdk != v1.2.3
charly: error: command "task": task "mods-check" failed (1 step(s) failed)
== verb:prune (removes merged worktree+branch, keeps unmerged) ==
== verb:prune (uninitialized submodule -> the SUPERPROJECT is never attributed to it) ==
   uninitialized submodule contributed nothing; the project's own targets still reported
== verb:prune mode=prune with an uninitialized submodule present (must complete cleanly) ==
   mode=prune completed cleanly; unmerged kept, merged reaped, uninitialized dir intact
gate-task: PASS — command:task + all five maintenance verbs executed live against charly main
GATE_RESTORED_RC=0

The R7 proof: the coverage must fail WITHOUT the change. The mutant drops the PARAM from the channel — the plugin still hands the SDK a StepEnv map, but the map carries only TASK_DIR — and the mutant COMPILES, so the gate reaches the data arm and the ARM turns red with its own verdict. That is the honest direction: the arm observes the BEHAVIOUR, not a compile error. The tree was restored afterwards and the gate run once more on it, which is the second half of the log.

=== the mutation ===
MUTATION APPLIED
MUTATION_APPLIED_RC=0
=== the live gate against the mutated tree ===
== a param reaches a step as DATA (the shell must not parse it) ==
FAIL: the param must reach the step's process environment VERBATIM, got ''
task datachannel: 1 step(s), 0 failed, 0 skipped
  [pass] run — the param arrives in the step's process environment, verbatim
      exit=0
GATE_MUTANT_RC=1
=== restore ===
RESTORE_RC=0
(the status above must be empty)
=== the live gate again, on the restored tree ===
== a param reaches a step as DATA (the shell must not parse it) ==
   | task datachannel: 1 step(s), 0 failed, 0 skipped
   |   [pass] run — the param arrives in the step's process environment, verbatim
   |       exit=0
   the param arrived as data: verbatim, and unexecuted
gate-task: PASS — command:task + all five maintenance verbs executed live against charly main
GATE_RESTORED_RC=0

The R5 sweep — the old sdk pin is gone from this repo's module, and the new one is in it:

$ git grep -n 'github.com/opencharly/sdk v0.2026276.1822' -- '*/go.mod' '*/go.sum'
$ echo "grep exit=$?"
grep exit=1
$ git grep -n 'github.com/opencharly/sdk v0.2026297.1851' -- '*/go.mod'
candy/plugin-task/go.mod:6:	github.com/opencharly/sdk v0.2026297.1851
NEWGREP_RC=0

grep exit=1 is git grep's "no such line" status: the old pin appears in no go.mod and no go.sum in this repo. Over the WHOLE tree the old string survives in seven lines, and the count listing names every file that carries one — three CHANGELOG/ entries (history) and the coherence control:

$ git grep -c 'v0.2026276.1822' -- .
CHANGELOG/2026.277.0711.md:3
CHANGELOG/2026.278.0508.md:1
CHANGELOG/2026.279.1213.md:1
scripts/check-coherence.sh:2
$ echo "grep exit=$?"
grep exit=0

The three CHANGELOG/ files are history. The two lines of the coherence control are the historical measurement this diff rewords, and the control's SYNTHETIC self-test fixtures — a leading pair, a lagging pair and an absent pin — which do not read this module's live pins.

Rulebook compliance

  • R0 — the skills this change's area selects were loaded before the first tool action: /charly-internals:plugin, /charly-image:layer, /charly-internals:git-workflow. The harness resolves no /charly-<family>:<skill> registration, so the rulebook's own fallback was taken and each skill was read by path from the marketplace checkout.
  • R1 — the RCA above names the mechanism, the missed control, the blast radius and the root fix. Two anomalies arose in this change's own work, and both were root-caused rather than worked around. The first is the mutation's own design: the first mutant removed the StepEnv field from the runner, which left the local variable unused and turned the mutant into a COMPILE error — and a compile failure is not a behavioural proof, because the gate would have gone red for a reason that says nothing about the channel. The missed control was the mutation, not the code: it changed the API surface instead of the DATA the API carries. The root fix went into the control — the mutant now drops the param from the map and keeps the field — and the log above carries both directions. The second is a staleness this pin bump created: the coherence control's header comment named "this module's v0.2026276.1822" as this module's pin, which this diff stops being true of. The measurement is history and stays; the false present-tense claim does not, and the clause is reworded in this same cutover.
  • R2 — one implementation of the merge rules. The collision rule, the rejection of a non-exportable name and the note that discloses it are the SDK's (kit.MergeStepEnv), not a second copy in this plugin. The plugin's contribution is the map it hands over and the clone that keeps a runtime stream out of it.
  • R3 — no workaround: the channel is the SDK's own mechanism, reached by raising the require to the release that carries it. No shim, no local fork of the merge, no pinned-downgrade of the dependency, no sleep and no retry.
  • R4 — the product first, then the prose: the runner, the tests and the live arm are in this diff, and the README section that documents the channel is in the same cutover. Every command the README names is the command the gate runs.
  • R5 — the old sdk pin is gone from this repo's module and the new one is in it; the claim-keyed grep above is the evidence, and it names every place the old string survives and why each one is not a live pin.
  • R6 — git status was read before the commit, and the tree was clean when the commit was made. The mutation was applied to the working tree and restored with git checkout -- on the same file, and the restore is in the log above with the status beside it. There is no amend after the push, no force-push, no hook bypass, and no push to main.
  • R7 — the coverage fails without the change: the mutant above drops the param from the channel, the gate's data arm turns red with its own verdict, and the restored tree turns it green again. The changed path executed live — the data arm ran against a real charly binary with this candy compiled in — and the unit tests cover the same contract at the export, the collision, the disclosure and the stdin exclusion.
  • R8 — no live service is mocked or claimed. The gate crosses no service boundary: it builds a real charly binary from main and runs the real commands against real git fixtures.
  • R9 — the gate ran against the worktree-local charly build, never a shared install.
  • R10 — the change-class gate ran on the committed tree this PR pushes: the module's build, test, vet, gofmt and linter steps, charly box validate, the CI coherence job's two steps, and the repo's live gate. Zero warnings, zero failures.

## Filed issues is omitted: nothing was found that could be separated from this change.

Change classification

Runtime change to a plugin library: the task engine gains a channel, the sdk require rises to the release that carries the channel, the live gate grows the arm that proves it, and the unit tests cover its contract.

The tier is analysed on a live system, not fully tested and validated: this repo's own gate and its live gate both ran on the committed tree and the changed path ran live, but the FULL assembled bed is charly's, and by the producer-first order it cannot run on this head — the tag this change produces does not exist until this PR merges, and charly cannot pin it before that.

Agent: charly

Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)

…nt as DATA

A task's `params:` are substituted into a plan step's TEXT, and the shell then
PARSES that text. A value that is DATA therefore becomes SOURCE: a `$(…)` inside
a value EXECUTES. The text channel cannot fix that — the value has to arrive by a
route the shell does not re-parse.

The environment is that route. A value delivered as an environment variable is
DATA, because the shell never re-parses the result of an expansion, and the
command verb applies the step's env as a shell-QUOTED export, so the value
arrives as ONE word whatever it contains. This change opens that channel: the
task's RESOLVED values — its `vars`, its `env`, its resolved `params` and
`TASK_DIR` — are handed to the SDK as `kit.RunnerConfig.StepEnv`, and the SDK's
`kit.MergeStepEnv` merges them into every step's environment.

Three rules bound the channel, and each is a rule the SDK's own merge already
owns, so the plugin adds no second implementation of any of them:

  - the step's OWN `env:` WINS a name collision — the author spoke last about
    their own step, and the task's value is a default;
  - a name the shell cannot export (outside `[A-Za-z_][A-Za-z0-9_]*`) is
    REJECTED and RETURNED, and the SDK renders it as a NOTE on the step's
    message ("not exported (not a valid shell identifier): …") — disclosed,
    never dropped silently;
  - a piped stdin is not a resolved value, so it is NOT exported. The map is
    CLONED before `captureStdin` mutates it, because the command verb renders an
    exported value into the command TEXT, and a NUL byte (a piped binary stream
    carries one) would make the exec un-runnable ("fork/exec /usr/bin/sh:
    invalid argument"). The stream keeps its own channel: `TASK_STDIN` /
    `TASK_STDIN_FILE` stay reachable through the text expansion they have
    always had.

A lowercase param is a second, independent win: the expander's grammar is
uppercase-only, so a lowercase param was previously unreachable from a step at
all. The environment is what makes it reachable BY NAME.

The evidence is a live arm, not a unit test alone. `scripts/gate-task.sh` grew
arm 1c: a task whose param carries `$(touch <marker>)` runs against a real
charly binary, and the arm asserts BOTH outcomes — the value arrives VERBATIM
in the step's environment, AND the marker file does not exist. The silent
outcome is the failure this channel exists to remove, so it is asserted too.
`candy/plugin-task/stepenv_test.go` covers the same contract at unit level: the
export, the collision rule, the disclosure of a non-exportable name, and the
stdin exclusion.

The pin bump also made ONE line of this repo's coherence control stale: its header
comment read "Measured at charly v2026.267.2313 (pinning sdk v0.2026266.1111 against
this module's v0.2026276.1822)", and "this module's v0.2026276.1822" named the pin
this module no longer carries. The measurement is history and stays; the false
present-tense claim does not, so the clause now reads "against a module pinning sdk
v0.2026276.1822 - the pin this module carried at the time of that measurement". The
control's self-test fixtures keep their versions on purpose: they are SYNTHETIC
(a leading pair, a lagging pair, an absent pin) and do not read this module's live
pins, so they still prove the comparator can pass and can fail.

Producer-first: the DATA channel is an SDK capability (`kit.StepEnvExporter`,
`kit.RunnerConfig.StepEnv`, `kit.MergeStepEnv`, `opencharly/sdk#475`, released as
`sdk v0.2026297.1851`), and this module's `require` therefore rises to it. This
repo's own coherence control (`scripts/check-coherence.sh`, run by the `coherence`
CI job on every PR) asserts that charly's `sdk`/`spec` pins LEAD this module's
requires, so the producer leg had to land first — and it has:
`opencharly/charly#1062` merged as `87f6302456f9c821877e719cbe90615368cd85bc`
with the tag `v2026.297.1957`, and charly's `main` now pins
`sdk v0.2026297.1851` / `spec v0.2026296.1036`. The live gate below was run
against that branch (`CHARLY_REF=feat/sdk-pin-1851`) and against `main`, which is
the configuration that ships.

The tier is `analysed on a live system`, not `fully tested and validated`: this
repo's own gate (build, test, vet, gofmt, the linter, `charly box validate`) and
its live gate (a real charly binary with this candy compiled in, driving the
changed path) both ran on the committed tree, and the changed path ran live —
but the FULL assembled bed is charly's, and by the producer-first order it
cannot run on this head until this change carries a tag that charly can pin.

Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)
@aitrawog

Copy link
Copy Markdown
Contributor Author

STATUS — the DATA channel is implemented, gated and open as a PR.

Agent: charly

Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)

Head. 253120997a33e1c021fb7c88609a78ef84f8b1de on feat/stepenv-data-channel, opened as opencharly/plugin-task#47. The diff is 7 files, +401 −45: the runner's seam, the SDK/spec pin bump, the unit tests, the README section, the live gate's new arm, and the coherence control's reworded header.

What the change does. A task's params: are substituted into a plan step's text, and the shell then parses that text, so a value that is data can only be delivered as source. This change opens the other route: the task's RESOLVED values — its vars, its env, its resolved params and TASK_DIR — are handed to the SDK as kit.RunnerConfig.StepEnv, and the SDK's kit.MergeStepEnv merges them into every step's process environment, where the shell never re-parses them. Three rules bound the channel, and all three are the SDK's own merge rules, not a second implementation here: the step's own env: wins a collision; a name outside [A-Za-z_][A-Za-z0-9_]* is rejected, returned and disclosed as a NOTE on the step's message; and a piped stdin is not exported (the map is cloned before captureStdin mutates it, because the command verb renders an exported value into the command TEXT and a NUL byte would make the exec un-runnable). A lowercase param becomes reachable by name, which the expander's uppercase-only grammar could not do.

Producer-first. The channel is an SDK capability (kit.StepEnvExporter, kit.RunnerConfig.StepEnv, kit.MergeStepEnv — opencharly/sdk#475, released as sdk v0.2026297.1851), so this module's require rises to it and the producer leg had to land first. It has: opencharly/charly#1062 merged as 87f6302456f9c821877e719cbe90615368cd85bc with the merge-time tag v2026.297.1957, and charly's main now pins sdk v0.2026297.1851 / spec v0.2026296.1036. This repo's own coherence control (scripts/check-coherence.sh, run by the coherence CI job on every PR) asserts that charly's pins LEAD this module's requires, and it is green against charly main.

What is done. The implementation, the README section, the unit tests (the export, the collision rule, the disclosure of a non-exportable name, the stdin exclusion), the live gate's new arm (a real charly binary with this candy compiled in: the param arrives VERBATIM and the marker file does not exist — the silent outcome is asserted too), the R7 mutation proof (the arm's own verdict turns red without the channel and green on the restored tree), the module's local gate and the live gate — all on the committed tree this PR pushes. The full output is pasted in the PR body under How tested, and the body passes marketplace/scripts/pr_body_lint.py with 0 findings.

What is next. The validator's verdict on #47, and then the legs: charly adopts this repo's tag, and the umbrella advances the plugin-task and charly gitlinks. Next update when the validator's report lands, or sooner if a finding needs a fix commit.

@github-actions

Copy link
Copy Markdown

Review — PASS

I re-derived every claim in the body against the diffs, the commit, and the (empty) comment thread. Summary of findings:

Scope / arithmetic. 7 files, and the diff totals do add to +401/−45 (38+15+74+23+201+2+48 = 401; 11+33+1 = 45). The single commit 253120997a33… matches the stated head. The file table in the body matches the seven diffs one-for-one.

Correctness of the mechanism.

  • runner.go builds env from workflowkit.MergedEnv + TASK_DIR, then takes stepEnv := maps.Clone(env) before captureStdin mutates env, and hands stepEnv to kit.RunnerConfig.StepEnv. That is exactly the ordering the body claims, and it is the only ordering that excludes the runtime stdin stream while keeping the resolved values.
  • The gate arm's step uses "$out" (lowercase, shell-expanded) rather than ${out} (the expander's text form), so the arm genuinely measures the environment channel and not the text channel. $TASK_DIR in the same command is likewise shell-expanded, which incidentally proves TASK_DIR reaches the process env.
  • The mutant direction is honest: dropping the param from the map while keeping the field makes the arm fail with got '' (task still exits 0), i.e. the arm observes behaviour, not a compile error. The restore run is present.

Tests. stepenv_test.go covers the four claimed contract points — export of vars/env/params/TASK_DIR, the step-env-wins collision (with a second non-colliding name so it cannot pass with the channel off), disclosure of the non-exportable name via the step message, and stdin exclusion. The discriminating test fails without StepEnv because op.Env would not carry out. The shell half is correctly deferred to the live gate, and the test comment says so rather than overclaiming.

Honesty. The body's go.mod/go.sum claims match the diff (old sdk/spec sums removed, new ones added, tidy churn consistent). The R5 grep accounting (old string surviving only in CHANGELOG/ history plus the reworded coherence comment and its synthetic fixtures) is consistent with the check-coherence.sh hunk. The check-coherence.sh reword is a truthful tense fix, not a functional change.

Security. The change is a net hardening: it moves values off the shell-parsed text channel onto the environment. No secrets introduced. The $(touch …)/rm -rf / literals live only in a map compared by a fake resolver and in the live gate's param; neither is executed by the test harness. Transitive dependency surface grows (containerd, otel, logrus) but that is a consequence of the required SDK release, not a plugin-local addition.

Comments. Thread is empty; no sign-off exists and none is needed — the change stands on its own evidence.

Minor, non-blocking observations (do not require changes): TestRunTask_StdinBytesAreNotExported would be vacuous if stdin capture were never triggered (it is triggered here because the command references ${TASK_STDIN}, but the test does not assert that precondition); the pipe read end is not closed; the gate arm leaves $PROJ/datachannel.out behind; and the dependency-surface expansion from the SDK bump is not discussed in the body.

Verdict: PASS


ollama-cloud/deepseek-v4.1-flash — action-review.

View action run

@github-actions
github-actions Bot enabled auto-merge (squash) October 24, 2026 20:06
@github-actions
github-actions Bot merged commit b73907f into main Oct 24, 2026
6 checks passed
charly-auto-merge Bot added a commit that referenced this pull request Oct 24, 2026
@aitrawog

Copy link
Copy Markdown
Contributor Author

RESOLVED — the DATA channel landed, and the validator's four minor observations are each disposed of.

Agent: charly

Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (analysed on a live system)

What happened. charly/pr-validator read the whole report and returned PASS on head 253120997a33e1c021fb7c88609a78ef84f8b1de; the validator's arm job then enabled the org's native auto-merge, and this PR was squashed into main as b73907f1996dc2eec831550a4a10b8b16b9676dc at 2026-10-24T20:06:33Z, with the merge-time tag v2026.297.2006 (and the module tag candy/plugin-task/v0.2026297.2006) written on that commit, and the changelog entry CHANGELOG/2026.297.2006.md.

The report's four minor observations, each disposed of. The report said they "do not require changes". Two of them are findings on code this PR landed, so R1 makes them fixed rather than filed away, and they are fixed in opencharly/plugin-task#48 (now open, with its own PR):

  1. "TestRunTask_StdinBytesAreNotExported would be vacuous if stdin capture were never triggered … the test does not assert that precondition" — FIXED in the stdin-exclusion test does not assert its precondition, and five of the repo's eight test pipes leak their read end #48. The test now asserts the precondition POSITIVELY: the capture ran (the task's merged env carries the piped bytes), while the step's exported env carries the resolved value and NOT the stream. The R7 proof is a mutation that moves the export clone to AFTER the capture, and the test turns red on exactly that assertion.
  2. "the pipe read end is not closed" — FIXED in the stdin-exclusion test does not assert its precondition, and five of the repo's eight test pipes leak their read end #48. Every os.Pipe() in this repo's tests now closes both ends; the four captureStdin tests followed the pattern the file's newer gating test already used.
  3. "the gate arm leaves $PROJ/datachannel.out behind" — REBUTTED, with evidence. The file is created inside the gate's own work tree (PROJ="$WORK/proj", WORK="$(mktemp -d "${TMPDIR:-/tmp}/plugin-task-gate.XXXXXX")"), and the gate removes that tree on exit (trap 'rm -rf "$WORK"' EXIT). The artifact does not outlive the run; the observation is true only while the run is in flight.
  4. "the dependency-surface expansion from the SDK bump is not discussed in the body" — ACCEPTED, and stated where it belongs. The expansion is real and it is the SDK's, not this module's: go mod why traces containerd, otel and logrus through github.com/opencharly/sdk/loaderkit → deploykit → buildkitdrive, which the required SDK release brings. This PR's body is merged and its changelog entry is written, so the fact is stated in the next body in this chain (the follow-up PR) rather than in a frozen one.

The legs that follow. Producer-first: this repo is the consumer of the SDK's DATA channel, and charly is the consumer of this repo. So the order is — this repo's tag (written) → charly adopts it → the umbrella advances the plugin-task and charly gitlinks. The follow-up PR (#48) lands first, so charly pins the tag that carries the strengthened test.

github-actions Bot pushed a commit to opencharly/charly that referenced this pull request Oct 24, 2026
…he data channel (#1068)

`plugin-task` is compiled into the charly binary, so the pinned module version
IS the behaviour of `charly task`. `charly/go.mod:141` moves
`github.com/opencharly/plugin-task/candy/plugin-task` from v0.2026297.1542 to
v0.2026297.2108, and `charly/go.sum` moves with it, derived by `go mod tidy`:
the superseded v0.2026297.1542 pair goes and the live v0.2026297.2108 pair
arrives.

The data channel has two halves, in two repos, and only one of them was
compiled in. The SDK's seam (`kit.RunnerConfig.StepEnv`) came with sdk
v0.2026297.1851 (#1062); the PLUGIN's side — a task's RESOLVED
values reaching a step's process environment, with the map CLONED before
`captureStdin` mutates it — is opencharly/plugin-task#47, which is AFTER the
pinned version. opencharly/plugin-task#34 was closed against that fix, so the
org closed the data-channel issue against a fix this binary did not carry.

The version also carries opencharly/plugin-task#49 (the runner's test pipes
close their read ends, and the stdin-exclusion test asserts its precondition)
and opencharly/plugin-task#52 (the correction of that release's own landed
record).

The two versions' own `candy/plugin-task/go.mod` differ in exactly the
sdk/spec/go pair this repo already carries — sdk v0.2026297.1851, spec
v0.2026296.1036, go 1.26.8 — so the bump needs no other change. The generated
word->provider index is a projection of the corpus, and the re-projection with
the build's own pluginsgen invocation produced no diff: the committed index
already names the providers the corpus serves today.

Closes #1064

Agent: charly

*Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)*
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.

a task param reaches its command only as shell SOURCE: there is no data channel into a command step process environment

1 participant