Skip to content

ci: fix version smoke assertion#34

Merged
vincentkoc merged 4 commits into
mainfrom
fix/ci-smoke-script
Jun 25, 2026
Merged

ci: fix version smoke assertion#34
vincentkoc merged 4 commits into
mainfrom
fix/ci-smoke-script

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

What Problem This Solves

The CLI smoke test failed because graincrawl version --json currently emits the legacy key-value output shape, not a JSON object containing "version": "ci".

Why This Change Was Made

Updated the CI assertion to check the actual emitted version: ci line from the workflow-built binary.

User Impact

Restores green CI without changing CLI runtime behavior.

Evidence

  • Workflow-equivalent go build -trimpath with CI linker metadata
  • /tmp/graincrawl-smoke version --json | grep -Eq '^version:[[:space:]]*ci$'
  • GOWORK=off go vet ./...
  • GOWORK=off go test -count=1 ./...

@vincentkoc vincentkoc requested a review from a team as a code owner June 25, 2026 06:50
@github-actions github-actions Bot added the ci label Jun 25, 2026
@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed June 25, 2026, 2:54 AM ET / 06:54 UTC.

Summary
The PR updates the CI workflow's CLI smoke test version assertion from a JSON object grep to a key-value version: ci grep.

Reproducibility: yes. The PR's own CI run fails in Smoke test CLI control surface, and source inspection shows post-command --json is not parsed as the global JSON flag.

Review metrics: 2 noteworthy metrics.

  • Workflow surface: 1 workflow changed, 1 line modified. The PR is a tightly scoped CI change, so the remaining blocker is in the same smoke-test block rather than broad application code.
  • Current PR CI: 1 failing CI job. The live PR test job still fails in the smoke step that this PR is meant to repair.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🌊 off-meta tidepool
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Fix the remaining --json smoke command mismatches in .github/workflows/ci.yml.
  • Rerun the PR CI and confirm the test job passes.

Risk before merge

  • [P1] Merging as-is would leave the CI test job failing in the CLI smoke step, so repository automation would remain broken even though this PR is intended to restore green CI.

Maintainer options:

  1. Fix all smoke JSON assertions before merge (recommended)
    Update the remaining smoke commands so --json is passed before the command name, or change their greps to match text output, then require the PR CI test job to pass.
  2. Close and replace if the branch stays narrow
    If maintainers do not want to repair this branch, close it and open a replacement that fixes the full smoke-step argument-order mismatch in one pass.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Update `.github/workflows/ci.yml` so every CLI smoke command's `--json` flag placement matches the parser or every grep matches the actual text output; keep runtime CLI behavior unchanged and verify with the workflow-equivalent smoke step plus `GOWORK=off go test -count=1 ./...`.

Next step before merge

  • [P2] A focused automation repair is possible because the remaining failure is confined to the CI smoke command and grep expectations.

Security
Cleared: The diff only changes a grep assertion in CI and does not add actions, permissions, dependencies, secrets handling, or code execution sources.

Review findings

  • [P2] Fix the rest of the JSON smoke assertions — .github/workflows/ci.yml:119-123
Review details

Best possible solution:

Land a narrow workflow fix that makes every smoke command's JSON expectation match the CLI's actual flag parsing, preferably without changing runtime CLI behavior.

Do we have a high-confidence way to reproduce the issue?

Yes. The PR's own CI run fails in Smoke test CLI control surface, and source inspection shows post-command --json is not parsed as the global JSON flag.

Is this the best way to solve the issue?

No. Updating only the version assertion is too narrow; the maintainable fix should align the whole smoke block with the CLI's current flag contract or deliberately change that contract with tests.

Full review comments:

  • [P2] Fix the rest of the JSON smoke assertions — .github/workflows/ci.yml:119-123
    This smoke step still calls several commands as command --json while grepping for JSON fields, but --json is only parsed before the command name. The PR run fails this same smoke step, so please move --json before the command names or update these greps to the actual text output instead of only changing the version assertion.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.92

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 4156d9f6390d.

Label changes

Label changes:

  • add P2: This is a normal-priority CI workflow repair with limited runtime user impact but a concrete failing automation path.
  • add merge-risk: 🚨 automation: The diff changes CI smoke-test behavior and the PR run shows the workflow would still fail if merged as-is.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The external-contributor proof gate does not apply to this member-authored PR; the live CI run provides negative validation that the fix is incomplete.

Label justifications:

  • P2: This is a normal-priority CI workflow repair with limited runtime user impact but a concrete failing automation path.
  • merge-risk: 🚨 automation: The diff changes CI smoke-test behavior and the PR run shows the workflow would still fail if merged as-is.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The external-contributor proof gate does not apply to this member-authored PR; the live CI run provides negative validation that the fix is incomplete.
Evidence reviewed

Acceptance criteria:

  • [P1] GOWORK=off go test -count=1 ./...
  • [P1] Workflow-equivalent GOWORK=off go build -trimpath with CI linker metadata and the CLI smoke commands from .github/workflows/ci.yml.

What I checked:

  • PR diff: The branch changes only one assertion in the CI smoke step, replacing the version JSON grep with a key-value version: ci grep. (.github/workflows/ci.yml:114, 33bdc7e500e4)
  • Current workflow context: The same smoke step still invokes init, metadata, status, tui, and snapshot with --json after the command name while grepping for JSON-shaped output. (.github/workflows/ci.yml:119, 4156d9f6390d)
  • Parser behavior: parseGlobalFlags stops parsing global flags when it reaches the command name, so command-local --json is left in the command arguments and does not set flags.JSON. (internal/cli/flags.go:12, 4156d9f6390d)
  • Version output behavior: runVersion emits the JSON envelope only when flags.JSON is set; otherwise it prints key-value lines for version, commit, and date. (internal/cli/app.go:213, 4156d9f6390d)
  • Live CI result: The PR merge ref ran the changed smoke step and the CI test job failed with exit code 1 in Smoke test CLI control surface; the PR remains mergeable but UNSTABLE. (.github/workflows/ci.yml:114, d7874a972f15)
  • Related item search: Searches for version smoke assertion, version --json, and ci smoke found no clearer canonical issue or replacement PR for this workflow failure beyond this PR.

Likely related people:

  • vincentkoc: Authored current main commit 4156d9f, which touched the CI/release workflows and CLI flag handling around this smoke-test surface before this PR. (role: recent area contributor; confidence: high; commits: 4156d9f6390d; files: .github/workflows/ci.yml, internal/cli/app.go, internal/cli/flags.go)
  • steipete: The initial v0.3.1 release commit introduced the CI workflow, CLI app, flag parser, and version output behavior that this PR is adjusting. (role: introduced workflow and CLI baseline; confidence: high; commits: 2cc577b8ebf4; files: .github/workflows/ci.yml, internal/cli/app.go, internal/cli/flags.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jun 25, 2026
@vincentkoc vincentkoc merged commit 9343c18 into main Jun 25, 2026
10 checks passed
@vincentkoc vincentkoc deleted the fix/ci-smoke-script branch June 25, 2026 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant