Skip to content

shell.exec bypasses pkg/process: no dry-run, no narration, and the preferred shell is never read #800

Open

Description

@David-Noble-at-work

Part of #799.

pkg/process states its own invariant: "the single bridge between os/exec and the runtime
environment's status and result channels. Every provider that shells out goes through Runner so
line-splitting, dry-run, cancellation, and error wrapping live in exactly one place."

pkg/op/provider/shell/provider.go imports os/exec directly and never imports pkg/process:

cmd := exec.CommandContext(p.RuntimeEnvironment().Context, "sh", "-c", command)

pkg/op/provider/powershell/provider.go does the same. This is the violation
6-execution-topology.md gives as the reason internal/pwsh was deleted — "a second
process-execution path beside pkg/process"
— now in the two providers whose entire job is
shelling out.

Three consequences

Dry-run. Runner implements it — dryRun bool // when true, narrate the command but skip execution, returning nil bytes with a nil error. shell.exec has zero occurrences of dry-run and
calls cmd.Run() unconditionally. Whether a dry-run graph reaches the provider is an executor
question not yet traced, so this is suspected, not proven — but nothing in the provider prevents
it, and a dry run that executes is the worst failure in the list.

Narration. Runner streams stderr through ctx.Status.Warn line by line. shell.exec
captures both streams into bytes.Buffer and emits them only on completion, so a long command is
silent until it finishes.

The preferred shell. schema/devlore-config.json declares lore.preferences.shell —
"Preferred shell", a free string with no enum. Production code never reads it; the only references
are config_test.go fixtures exercising generic nested-key get, set and delete. The key is
settable, persists, and changes nothing. sh -c is hardcoded regardless.

Why these are one issue

The fix is a single change. Routing through Runner is what restores dry-run, narration and error
wrapping, and Runner takes a prepared *exec.Cmd — so the shell to invoke is chosen exactly
where the Cmd is built. Fixing the preference alone would entrench the bypass; fixing the bypass
without the preference would touch the same lines twice.

Two things not asserted: that the design intends lore.preferences.shell to govern shell.exec
specifically (nothing in the repository says so), and the dry-run execution path end to end.

This title expires with the rename

The title names shell.exec, which is accurate about the action as it stands today and becomes wrong
the moment #799's rename lands: this action becomes shell.run_command, and shell.exec becomes the
argv-replacing action of #802.

Retitle then, to shell.run_command bypasses pkg/process: no dry-run, no narration, and the preferred shell is never read. Renaming it earlier would describe an action that does not exist yet;
leaving it afterwards points a reader at the wrong one.

Plan and design documents

Kind Document
Plan not yet — the plan arrives when the branch does
Design draft in hand, uncommitted; lands with the first worktree

Epic: #815 · Feature: #799

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area:ArchitectureSystem architecture and designEpic:ProvidersThe provider surface: one feature per providerbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions