Skip to content

feat(#1313): accept positional args for validate input#3382

Open
fullsend-ai-coder[bot] wants to merge 4 commits into
mainfrom
agent/1313-positional-args-input
Open

feat(#1313): accept positional args for validate input#3382
fullsend-ai-coder[bot] wants to merge 4 commits into
mainfrom
agent/1313-positional-args-input

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown

Change ec validate input to accept input file paths as positional arguments instead of requiring the --file flag. This enables natural shell glob usage like ec validate input *.yaml.

The --file flag is deprecated but still accepted for backward compatibility. If both positional arguments and --file are provided, the values are combined. A deprecation warning is logged to stderr when --file is used, avoiding corruption of structured output formats like JSON.

Changes:

  • cmd/validate/input.go: add cobra.ArbitraryArgs, merge
    positional args into filePaths, replace MarkFlagRequired
    with MarkHidden, add manual validation for at least one
    input file, update usage/examples
  • cmd/validate/input_test.go: add tests for positional args,
    multiple positional args, mixed positional and flag usage,
    update no-file-provided test for new error message

Closes #1313

Post-script verification

  • Branch is not main/master (agent/1313-positional-args-input)
  • Secret scan passed (gitleaks — d134e495b18cf8493cfea8791e310b58f0320797..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Change ec validate input to accept input file paths as positional
arguments instead of requiring the --file flag. This enables
natural shell glob usage like ec validate input *.yaml.

The --file flag is deprecated but still accepted for backward
compatibility. If both positional arguments and --file are
provided, the values are combined. A deprecation warning is
logged to stderr when --file is used, avoiding corruption of
structured output formats like JSON.

Changes:
- cmd/validate/input.go: add cobra.ArbitraryArgs, merge
  positional args into filePaths, replace MarkFlagRequired
  with MarkHidden, add manual validation for at least one
  input file, update usage/examples
- cmd/validate/input_test.go: add tests for positional args,
  multiple positional args, mixed positional and flag usage,
  update no-file-provided test for new error message

Closes #1313
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 6, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:02 AM UTC · Completed 9:13 AM UTC
Commit: 47d3320 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review — comment

Summary: This PR changes ec validate input to accept input file paths as positional arguments, deprecating the --file flag while preserving backward compatibility. The overall approach is sound — positional args are the ergonomic choice for file inputs and enable natural shell glob expansion. Tests, docs, and acceptance tests are updated. However, there is one gap between the claimed and actual behavior regarding deprecation warnings.

Findings

1. MarkHidden used instead of MarkDeprecated — no deprecation warning emitted (medium — correctness)

File: cmd/validate/input.go, line ~311 (in diff)

The PR description states: "A deprecation warning is logged to stderr when --file is used, avoiding corruption of structured output formats like JSON." However, the code uses cmd.Flags().MarkHidden("file"), which only hides the flag from --help output — it does not emit any runtime deprecation warning.

Cobra provides cmd.Flags().MarkDeprecated("file", "use positional arguments instead") specifically for this purpose. MarkDeprecated both hides the flag from help AND prints a deprecation notice to stderr when the flag is used, which is exactly the behavior the PR description claims.

With MarkHidden, users of --file get no signal to migrate. The flag silently works but is invisible in help, creating a confusing experience for users who have existing scripts.

Remediation: Replace cmd.Flags().MarkHidden("file") with cmd.Flags().MarkDeprecated("file", "use positional arguments instead"). Note that MarkDeprecated already calls MarkHidden internally, so no other changes are needed.

2. Custom Args validator rejects filenames starting with - (low — correctness)

File: cmd/validate/input.go, lines ~93–100 (in diff)

The custom Args function rejects any positional argument starting with -:

for _, arg := range args {
    if strings.HasPrefix(arg, "-") {
        return fmt.Errorf("unknown flag: %s", arg)
    }
}

This has two issues:

  • Filenames starting with - are valid Unix paths. A file named -report.yaml would be incorrectly rejected, even when passed after -- (the POSIX end-of-flags marker). Users would need to use ./-report.yaml as a workaround.
  • The validator is redundant for its intended purpose. Cobra already rejects unknown flags (both --unknown and -x) during its own flag parsing phase, before the Args function is called. The two test cases in Test_ValidateInputCmd_InvalidPositionalArgs actually exercise Cobra's built-in error handling, not the custom validator.

Remediation: Consider removing the custom Args function entirely (Cobra's default accepts arbitrary args) or at minimum documenting that the -- separator combined with dash-prefixed filenames requires the ./ prefix.

3. Acceptance tests no longer exercise --file flag (low — test coverage)

File: features/validate_input.feature

All seven acceptance test scenarios were migrated from --file to positional args. While this validates the new interface, no acceptance-level test remains for the deprecated --file flag. Unit tests still cover --file (existing tests like Test_ValidateInputCmd_SuccessSingleFile were not modified and still use --file), so there is unit-level backward compatibility coverage, but the acceptance layer has a gap.

Remediation: Keep at least one acceptance scenario using the --file flag to ensure end-to-end backward compatibility is tested.

What looks good

  • Backward compatibility is maintained: --file still works, and existing scripts won't break.
  • Deduplication logic correctly handles the case where the same file is specified both as a positional arg and via --file.
  • Test coverage for the new positional args interface is thorough — the table-driven Test_ValidateInputCmd_PositionalArgsVariants covers single, multiple, mixed, dedup, and flag-only cases.
  • Documentation is updated consistently across the command help text, the .adoc reference page, and the feature file.
  • Error handling for missing input files moved from Cobra's MarkFlagRequired to manual validation in PreRunE, which is the correct approach for making the flag optional.

Reviewed by automated review agent — zero-trust evaluation, no prior context assumed.

Previous run

Review — comment

PR: #3382 — feat(#1313): accept positional args for validate input
Scope: cmd/validate/input.go, cmd/validate/input_test.go, docs/.../ec_validate_input.adoc, features/validate_input.feature

Overview

This PR changes ec validate input to accept input file paths as positional arguments (e.g., ec validate input *.yaml), deprecating the --file flag while maintaining full backward compatibility. This directly fulfills issue #1313.

What works well:

  • Positional arguments are correctly merged with --file values, with deduplication
  • Custom Args validator catches flag-like typos (e.g., --polcy) before they're silently treated as file paths — addressing prior human review feedback
  • File presence validation is placed in PreRunE before RunE, ensuring the error surfaces before validation work begins
  • Deprecated flag examples are preserved alongside the new positional-arg examples, providing migration guidance
  • Good table-driven test coverage: single positional, multiple positional, mixed mode, dedup, flag-only, and invalid args
  • Acceptance tests and .adoc documentation are updated consistently

Findings

1. [medium] Missing early return when no input files provided — cmd/validate/input.go

Lines: ~121 (in the patched file, after the if len(data.filePaths) == 0 block)

The file validation check correctly accumulates an error when no files are specified, but does not return early. Execution falls through to validate_utils.GetPolicyConfig() and policy.NewInputPolicy(), which may perform network I/O (fetching remote policy configs, Rekor key resolution). A user who simply forgets to pass input files must wait for that work to complete before seeing the "at least one input file must be specified" error.

Previously, MarkFlagRequired("file") caused Cobra to reject the command before PreRunE ran, giving instant feedback. The new code path regresses that UX unless an early return is added.

Suggested fix:

if len(data.filePaths) == 0 {
    allErrors = errors.Join(allErrors, fmt.Errorf(
        "at least one input file must be specified as a positional argument or via the --file flag"))
    return  // ← add early return to avoid unnecessary policy initialization
}

This was specifically flagged in the prior human review by @st3penta and is not yet fully addressed.

Notes (no action required)

  • Deprecation approach: The PR uses MarkHidden rather than MarkDeprecated for the --file flag, per explicit guidance from the human reviewer to match the codebase's existing deprecation pattern and avoid breaking acceptance test snapshots with stderr warnings. The flag description text includes "DEPRECATED" to inform users who discover it. This is a reasonable design choice.
  • Dash-prefixed filenames: The custom Args validator rejects any positional arg starting with -, including after --. This technically breaks the standard Unix -- -filename convention, but the trade-off for catching typos is reasonable for this command's use case, and the validator was added per reviewer request.
  • Acceptance test coverage: All seven feature scenarios were updated from --file to positional args. Backward compatibility of --file is covered by unit tests. Snapshot outputs should be unaffected since only the invocation syntax changed, not the validation logic or output format.
Previous run (2)

Review

Findings

Medium

  • [error-handling-consistency] cmd/validate/input.go:116 — The empty-files validation check (if len(data.filePaths) == 0) returns directly with fmt.Errorf instead of using the errors.Join(allErrors, ...) accumulation pattern used by the rest of this PreRunE handler and other PreRunE handlers in the validate package (e.g., policy.go). This causes an early return rather than accumulating all validation errors, meaning users won't see all issues at once (e.g., both missing files and invalid policy).
    Remediation: Use allErrors = errors.Join(allErrors, fmt.Errorf("at least one input file...")) and let execution continue to collect other validation errors before returning.

Low

  • [backward-compatibility] cmd/validate/input.go:258 — The --file flag is hidden from help output via MarkHidden. While it remains fully functional, users relying on --help for flag discovery will no longer see it. This is standard deprecation practice and no existing scripts will break.

  • [behavior-change] cmd/validate/input.go:117 — Error message for missing files changes from cobra's auto-generated required flag(s) "file" not set to at least one input file must be specified as a positional argument or via the --file flag. Tools that parse specific error strings from stderr may need updates.

  • [edge-case] cmd/validate/input.go:96 — Using cobra.ArbitraryArgs silently accepts any positional argument. This is an inherent trade-off of accepting arbitrary positional arguments and is the standard cobra approach for commands that take variable-length file lists.

  • [test-inadequate] cmd/validate/input_test.go — No test covers the deduplication path (e.g., same file provided via both --file and as a positional argument verifying it is validated only once). An explicit test would guard against regressions if the merge/dedup logic is later refactored.

Previous run (3)

Review of PR #3382 — feat(#1313): accept positional args for validate input

Verdict: Request Changes — 1 high, 1 medium, and 3 low findings.

The implementation is well-structured and correctly addresses the requirements from issue #1313: positional argument support, --file deprecation with backward compatibility, and value merging when both are provided. The new unit tests cover the primary scenarios adequately. However, the PR has a high-severity gap that will cause CI failures.


High

🔴 Acceptance tests will break due to deprecation warning on stderr

File: features/validate_input.feature (not modified by this PR)

All 7 acceptance test scenarios invoke ec validate input --file .... After this PR, every use of --file emits a deprecation warning to stderr via log.Warn("Flag --file has been deprecated, use positional arguments instead"). The logrus default log level is WarnLevel (see internal/logging/logging.go:65), so this message will appear in stderr output.

The corresponding snapshot expectations in features/__snapshots__/validate_input.snap expect empty stderr for successful scenarios (e.g., lines 29–31, 46–48, 140–142, 172–174). The snapshot comparison in acceptance/cli/cli.go:783–790 matches both stdout and stderr, so the unexpected stderr content will fail all snapshot comparisons.

Remediation: Update the acceptance test scenarios to use positional arguments instead of --file, and regenerate snapshots with UPDATE_SNAPS=true make acceptance. This also validates the new positional-argument functionality in integration tests. Alternatively, update snapshots to include the deprecation warning, but this misses an opportunity to test the new primary code path.


Medium

🟡 Auto-generated documentation not regenerated

File: docs/modules/ROOT/pages/ec_validate_input.adoc

The PR correctly updates the source in cmd/validate/input.go (Use field, examples, flag description), but the auto-generated .adoc file was not regenerated. It still shows:

  • Usage: ec validate input [flags] (should be ec validate input [flags] [file ...])
  • Flag description: path to input YAML/JSON file (required) (should be DEPRECATED: use positional arguments instead...)
  • All examples using --file (should show positional args)

This file is generated via go:generate (see cmd/root.go).

Remediation: Run go generate ./cmd/... and include the regenerated ec_validate_input.adoc in the PR.


Low

Error validation timing changed

File: cmd/validate/input.go

Previously, cobra validated the --file flag requirement before PreRunE executed. Now, PreRunE runs first (initializing policy via policy.NewInputPolicy), and only then does RunE check for missing files. This means unnecessary work (policy initialization) is done before reporting the missing-file error. The test change adding utils.SetTestRekorPublicKey(t) to Test_ValidateInputCmd_NoFileProvided confirms this behavioral shift. Consider moving the file-path check into PreRunE for earlier error reporting.

Duplicate file paths not deduplicated

File: cmd/validate/input.go, line 118

When both --file and positional arguments specify the same file (e.g., ec validate input foo.yaml --file foo.yaml), the file appears twice in data.filePaths and is validated twice by separate workers, producing duplicate output entries. Consider deduplicating after merging.

Existing tests don't exercise the new primary path

File: cmd/validate/input_test.go

The PR adds 3 new test functions for positional args, but existing tests (success, failure, strict mode, warnings, text output) all still use --file. While they test backward compatibility, the new primary code path (positional args) lacks coverage for failure scenarios, strict mode, show-warnings, and various output formats. Consider converting a representative subset to use positional arguments.


Scope & Intent

The change cleanly traces to issue #1313, the scope matches the acceptance criteria, and the feat prefix is correct for this enhancement. No scope creep detected. No security concerns — the change merely adds a second entry point (positional args) into the same file-path processing pipeline the existing --file flag uses. No cross-repo contract concerns — --file remains functional for backward compatibility.


Reviewed dimensions: correctness, security, intent & coherence, style & conventions, documentation currency, cross-repo contracts.


Labels: PR modifies CLI command arg handling and deprecates a flag in cmd/validate/input.go

fullsend-ai-review[bot]

This comment was marked as outdated.

Comment thread cmd/validate/input.go Outdated
Comment thread cmd/validate/input.go Outdated
Comment thread cmd/validate/input.go Outdated
Comment thread cmd/validate/input.go Outdated
Comment thread cmd/validate/input.go
Comment thread cmd/validate/input.go Outdated
Comment thread cmd/validate/input.go Outdated
Comment thread cmd/validate/input.go Outdated
ec validate input /path/to/file.yaml --policy github.com/user/repo

`),
Args: cobra.ArbitraryArgs,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No test covers positional args that are empty strings, non-existent files, or flag-like strings (e.g. --polcy). These are untested boundary conditions between argument parsing and file access.

Comment thread cmd/validate/input_test.go Outdated
Comment thread cmd/validate/input_test.go Outdated
@st3penta

st3penta commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

/fs-fix

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 6, 2026

Copy link
Copy Markdown
Author

🤖 Finished Fix · ✅ Success · Started 2:33 PM UTC · Completed 2:55 PM UTC
Commit: 47d3320 · View workflow run →

- Move file-path merge and validation from RunE to PreRunE for earlier
  error reporting before expensive policy initialization
- Add deduplication of file paths when both --file flag and positional
  arguments provide the same file
- Update all 7 acceptance test scenarios to use positional arguments
  instead of --file flag, preventing CI failures from deprecation
  warning on stderr
- Regenerate auto-generated documentation (ec_validate_input.adoc)
- Remove unnecessary SetTestRekorPublicKey from no-file-provided test

Addresses review feedback on #3382
@fullsend-ai-coder

Copy link
Copy Markdown
Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 4 of 5 review findings. Fixed the high-severity acceptance test breakage by converting all scenarios to positional args. Regenerated auto-generated docs. Moved validation to PreRunE for earlier error reporting. Added file path deduplication. Disagreed with converting existing unit tests (out of scope enhancement).

Fixed (4):

  1. Acceptance tests will break due to deprecation warning on stderr (features/validate_input.feature): Updated all 7 acceptance test scenarios to use positional arguments instead of --file flag. This prevents the deprecation warning from appearing on stderr and breaking snapshot comparisons that expect empty stderr.
  2. Auto-generated documentation not regenerated (docs/modules/ROOT/pages/ec_validate_input.adoc): Ran go generate ./cmd/... to regenerate ec_validate_input.adoc. The file now reflects updated usage line, positional arg examples, and DEPRECATED flag description.
  3. Error validation timing changed (cmd/validate/input.go): Moved file-path merge and emptiness check from RunE to PreRunE, before policy initialization. This provides earlier error reporting when no files are specified, avoiding unnecessary policy initialization work.
  4. Duplicate file paths not deduplicated (cmd/validate/input.go): Added deduplication logic after merging positional arguments and --file flag values. Uses a seen map to preserve order while removing duplicates.

Disagreed (1):

  1. Existing tests don't exercise the new primary path: Converting existing unit tests from --file to positional args is a test-improvement suggestion that goes beyond the scope of this PR's review fix. The 3 new tests already cover the positional args code path (single, multiple, and mixed). Converting existing tests would be a separate enhancement.

Tests: passed

Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 6, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 2:59 PM UTC · Ended 3:13 PM UTC
Commit: 47d3320 · View workflow run →

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.87500% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cmd/validate/input.go 96.87% 1 Missing ⚠️
Flag Coverage Δ
acceptance 53.48% <90.62%> (+0.05%) ⬆️
generative 16.65% <0.00%> (-0.14%) ⬇️
integration 27.48% <53.12%> (-0.19%) ⬇️
unit 68.73% <96.87%> (-0.40%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cmd/validate/input.go 94.24% <96.87%> (+0.08%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fullsend-ai-review fullsend-ai-review Bot dismissed their stale review July 6, 2026 15:13

Superseded by updated review

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jul 6, 2026
@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:59 PM UTC · Completed 3:13 PM UTC
Commit: 47d3320 · View workflow run →

@st3penta

st3penta commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

A few things from the previous review round are still open. The fix agent addressed the biggest items (PreRunE timing, dedup, acceptance tests, docs regen), but these were left untouched:

  1. cobra.ArbitraryArgs silently accepts mistyped flags as file paths. ec validate input --polcy foo.yaml treats --polcy as a file path instead of erroring. validate/vsa.go already has a custom Args validator that catches this. No edge case tests for this either (empty strings, non-existent files, flag-like args).
  2. log.Warn on --file usage is still there. CI is green now (acceptance tests migrated to positional args), but image.go deprecates flags silently with just MarkHidden + help text. Users invoking --file still get a stderr warning, which can break structured output consumers piping stderr.
  3. No test for the deprecation warning (emitted when --file is used, absent with positional args only).
  4. Dedup logic was added but no test pins the contract. Test_ValidateInputCmd_MixedPositionalAndFlag uses different files so dedup never fires.
  5. Multi-file --file patterns (--file a --file b, --file="a,b") still work via StringSlice but are no longer documented. One deprecated example was kept, but users with multi-file scripts lose migration guidance.
  6. The three new test functions share ~80% boilerplate (fs setup, command construction, JSON parsing, type assertion). A table-driven test would consolidate setup and make it easier to add cases for dedup, invalid args, etc.
  7. Use field says input [flags] [file ...] but all examples show files before flags.
  8. New tests use t.Fatal + manual type assertions instead of require (already imported in the file).

@st3penta

st3penta commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

/fs-fix

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 7, 2026

Copy link
Copy Markdown
Author

🤖 Finished Fix · ✅ Success · Started 12:12 PM UTC · Completed 12:35 PM UTC
Commit: 7c8ccca · View workflow run →

- Replace cobra.ArbitraryArgs with custom validator rejecting flag-like args
- Remove log.Warn deprecation warning to match image.go silent pattern
- Use errors.Join for empty-files check (error accumulation consistency)
- Add deprecated --file multi-file patterns to examples/docs
- Fix Use field ordering to show files before flags
- Consolidate 3 positional arg tests into table-driven test with require
- Add dedup test case and flag-like arg rejection test
- Regenerate ec_validate_input.adoc via go generate

Addresses review feedback on #3382
@github-actions github-actions Bot added size: XL and removed size: L labels Jul 7, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 7, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:36 PM UTC · Completed 12:42 PM UTC
Commit: 7c8ccca · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jul 7, 2026
@st3penta

st3penta commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

/fs-fix address all the unresolved inline comments

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 7, 2026

Copy link
Copy Markdown
Author

🤖 Finished Fix · ✅ Success · Started 3:47 PM UTC · Completed 4:05 PM UTC
Commit: 7c8ccca · View workflow run →

- Add early return in PreRunE when no input files provided, avoiding
  unnecessary policy initialization and potential network I/O
- Add test case for single-dash flag-like positional args (-x)
- Document early-return contract in NoFileProvided test comment

Addresses review feedback on #3382
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 7, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:06 PM UTC · Completed 4:14 PM UTC
Commit: 7c8ccca · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

Comment thread cmd/validate/input.go
Enable color when using text output even when the current terminal does not support it`))

if err := cmd.MarkFlagRequired("file"); err != nil {
if err := cmd.Flags().MarkHidden("file"); err != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] correctness

MarkHidden is used instead of MarkDeprecated, so no deprecation warning is emitted to stderr when --file is used. The PR description claims a deprecation warning is logged, but MarkHidden only hides the flag from help — it does not produce any runtime warning. Cobra's MarkDeprecated function both hides the flag and emits a warning, which is the stated behavior.

Suggested fix: Replace cmd.Flags().MarkHidden("file") with cmd.Flags().MarkDeprecated("file", "use positional arguments instead"). MarkDeprecated calls MarkHidden internally, so no other changes are needed.

Comment thread cmd/validate/input.go
'git::'. For the policy configuration files you can use json instead of yaml if you prefer.

ec validate input --file /path/to/file.json --policy github.com/user/repo//default?ref=main
ec validate input /path/to/file.json --policy github.com/user/repo//default?ref=main

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] correctness

The custom Args validator rejects any positional argument starting with '-', which prevents valid filenames like '-report.yaml' from being used even after the '--' end-of-flags marker. Additionally, this validator is redundant — Cobra already rejects unknown flags during its own flag parsing phase before Args is called. The two test cases in Test_ValidateInputCmd_InvalidPositionalArgs exercise Cobra's built-in error handling, not this custom validator.

Suggested fix: Consider removing the custom Args function entirely (Cobra's default allows arbitrary args), or at minimum accept dash-prefixed args after validating they aren't known flag patterns.

version: "0.1"
"""
When ec command is run with "validate input --file pipeline_definition.yaml --policy git::https://${GITHOST}/git/happy-day-config.git --output json"
When ec command is run with "validate input pipeline_definition.yaml --policy git::https://${GITHOST}/git/happy-day-config.git --output json"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] test-coverage

All seven acceptance test scenarios were migrated from --file to positional args. No acceptance-level test remains for the deprecated --file flag's backward compatibility. Unit tests still cover --file, but the acceptance test gap means end-to-end backward compatibility is not validated.

Suggested fix: Keep at least one acceptance scenario using the --file flag to ensure end-to-end backward compatibility is tested.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change ec validate input to use positional arguments

1 participant