Skip to content

feat: add include_env input and environments output#2

Open
eliran-ops wants to merge 2 commits into
ship-mainfrom
feat/ship-main
Open

feat: add include_env input and environments output#2
eliran-ops wants to merge 2 commits into
ship-mainfrom
feat/ship-main

Conversation

@eliran-ops
Copy link
Copy Markdown
Collaborator

@eliran-ops eliran-ops commented May 20, 2026

Summary

Adds the include_env + git_token inputs and the environments output to the action, so callers can read the local environments: block from skyhook.yaml in the same step that resolves service config.

  • include_env=true + block present → emit (YAML, heredoc-safe; [] passes through)
  • include_env=true + block missing (key absent, any YAML null spelling, bare key, or whole file missing) → exit 1 Not supported yet
  • include_env=false or unset → emit empty (backwards compatible)
  • git_token declared but unused — reserved placeholder for the future external-repository fetch path

Null detection uses yq … | tag == "!!null" so all four null spellings (null/Null/NULL/~) and bare environments: are caught uniformly.

Reviewer guide

Order to read:

  1. action.yml — new inputs, new output, env wiring.
  2. scripts/parse.sh — new emit_environments() helper + two new exit branches (config-missing-with-include_env, env-block-missing). Existing service-lookup paths unchanged.
  3. tests/parse_test.sh — 24 new assertions: happy path, every null spelling, [] pass-through, regression guard for include_env=false, log-line guards, orthogonality between env emission and service lookup.
  4. tests/fixtures-no-env/ — small fixture for the new CI smoke "Not supported yet" contract step.
  5. .github/workflows/test.yml — two new smoke steps (happy + continue-on-error failure contract).
  6. README.md — new inputs/outputs in the tables, env-passthrough usage example, expanded behavior matrix.

What's risky:

  • The CI smoke "failure contract" step uses continue-on-error: true. If the action ever silently succeeded when it should have failed, the immediately-following outcome != "failure" assertion catches it.
  • Heredoc-output for the environments value relies on the existing write_output helper (16-hex random delimiter, pre-existing pattern).

What's mechanical:

  • README and action.yml description text — they were kept in sync after each iteration of review (any drift was flagged and corrected).
  • 4 cycles of /review --deep + 1 cycle of /pre-pr (DEEP DEEP) ran on this branch before opening. Every actionable item was addressed and re-verified.

Test plan

  • bash tests/metadata_test.sh — passes locally
  • bash tests/parse_test.sh — 81 PASS, 0 FAIL
  • shellcheck -S warning scripts/parse.sh tests/parse_test.sh tests/metadata_test.sh — clean
  • yq e '.' action.yml and both fixture skyhook.yaml files — valid YAML
  • CI smoke job (ubuntu-latest) — runs the composite action end-to-end against fixtures, including the two new include_env steps
  • CI shell-tests matrix (ubuntu-latest + macos-latest) — runs the bash unit suite on both OSes

Depends on: #1 (feat: add required default_build_context / default_dockerfile_path inputs) — this PR targets ship-main so the diff is exactly the include_env work; rebase to main after #1 lands.

🤖 Generated with Claude Code


Note

Medium Risk
Adds a new output and new failure modes when include_env=true (including failing on missing config/environments), which can break workflows that opt into the flag. Core service parsing remains largely unchanged, lowering risk for existing users who keep defaults.

Overview
Adds optional include_env/git_token inputs and a new environments output so the action can return the top-level environments: block from the local skyhook.yaml (heredoc-safe, including pass-through of environments: []).

Updates scripts/parse.sh to emit environments when enabled and to fail fast with Not supported yet when include_env=true but the config file is missing or environments is absent/null (detected via yq tag to cover all YAML null spellings).

Expands CI and docs: new smoke steps and fixtures for the happy-path and expected-failure contract, plus extensive unit tests covering the new flag behavior and regressions when include_env=false.

Reviewed by Cursor Bugbot for commit 0110e19. Bugbot is set up for automated code reviews on this repo. Configure here.

eliran-mic and others added 2 commits May 20, 2026 11:48
Adds two new action inputs and one output, gated by `include_env`:

- `include_env` (default `false`): when `true`, emit the local
  skyhook.yaml `environments:` block on the new `environments` output.
- `git_token` (default `''`): reserved for a future external-repository
  fetch path; currently unused.
- `environments` output: YAML block, multi-line, passed back via the
  $GITHUB_OUTPUT heredoc form so quotes / $ / backticks round-trip
  intact when consumed via env-passthrough.

Behavior (per the spec "return it if in the yaml, else 'Not supported
yet'"):

- `include_env=true` + block present -> emit (incl. `[]` pass-through).
- `include_env=true` + block missing (key absent, `null` / `Null` /
  `NULL` / `~` / bare `environments:`) -> exit 1 "Not supported yet".
- `include_env=true` + config file missing -> exit 1 "Not supported yet".
- `include_env=false` or unset -> emit empty (backwards compatible).

Null detection uses yq's `tag` (`!!null`) so every YAML null spelling is
caught - the original textual `"null"` check only matched lowercase.

Tests: +24 cases in parse_test.sh covering happy path, every null
spelling, `[]` pass-through, missing-config, missing-service-but-env-
present orthogonality, log-line assertions, and an `INCLUDE_ENV` unset
regression guard. CI smoke job gains an `include_env=true` happy-path
contract and a `continue-on-error` "Not supported yet" failure contract
using a new `tests/fixtures-no-env/` fixture.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
macOS runners queue for hours on GitHub's hosted pool while ubuntu-latest
finishes in 6s. The shell unit suite is OS-independent (bash + yq behave
identically across runners), so the macOS leg adds wall-clock latency
without proportional coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

2 participants