Skip to content

Remove deprecated eval_env references#292

Merged
kindermax merged 2 commits intomasterfrom
codex/remove-deprecated-evalenv
Mar 12, 2026
Merged

Remove deprecated eval_env references#292
kindermax merged 2 commits intomasterfrom
codex/remove-deprecated-evalenv

Conversation

@kindermax
Copy link
Copy Markdown
Collaborator

@kindermax kindermax commented Mar 12, 2026

Summary

  • document that eval_env is gone and show how to use env with sh
  • drop the deprecated parsing/help text and remove the legacy tests/doc references now that commands/configs no longer accept it

Testing

  • Not run (not requested)

Summary by Sourcery

Remove support for the deprecated eval_env directive and update configuration handling, documentation, and examples to rely solely on env with sh execution mode for dynamic environment variables.

Enhancements:

  • Disallow eval_env as a recognized config keyword and simplify command/config unmarshalling by removing legacy eval_env handling and logging.
  • Clarify error messaging when shell script execution for env values fails to reference env.sh rather than eval_env.
  • Update example configuration (Python example) to demonstrate dynamic environment variables using env with sh instead of eval_env.
  • Adjust agent onboarding documentation to list only the currently supported top-level config fields.

Documentation:

  • Remove eval_env from the config reference and advanced usage docs, replacing it with guidance and examples for using env with sh for dynamic environment values.
  • Add a changelog entry documenting the removal of the deprecated eval_env directive and the recommended replacement.

Tests:

  • Update global eval_env tests to assert that using eval_env now fails with an appropriate error, and remove legacy per-command eval_env test cases.

Chores:

  • Delete obsolete tests and config examples tied to the removed eval_env support.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai bot commented Mar 12, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Removes all remaining support and documentation for the deprecated eval_env directive, updating configuration parsing, error messages, docs, examples, and tests to require env with sh instead.

Sequence diagram for dynamic env evaluation using env.sh

sequenceDiagram
    actor User
    participant LetsCLI
    participant ConfigLoader
    participant EnvExecutor
    participant Shell

    User->>LetsCLI: run command
    LetsCLI->>ConfigLoader: parse lets.yaml
    ConfigLoader-->>LetsCLI: Config with Env entries (some with Sh)

    LetsCLI->>EnvExecutor: resolve Env for command
    EnvExecutor->>EnvExecutor: detect Env entries with Sh
    EnvExecutor->>Shell: executeScript(shell, script)
    Shell-->>EnvExecutor: stdout
    EnvExecutor-->>LetsCLI: resolved environment map
    LetsCLI->>LetsCLI: run command with resolved env

    EnvExecutor-->>LetsCLI: error fmt("can not get output from env.sh script: %s", err)
Loading

Updated class diagram for config and command models without eval_env

classDiagram
    class ConfigBefore {
        +map~string, CommandBefore~ Commands
        +string Version
        +string Shell
        +Envs Env
        +Envs EvalEnv
        +string Before
        +string Init
        +UnmarshalYAML(unmarshal func_interface_error_) error
    }

    class ConfigAfter {
        +map~string, CommandAfter~ Commands
        +string Version
        +string Shell
        +Envs Env
        +string Before
        +string Init
        +UnmarshalYAML(unmarshal func_interface_error_) error
    }

    class CommandBefore {
        +string Name
        +string Description
        +string Shell
        +Envs Env
        +Envs EvalEnv
        +string Options
        +Deps Depends
        +string WorkDir
        +string After
        +Checksum Checksum
        +bool PersistChecksum
        +string Ref
        +Args Args
        +UnmarshalYAML(unmarshal func_interface_error_) error
    }

    class CommandAfter {
        +string Name
        +string Description
        +string Shell
        +Envs Env
        +string Options
        +Deps Depends
        +string WorkDir
        +string After
        +Checksum Checksum
        +bool PersistChecksum
        +string Ref
        +Args Args
        +UnmarshalYAML(unmarshal func_interface_error_) error
    }

    class Envs {
        +bool Empty() bool
        +Range(fn func_string_Env_error_) error
        +Set(name string, value Env) void
    }

    class Env {
        +string Name
        +string Value
        +string Sh
    }

    ConfigBefore "1" --> "*" CommandBefore : commands
    ConfigAfter "1" --> "*" CommandAfter : commands
    CommandBefore "1" --> "1" Envs : env
    CommandAfter "1" --> "1" Envs : env
    ConfigBefore "1" --> "1" Envs : eval_env
    CommandBefore "1" --> "1" Envs : eval_env

    %% Highlighting the removal conceptually
    classDef removed fill:#ffe6e6,stroke:#cc0000,stroke-width:1px;
    class ConfigBefore removed
    class CommandBefore removed
Loading

File-Level Changes

Change Details Files
Remove deprecated eval_env support from config/command YAML parsing.
  • Drop EvalEnv field from Command and Config unmarshalling structs.
  • Remove logic that merged deprecated eval_env values into Env with shell execution and the associated deprecation log message.
  • Stop treating eval_env as a recognized top-level keyword in configs.
internal/config/config/command.go
internal/config/config/config.go
Update documentation and examples to describe dynamic env via env.sh instead of eval_env.
  • Remove eval_env sections from the config reference and anchors from the TOC.
  • Rewrite advanced_usage to show dynamic environment variables using env entries with sh.
  • Update the Python example lets.yaml to replace eval_env with env entries using sh.
  • Document eval_env removal in the changelog and adjust AGENTS.md top-level field list.
docs/docs/config.md
docs/docs/advanced_usage.md
examples/python/lets.yaml
docs/docs/changelog.md
AGENTS.md
Adjust tests and error messages to reflect eval_env removal.
  • Change global_eval_env test to assert failure and specific unsupported keyword message.
  • Delete command_eval_env tests and associated test config directory.
  • Update executeScript error message to reference env.sh instead of eval_env script.
tests/global_eval_env.bats
tests/command_eval_env.bats
tests/command_eval_env/lets.yaml
internal/config/config/env.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

sourcery-ai[bot]

This comment was marked as outdated.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 12, 2026

Greptile Summary

This PR completes the removal of the long-deprecated eval_env directive from lets, replacing documentation, examples, and tests with the modern env + sh execution mode equivalent.

Key changes:

  • eval_env is removed from the top-level keywords set in config.go, so any config file using global eval_env now produces a clear parse error: "keyword 'eval_env' not supported".
  • The EvalEnv field and its migration shim are removed from Command.UnmarshalYAML in command.go, along with the now-unused logrus import.
  • tests/global_eval_env.bats is updated to assert the new hard-error behaviour.
  • tests/command_eval_env.bats and its fixture are deleted rather than updated to cover the new behaviour.
  • Documentation (config.md, advanced_usage.md, changelog.md, AGENTS.md) and the Python example are updated to reflect the removal.
  • The error message in env.go is updated from eval_env script to env.sh script.

Notable inconsistency: Global eval_env now fails loudly, but command-level eval_env is silently ignored by gopkg.in/yaml.v3 (unknown fields are dropped by default). Users who had eval_env inside a command block will lose that env variable with no error or warning, which could surface as a subtle runtime regression. The deleted test suite for command-level eval_env leaves this path uncovered.

Confidence Score: 3/5

  • Mostly safe to merge, but the silent-drop of command-level eval_env is an undocumented breaking behaviour that could cause subtle regressions for existing users.
  • Global eval_env removal is clean and well-tested. The main risk is command-level eval_env: yaml.v3 silently ignores unknown struct fields, so any command that relied on eval_env will silently lose its computed env var with no error, no warning, and no test coverage for this path. This is a quiet breaking change that could be confusing in production configs.
  • internal/config/config/command.go — command-level eval_env is silently ignored; tests/command_eval_env.bats — deleted without a replacement that covers the new behaviour.

Important Files Changed

Filename Overview
internal/config/config/config.go Removes eval_env from the top-level keywords set and the anonymous unmarshal struct, so any config using global eval_env now fails with a clear "keyword not supported" error. Clean removal.
internal/config/config/command.go Removes EvalEnv field and its migration shim from the command unmarshal struct and drops the now-unused logrus import. Unlike global eval_env, command-level eval_env will now be silently ignored by the YAML parser rather than producing an error, which is an inconsistency.
tests/global_eval_env.bats Test updated to assert failure with "keyword 'eval_env' not supported" — correctly covers the new hard-error behaviour for global eval_env.
tests/command_eval_env.bats Deleted entirely rather than updated to cover the new (silently-ignored) behaviour for command-level eval_env, leaving a coverage gap for an inconsistency between global and command-level handling.
tests/command_eval_env/lets.yaml Fixture deleted alongside the test; demonstrates the silent-ignore path for command-level eval_env that is now untested.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Parse lets.yaml] --> B{Top-level keyword check}
    B -- "eval_env found" --> C["Error: keyword 'eval_env' not supported"]
    B -- "known keyword" --> D[Unmarshal Config struct]
    D --> E[Unmarshal Commands]
    E --> F{Command has eval_env?}
    F -- "yes (yaml.v3 silent ignore)" --> G["eval_env silently dropped\nNo error raised"]
    F -- "no" --> H[Unmarshal env with sh / value / checksum]
    H --> I[Execute sh scripts & checksums]
    I --> J[Run command with resolved env]
    G --> J
Loading

Comments Outside Diff (1)

  1. tests/command_eval_env.bats

    Silent failure for command-level eval_env

    The global-level eval_env now produces a clear parse error ("keyword 'eval_env' not supported") because eval_env was removed from the top-level keywords set. However, eval_env inside a command block is handled differently: the command struct uses an anonymous inner struct for unmarshaling, and gopkg.in/yaml.v3 silently ignores unknown fields by default. This means users who had command-level eval_env will get no error — the variable simply won't be set — which is a confusing silent regression.

    The deleted fixture (tests/command_eval_env/lets.yaml) actually demonstrates the issue: COMPUTED: echo "Computed env" under eval_env inside a command would now be silently dropped. There's no assertion test to document what behaviour users should now expect (an error, or a migration hint).

    Consider either:

    1. Adding a strict-mode parse check for unknown fields in Command.UnmarshalYAML so command-level eval_env also surfaces a clear error, or
    2. Adding a new bats test (replacing the deleted one) that asserts the current silent-ignore behaviour is intentional, with an appropriate warning printed.

Last reviewed commit: c746358

@kindermax kindermax merged commit 382563d into master Mar 12, 2026
5 checks passed
@kindermax kindermax deleted the codex/remove-deprecated-evalenv branch March 12, 2026 16:30
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