Skip to content

fix(command): apply the step env: modifier to the executed command - #4

Merged
github-actions[bot] merged 1 commit into
mainfrom
feat/command-verb-step-env
Sep 1, 2026
Merged

github-actions[bot] merged 1 commit into
mainfrom
feat/command-verb-step-env

Conversation

@atrawog

@atrawog atrawog commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

The step-level env: modifier (a base #Op field, spec/schema/_common.cue) was schema-declared but never wired: the command verb ran the command with the container's default env, so an env-scoped probe (e.g. OMARCHY_PATH=/tmp/missing-omarchy) silently tested the wrong configuration. RCA: no executor path read op.Env — the command plugin's RunVerb called cc.Exec().RunCapture(ctx, script) with no env, and the runner's EffectiveEnv (variable expansion) excludes op.Env.

Fix: prepend the step env as shell exports (sorted keys, spec/shellquote quoting) to the command for the in-container, host-side, and background legs.

How tested

$ cd candy/plugin-command && go build ./... && go test ./...
ok  	github.com/opencharly/plugin-command/candy/plugin-command	0.005s
?   	github.com/opencharly/plugin-command/candy/plugin-command/cmd/serve	[no test files]
?   	github.com/opencharly/plugin-command/candy/plugin-command/params	[no test files]
$ gofmt -l .   # empty
$ go vet ./... # clean

New tests: TestCommandVerb_StepEnvReachesCommand (the export KEY='value' reaches the executed script) + TestCommandVerb_NoEnvLeavesCommandUntouched (no spurious exports when env: is absent). Both pass; the full existing suite passes.

Rulebook compliance

  • R1: RCA'd the env gap against the actual code (schema declares env? on #Op; no executor path reads it) before fixing.
  • R3: the fix reuses spec/shellquote.ShellQuote (the one quoter) rather than a new quoter.
  • R10: Go tests added that fail without the fix; build + vet + test green on the final tree.

Change classification

  • Change class: bug fix (schema-declared field never wired to execution).
  • Verification gate: R10 (Go tests + build + vet).
  • Attribution tier: fully tested and validated (build + tests + vet green on the final tree).

Assisted-by: pi openrouter/deepseek/deepseek-v4-flash-0731 (fully tested and validated)

The step-level env: modifier (a base #Op field, spec/schema/_common.cue) was
schema-declared but never wired: the command verb ran the command with the
container's default env, so an env-scoped probe (e.g. OMARCHY_PATH=/tmp/...)
silently tested the wrong configuration. RCA: no executor path read op.Env —
the command plugin's RunVerb called cc.Exec().RunCapture(ctx, script) with no
env, and the runner's EffectiveEnv (variable expansion) excludes op.Env.

Fix: prepend the step env as shell exports (sorted keys, spec/shellquote
quoting) to the command for the in-container, host-side, and background legs.
Tests: TestCommandVerb_StepEnvReachesCommand (the export reaches the script)
+ TestCommandVerb_NoEnvLeavesCommandUntouched (no spurious exports).
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review — PASS

Head SHA: 1bdeb396c744df5d0feaaa3c8c3d31e8cbc1ede1 (branch: main)

Security screen: PASS — no scope mismatch (diff matches the described env-modifier fix exactly), no secrets/exfiltration, no new dependencies (imports are stdlib + existing spec/shellquote), no weakened guardrails, no supply-chain changes.

Checklist: All applicable rules PASS or NA.

  • A1/A2/A3: Body accounts for both changed files, pastes real go build/go test output, names the R10 gate, and the new tests execute the changed code path — fully tested and validated is justified. Attribution footer present in the exact required form.
  • R1/R2/R3/R4/R5: RCA provided for the env gap; no pre-existing/out-of-scope split; reuses the single spec/shellquote.ShellQuote quoter (no duplication); no ad-hoc workarounds; no transitional/legacy paths.
  • R10: New tests TestCommandVerb_StepEnvReachesCommand and TestCommandVerb_NoEnvLeavesCommandUntouched prove the new behavior and would fail without the fix; pasted go test output is internally consistent (recordingExec records the in-container script, asserting the export KEY='value' prepend).
  • B16 placement: CORRECT — the env application is a command-verb capability living in the candy layer; no alias files, no new core→sdk kit imports, no Legacy identifiers.
  • B13/B14/B15/B17/B18: No schema/cue/proto changes (SDD N/A); no concurrency failures or forbidden framing; single atomic commit; no disposable-target destroy; diff is gofmt-clean with no lint-visible issues.

Verdict: PASS


ollama/deepseek-v4-flash — pi-review-action.

View action run

@github-actions
github-actions Bot enabled auto-merge (squash) September 1, 2026 06:16
@github-actions
github-actions Bot merged commit 61df41c into main Sep 1, 2026
2 checks passed
charly-auto-merge Bot added a commit that referenced this pull request Sep 1, 2026
@aitrawog
aitrawog deleted the feat/command-verb-step-env branch September 25, 2026 19:22
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