couchbaselabs / couchbaselabs/fit-cli

Shared workflow declares definition_file / definition_base_64 but never uses them

Open
#37 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
0
Forks
4
Avg merge
2d 19h
Merged PRs (30d)
16

Description

`.github/workflows/fit-cli.yaml` declares `definition_file` and `definition_base_64` on both the `workflow_dispatch` and `workflow_call` interfaces, and reads neither anywhere in the file. The run step is unconditionally:

```bash
args=(preset "$PRESET" --performer "$PERFORMER")
```

So a caller that supplies a definition silently gets the plain preset instead. It does not error — it produces a green run that tested something other than what was asked for, which is the failure mode most likely to go unnoticed.

### How it got here

`6cbde10` added both inputs already dead, so nothing has regressed and no behaviour was lost. A code search across `couchbase` and `couchbaselabs` finds no workflow passing either, so nothing depends on them today.

### Why this isn't a quick wiring job

Worth recording, since "just pass it through" is the obvious first instinct:

* `--performer` is preset-only — a definition carries its image inside — but the workflow's `performer` input is `required: true`, so every definition caller would have to supply a value that cannot be used.
* `--env-override` (added in #36) is preset-only too, since a definition is already rendered. That input and a definition path are mutually exclusive.
* The job matrix comes from `fit definition expand-preset-group "$PRESETS"`. A definition is not a preset list, so the matrix has no meaning for it.

Wiring them up therefore means conditional input requirements, a second matrix strategy, and a branched command construction — a change to the workflow's contract rather than a passthrough.

### Options

1. **Remove them.** `fit run definition` stays available on the CLI, where gist URLs were recently added (`ff125a6`). Picking a server version from CI — the one use case I reached for a definition for — is covered by `env_override` in #36.
2. **Implement them**, accepting the contract change above.

Either is fine by me; the thing worth not leaving as-is is an input that looks supported and silently isn't. I had a removal commit on #36 and dropped it, since which way to go is a design call for this repo rather than something to decide in a drive-by PR.

Found while adding `env_override` (#36) so that .NET could verify hybrid search score fusion against Server 8.1 in CI — that feature's tests all skip below 8.1, so the default `8.0-stable` run proves nothing.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with .github/workflows/fit-cli.yaml and inspect its workflow inputs, matrix strategy, and run step. Decide with maintainers whether the unused definition inputs should be removed or supported as a separate workflow path. Done means the workflow contract no longer advertises inputs that are silently ignored, with the relevant workflow checks passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions
Domain
ci-cd, cli
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.