Fail fast on unpublished pinned agent versions in codefly ci run (#101) - #102
Conversation
Resolve each affected service's pinned agent against its configured artifact sources before the phase loop runs, so an unpublished pin (a git tag with no downloadable release asset / OCI manifest) fails fast with a single legible report instead of an opaque mid-run 404. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🤖 CodeAnt AI — Review Status
Updated in place by CodeAnt AI · last 5 reviews |
💳 Subscription Requiredcodefly-dev has used all free reviews this month. |
| if _, err := resolveAgentLatest(ctx, agent); err != nil { | ||
| unpublished = append(unpublished, agentArtifactStatus{ | ||
| agent: agent, | ||
| sources: []agentSourceProbe{{label: "version resolution", detail: err.Error()}}, | ||
| }) | ||
| continue |
There was a problem hiding this comment.
Suggestion: Any version-resolution failure is currently reported as “not downloadable / no published artifact,” but resolution errors can also be transient/environmental (network/auth/config) and are not equivalent to an unpublished pin. Return a distinct error path for resolution failures so the report does not misclassify the root cause. [logic error]
Severity Level: Major ⚠️
- ⚠️ Network or auth issues misreported as unpublished agent pins.
- ⚠️ CI users may chase incorrect “publish artifact” fixes.
- ⚠️ Harder to distinguish real missing releases from infra issues.Steps of Reproduction ✅
1. Run `codefly ci run` so `RunCmd.RunE` in `cmd/ci/run.go:22-86` executes and constructs
a non-empty `plan` with services that have agents (`Plan.BuildPlan` and
`collectPlanAgents` in `cmd/ci/agent_versions.go:94-123`).
2. Within the `runWithCIReport` callback in `cmd/ci/run.go:54-85`,
`validateAgentVersions(ctx, workspace, plan)` is called before any phase runs, entering
the loop over agents at `cmd/ci/agent_versions.go:70-86`.
3. For an agent whose version cannot be resolved (i.e., `resolveAgentLatest(ctx, agent)`
returns a non-nil error for any reason such as network, auth, or configuration), the code
at `cmd/ci/agent_versions.go:72-77` appends an `agentArtifactStatus` whose only source is
`{label: "version resolution", detail: err.Error()}` and classifies this under the
`unpublished` slice, then continues without distinguishing transient failures from truly
unpublished pins.
4. After the loop, `formatUnpublishedReport(unpublished)` is called
(`cmd/ci/agent_versions.go:88-92`), producing a report whose header and per-agent lines
state that the agent pins are “not downloadable in CI (no published artifact)” and “is not
published (no CI-downloadable artifact)” (`cmd/ci/agent_versions.go:212-215`, asserted in
`cmd/ci/agent_versions_test.go:114-131`), so any resolution error—regardless of cause—is
surfaced to users as a “no published artifact” problem rather than as a distinct
resolution or environment failure.(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** cmd/ci/agent_versions.go
**Line:** 72:77
**Comment:**
*Logic Error: Any version-resolution failure is currently reported as “not downloadable / no published artifact,” but resolution errors can also be transient/environmental (network/auth/config) and are not equivalent to an unpublished pin. Return a distinct error path for resolution failures so the report does not misclassify the root cause.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if local, err := agentAlreadyLocal(ctx, agent); err == nil && local { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Suggestion: The local-agent presence check suppresses all errors and treats them as “not local,” which can mask filesystem/configuration failures and produce misleading unpublished-artifact reports. Propagate or explicitly handle the agentAlreadyLocal error instead of silently ignoring it. [incorrect condition logic]
Severity Level: Major ⚠️
- ⚠️ Local cache errors hidden behind remote probe behavior.
- ⚠️ Unpublished-agent report omits underlying local failure context.
- ⚠️ Debugging CI agent issues harder for affected services.Steps of Reproduction ✅
1. Invoke the `codefly ci run` CLI entrypoint implemented in `cmd/ci/run.go:22-86` (e.g.,
`codefly ci run`) so that `RunCmd.RunE` executes.
2. Inside `RunCmd.RunE`, after building the plan and phases (`cmd/ci/run.go:45-52`),
`runWithCIReport` is called with a callback that unconditionally calls
`validateAgentVersions(ctx, workspace, plan)` (`cmd/ci/run.go:54-57`).
3. In `validateAgentVersions` (`cmd/ci/agent_versions.go:56-92`), each collected agent is
processed; after successful `resolveAgentLatest`, the code checks local presence via
`agentAlreadyLocal(ctx, agent)` aliased to `manager.Downloaded`
(`cmd/ci/agent_versions.go:21-23, 79-81`).
4. When `manager.Downloaded` returns a non-nil error for a given agent (for example due to
a filesystem or local cache/configuration problem), the condition `err == nil && local` on
line 79 is false, the error is silently discarded (no logging or wrapping), and the loop
continues to remote probing as if the agent were simply “not local”, masking the original
local failure and leading any later report (from `formatUnpublishedReport` at
`cmd/ci/agent_versions.go:206-221`) to omit the real local error cause.(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** cmd/ci/agent_versions.go
**Line:** 79:81
**Comment:**
*Incorrect Condition Logic: The local-agent presence check suppresses all errors and treats them as “not local,” which can mask filesystem/configuration failures and produce misleading unpublished-artifact reports. Propagate or explicitly handle the `agentAlreadyLocal` error instead of silently ignoring it.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if err := validateAgentVersions(ctx, workspace, plan); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Suggestion: The pre-flight validation is now unconditional for every ci run invocation, which will fail runs that only execute non-agent phases (for example verify) due to unrelated artifact/network checks. Gate this validation so it only runs when at least one selected phase actually needs service agents. [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ `codefly ci run --phase verify` can fail on agents.
- ⚠️ Workspace integrity checks blocked by unrelated agent publishing.
- ⚠️ CI pipeline for verify-only gates becomes unnecessarily fragile.Steps of Reproduction ✅
1. Execute `codefly ci run --phase verify` (or an equivalent Cobra invocation) so that
`RunCmd.RunE` in `cmd/ci/run.go:22-86` is entered, `runPhases` contains only `"verify"`,
and `plan` is built for the workspace services (`cmd/ci/run.go:45-48`).
2. `normalizeRunPhases` at `cmd/ci/run.go:171-192` returns a `phases` slice containing
only `"verify"`, which is then passed into the `runWithCIReport` callback at
`cmd/ci/run.go:54-85`.
3. Before any phase-specific logic runs, the callback calls `validateAgentVersions(ctx,
workspace, plan)` unconditionally at `cmd/ci/run.go:55-57`; inside `validateAgentVersions`
(`cmd/ci/agent_versions.go:56-92`), every agent pin is resolved and probed against
GitHub/OCI/Nix, and if any pin lacks a downloadable artifact or hits a transient error, an
error report is returned immediately.
4. Because `runCIPhases` is only invoked after this callback completes successfully
(`cmd/ci/run.go:81-84`), a `--phase verify` run will fail early on agent artifact/network
issues even though the only selected phase, `verify`, is implemented by
`runVerifyWorkspace` (`cmd/ci/run.go:120-123`) which in turn calls `integrity.VerifyBase`
over module manifests (`pkg/integrity/base.go:53-132`) and does not depend on service
agents, meaning verify-only runs are incorrectly gated on agent artifact availability.(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** cmd/ci/run.go
**Line:** 55:57
**Comment:**
*Incomplete Implementation: The pre-flight validation is now unconditional for every `ci run` invocation, which will fail runs that only execute non-agent phases (for example `verify`) due to unrelated artifact/network checks. Gate this validation so it only runs when at least one selected phase actually needs service agents.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
User description
Closes #101.
Summary
cannot download agent ... 404deep inside a phase. This adds a pre-flight step tocodefly ci runthat resolves every affected service's agent against its configured sources before the phase loop and fails fast with one legible report listing every pin that has no downloadable artifact.AGENT_REGISTRY) and Nix flake output (AGENT_NIX_FLAKE). An agent already present locally, or resolvable underCODEFLY_AGENT_SOURCE=local, is skipped so dev/offline runs don't hit spurious failures.Test plan
go test ./cmd/ci/— new probe/report tests exercise missing vs. published GitHub assets and OCI manifests against realhttptestservers (no mocks), plus the report formatting.go build ./...go vet ./cmd/ci/CodeAnt-AI Description
Fail fast when a pinned agent version is not published
What Changed
codefly ci runnow checks each affected service’s pinned agent before CI phases start, instead of failing later with a hidden download errorImpact
✅ Fewer mid-run CI failures✅ Clearer missing-agent errors✅ Faster feedback on unpublished agent pins🔄 Retrigger CodeAnt AI Review
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.