feat: dev binary channel with CLI flags and auto-update#11
feat: dev binary channel with CLI flags and auto-update#11TerrifiedBug merged 12 commits intomainfrom
Conversation
Greptile SummaryThis PR introduces a dev binary channel end-to-end: CI publishes rolling Key points:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant CI as GitHub Actions CI
participant GH as GitHub Releases
participant Browser as Fleet UI
participant tRPC as settings.checkVersion
participant DB as PostgreSQL
participant Agent as vf-agent (node)
Note over CI,GH: On every push to main
CI->>CI: Build vf-agent-linux-amd64 + arm64 (dev-SHA)
CI->>CI: Write dev-version.txt (e.g. dev-abc1234)
CI->>GH: gh release delete dev (idempotent)
CI->>GH: gh release create dev (prerelease, binaries + checksums)
Note over Browser,Agent: Fleet page load
Browser->>tRPC: checkVersion (staleTime: Infinity)
tRPC->>DB: findUnique SystemSettings (×3 parallel)
alt needsCheck (>24h or force)
tRPC->>GH: GET /releases/latest (stable agent)
tRPC->>GH: GET /releases/tags/dev (dev agent)
GH-->>tRPC: release metadata + assets
tRPC->>GH: GET dev-version.txt asset
tRPC->>GH: GET checksums.txt assets
tRPC->>DB: upsert SystemSettings (versions + checksums)
end
tRPC-->>Browser: { agent, devAgent: { latestVersion, checksums } }
Browser->>Browser: getNodeLatest(node) — route by dev- prefix
alt node.agentVersion starts with "dev-"
Browser->>Browser: compare SHA: current !== latestDevVersion
Browser-->>Browser: show "Update available" badge
Browser->>tRPC: fleet.triggerAgentUpdate (tag="dev", url=.../dev/vf-agent-linux-amd64)
tRPC->>Agent: pendingAction → self-update
else stable node
Browser->>Browser: semver compare
Browser->>tRPC: fleet.triggerAgentUpdate (tag="v{ver}", stable binary)
tRPC->>Agent: pendingAction → self-update
end
Last reviewed commit: 3efbfc1 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
prisma/schema.prisma
Outdated
| latestServerReleaseCheckedAt DateTime? | ||
| latestAgentRelease String? | ||
| latestAgentReleaseCheckedAt DateTime? | ||
| latestDevAgentRelease String? |
There was a problem hiding this comment.
Missing Prisma migration for new fields
Two new columns are added to SystemSettings (latestDevAgentRelease, latestDevAgentReleaseCheckedAt), but no corresponding migration file exists in prisma/migrations/. The most recent migration is 20260304000000_add_backup_settings — nothing covers these fields.
At runtime, Prisma's generated client will expect these columns to exist in PostgreSQL. Without a migration, queries in checkDevAgentVersion that read settings?.latestDevAgentRelease and settings?.latestDevAgentReleaseCheckedAt will fail (or silently return undefined) on a production database that hasn't had this column added.
Per the project conventions, a migration needs to be generated with:
npx prisma migrate dev --name add_dev_agent_version
and the resulting file committed alongside the schema change.
Prompt To Fix With AI
This is a comment left during a code review.
Path: prisma/schema.prisma
Line: 415-418
Comment:
**Missing Prisma migration for new fields**
Two new columns are added to `SystemSettings` (`latestDevAgentRelease`, `latestDevAgentReleaseCheckedAt`), but no corresponding migration file exists in `prisma/migrations/`. The most recent migration is `20260304000000_add_backup_settings` — nothing covers these fields.
At runtime, Prisma's generated client will expect these columns to exist in PostgreSQL. Without a migration, queries in `checkDevAgentVersion` that read `settings?.latestDevAgentRelease` and `settings?.latestDevAgentReleaseCheckedAt` will fail (or silently return `undefined`) on a production database that hasn't had this column added.
Per the project conventions, a migration needs to be generated with:
```
npx prisma migrate dev --name add_dev_agent_version
```
and the resulting file committed alongside the schema change.
How can I resolve this? If you propose a fix, please make it concise.
src/server/services/version-check.ts
Outdated
| if (needsCheck) { | ||
| const release = await fetchDevRelease(); | ||
| if (release) { | ||
| latestVersion = await fetchDevVersionString(release) ?? "dev-unknown"; |
There was a problem hiding this comment.
"dev-unknown" fallback triggers false update notifications
When fetchDevVersionString returns null (e.g., the dev-version.txt asset is missing from the release), latestVersion is set to the string "dev-unknown". The isVersionOlder function in src/lib/version.ts then compares current !== latest for dev-to-dev checks.
This means any running dev agent (e.g. "dev-abc1234") will always be considered "outdated" when the server can't determine the real latest SHA, because "dev-abc1234" !== "dev-unknown" is true. The Fleet UI would show an "Update available" badge and — worse — clicking Update would dispatch an update action with targetVersion: "dev-unknown" and whatever incomplete checksums were fetched.
A safer approach is to return null and skip the update offer when the version string cannot be determined:
| latestVersion = await fetchDevVersionString(release) ?? "dev-unknown"; | |
| latestVersion = await fetchDevVersionString(release) ?? null; |
Then in the upsert, only write to the DB if latestVersion is non-null, and the caller already handles latestVersion: null safely.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/server/services/version-check.ts
Line: 216
Comment:
**`"dev-unknown"` fallback triggers false update notifications**
When `fetchDevVersionString` returns `null` (e.g., the `dev-version.txt` asset is missing from the release), `latestVersion` is set to the string `"dev-unknown"`. The `isVersionOlder` function in `src/lib/version.ts` then compares `current !== latest` for dev-to-dev checks.
This means any running dev agent (e.g. `"dev-abc1234"`) will always be considered "outdated" when the server can't determine the real latest SHA, because `"dev-abc1234" !== "dev-unknown"` is `true`. The Fleet UI would show an "Update available" badge and — worse — clicking Update would dispatch an update action with `targetVersion: "dev-unknown"` and whatever incomplete checksums were fetched.
A safer approach is to return `null` and skip the update offer when the version string cannot be determined:
```suggestion
latestVersion = await fetchDevVersionString(release) ?? null;
```
Then in the `upsert`, only write to the DB if `latestVersion` is non-null, and the caller already handles `latestVersion: null` safely.
How can I resolve this? If you propose a fix, please make it concise.| let latestVersion = settings?.latestDevAgentRelease ?? null; | ||
| let checksums: Record<string, string> = {}; | ||
| let checkedAt: Date | null = lastChecked ?? null; | ||
|
|
||
| if (needsCheck) { | ||
| const release = await fetchDevRelease(); | ||
| if (release) { | ||
| latestVersion = await fetchDevVersionString(release) ?? "dev-unknown"; | ||
| checksums = await fetchChecksums(release); | ||
| checkedAt = new Date(); | ||
| await prisma.systemSettings.upsert({ | ||
| where: { id: "singleton" }, | ||
| update: { | ||
| latestDevAgentRelease: latestVersion, | ||
| latestDevAgentReleaseCheckedAt: checkedAt, | ||
| }, | ||
| create: { | ||
| id: "singleton", | ||
| latestDevAgentRelease: latestVersion, | ||
| latestDevAgentReleaseCheckedAt: checkedAt, | ||
| }, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| return { latestVersion, checksums, checkedAt }; |
There was a problem hiding this comment.
Checksums are not persisted and will be empty on cache hits
The same pattern applies to the existing checkAgentVersion function: checksums are fetched from GitHub when needsCheck = true, but they are never written to the database. When a cached response is served (needsCheck = false), the function returns checksums: {}.
In fleet/page.tsx the versionQuery uses staleTime: Infinity, so the React Query client-side cache preserves checksums for the session lifetime. However, for any new session where the server-side 24-hour cache is still valid, the server returns empty checksums, causing the Update button to dispatch an action with checksum: "sha256:". In the agent's updater.go, this results in a guaranteed checksum mismatch error and a failed update.
Consider adding a checksums (JSON string) column to SystemSettings and persisting the parsed map alongside the version, mirroring how latestDevAgentRelease is stored. This affects both stable and dev channels.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/server/services/version-check.ts
Line: 209-234
Comment:
**Checksums are not persisted and will be empty on cache hits**
The same pattern applies to the existing `checkAgentVersion` function: `checksums` are fetched from GitHub when `needsCheck = true`, but they are never written to the database. When a cached response is served (`needsCheck = false`), the function returns `checksums: {}`.
In `fleet/page.tsx` the `versionQuery` uses `staleTime: Infinity`, so the React Query client-side cache preserves checksums for the session lifetime. However, for any new session where the server-side 24-hour cache is still valid, the server returns empty checksums, causing the Update button to dispatch an action with `checksum: "sha256:"`. In the agent's `updater.go`, this results in a guaranteed checksum mismatch error and a failed update.
Consider adding a `checksums` (JSON string) column to `SystemSettings` and persisting the parsed map alongside the version, mirroring how `latestDevAgentRelease` is stored. This affects both stable and dev channels.
How can I resolve this? If you propose a fix, please make it concise.| VF_TOKEN=${VF_TOKEN} | ||
| VF_DATA_DIR=${DATA_DIR} |
There was a problem hiding this comment.
VF_CHANNEL is written to the env file but never read by the agent
The install script writes VF_CHANNEL=${CHANNEL} to the environment file, which suggests the agent should be channel-aware. However, agent/internal/config/config.go does not read VF_CHANNEL, and it is absent from the Config struct. The --help output in agent/main.go also does not list it.
In practice, cross-channel safety is enforced entirely by the server-side isVersionOlder check on the version string prefix (dev-). The VF_CHANNEL env var in the env file is currently dead code and may confuse operators who try to rely on it.
Either remove the write from the install script or wire it up through config.go so the agent can use it (e.g., to include the channel in heartbeat payloads for server-side validation).
Prompt To Fix With AI
This is a comment left during a code review.
Path: agent/install.sh
Line: 206-207
Comment:
**`VF_CHANNEL` is written to the env file but never read by the agent**
The install script writes `VF_CHANNEL=${CHANNEL}` to the environment file, which suggests the agent should be channel-aware. However, `agent/internal/config/config.go` does not read `VF_CHANNEL`, and it is absent from the `Config` struct. The `--help` output in `agent/main.go` also does not list it.
In practice, cross-channel safety is enforced entirely by the server-side `isVersionOlder` check on the version string prefix (`dev-`). The `VF_CHANNEL` env var in the env file is currently dead code and may confuse operators who try to rely on it.
Either remove the write from the install script or wire it up through `config.go` so the agent can use it (e.g., to include the channel in heartbeat payloads for server-side validation).
How can I resolve this? If you propose a fix, please make it concise.- Replace "dev-unknown" fallback with null to prevent false update notifications for dev agents when version string can't be fetched - Add missing Prisma migration for latestDevAgentRelease fields - Add comment clarifying VF_CHANNEL is documentation-only
| if (needsCheck) { | ||
| const release = await fetchDevRelease(); | ||
| if (release) { | ||
| const versionString = await fetchDevVersionString(release); | ||
| if (versionString) { | ||
| latestVersion = versionString; | ||
| checksums = await fetchChecksums(release); | ||
| checkedAt = new Date(); | ||
| await prisma.systemSettings.upsert({ | ||
| where: { id: "singleton" }, | ||
| update: { | ||
| latestDevAgentRelease: latestVersion, | ||
| latestDevAgentReleaseCheckedAt: checkedAt, | ||
| }, | ||
| create: { | ||
| id: "singleton", | ||
| latestDevAgentRelease: latestVersion, | ||
| latestDevAgentReleaseCheckedAt: checkedAt, | ||
| }, | ||
| }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
checkedAt never updated when dev-version.txt is missing
When fetchDevVersionString returns null (e.g., the dev-version.txt asset is absent from the release), the upsert block is never reached (lines 221–232). This means latestDevAgentReleaseCheckedAt is not written to the database, so lastChecked remains null on the next invocation of checkDevAgentVersion.
On subsequent calls, needsCheck evaluates to true (line 204–207) and fetchDevRelease() is called again—with no 24-hour cooldown in effect. Under normal load, repeated API calls without rate limiting can exhaust GitHub's unauthenticated API limit (60 req/hr), causing 403 errors.
Fix: Always update checkedAt when the release fetch succeeds, even if the version string is unavailable:
if (needsCheck) {
const release = await fetchDevRelease();
if (release) {
checkedAt = new Date(); // ← always bump the timestamp on successful fetch
const versionString = await fetchDevVersionString(release);
if (versionString) {
latestVersion = versionString;
checksums = await fetchChecksums(release);
}
await prisma.systemSettings.upsert({
where: { id: "singleton" },
update: {
latestDevAgentRelease: latestVersion,
latestDevAgentReleaseCheckedAt: checkedAt,
},
create: {
id: "singleton",
latestDevAgentRelease: latestVersion,
latestDevAgentReleaseCheckedAt: checkedAt,
},
});
}
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/server/services/version-check.ts
Line: 213-235
Comment:
`checkedAt` never updated when `dev-version.txt` is missing
When `fetchDevVersionString` returns `null` (e.g., the `dev-version.txt` asset is absent from the release), the `upsert` block is never reached (lines 221–232). This means `latestDevAgentReleaseCheckedAt` is not written to the database, so `lastChecked` remains `null` on the next invocation of `checkDevAgentVersion`.
On subsequent calls, `needsCheck` evaluates to `true` (line 204–207) and `fetchDevRelease()` is called again—with no 24-hour cooldown in effect. Under normal load, repeated API calls without rate limiting can exhaust GitHub's unauthenticated API limit (60 req/hr), causing 403 errors.
**Fix:** Always update `checkedAt` when the release fetch succeeds, even if the version string is unavailable:
```typescript
if (needsCheck) {
const release = await fetchDevRelease();
if (release) {
checkedAt = new Date(); // ← always bump the timestamp on successful fetch
const versionString = await fetchDevVersionString(release);
if (versionString) {
latestVersion = versionString;
checksums = await fetchChecksums(release);
}
await prisma.systemSettings.upsert({
where: { id: "singleton" },
update: {
latestDevAgentRelease: latestVersion,
latestDevAgentReleaseCheckedAt: checkedAt,
},
create: {
id: "singleton",
latestDevAgentRelease: latestVersion,
latestDevAgentReleaseCheckedAt: checkedAt,
},
});
}
}
```
How can I resolve this? If you propose a fix, please make it concise.| downloadUrl: `https://github.com/${AGENT_REPO}/releases/download/${latest.tag}/vf-agent-linux-amd64`, | ||
| checksum: `sha256:${latest.checksums["vf-agent-linux-amd64"] ?? ""}`, |
There was a problem hiding this comment.
Hardcoded amd64 architecture breaks arm64 agent updates
The download URL and checksum key both hardcode vf-agent-linux-amd64, but the CI workflow explicitly builds and publishes both vf-agent-linux-amd64 and vf-agent-linux-arm64 for the dev channel (.github/workflows/ci.yml lines 169–170, 188–189). An arm64 agent (e.g., running on AWS Graviton or Raspberry Pi) that is offered an update will download and attempt to execute the amd64 binary, causing the agent to crash or fail to restart.
The VectorNode model includes an os field but no architecture information. To fix this, either:
- Add an
archfield toVectorNode(persisted from agent heartbeat metadata) - Infer the architecture from
agentVersionormetadataif available - Default to
amd64with an override mechanism for known arm64 deployments
Then use the per-node architecture in the update command:
const arch = node.arch ?? "amd64"; // e.g. "amd64" | "arm64"
triggerUpdate.mutate({
nodeId: node.id,
targetVersion: latest.version!,
downloadUrl: `https://github.com/${AGENT_REPO}/releases/download/${latest.tag}/vf-agent-linux-${arch}`,
checksum: `sha256:${latest.checksums[`vf-agent-linux-${arch}`] ?? ""}`,
});Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/fleet/page.tsx
Line: 204-205
Comment:
Hardcoded `amd64` architecture breaks arm64 agent updates
The download URL and checksum key both hardcode `vf-agent-linux-amd64`, but the CI workflow explicitly builds and publishes both `vf-agent-linux-amd64` and `vf-agent-linux-arm64` for the dev channel (`.github/workflows/ci.yml` lines 169–170, 188–189). An arm64 agent (e.g., running on AWS Graviton or Raspberry Pi) that is offered an update will download and attempt to execute the `amd64` binary, causing the agent to crash or fail to restart.
The `VectorNode` model includes an `os` field but no architecture information. To fix this, either:
1. Add an `arch` field to `VectorNode` (persisted from agent heartbeat metadata)
2. Infer the architecture from `agentVersion` or `metadata` if available
3. Default to `amd64` with an override mechanism for known arm64 deployments
Then use the per-node architecture in the update command:
```typescript
const arch = node.arch ?? "amd64"; // e.g. "amd64" | "arm64"
triggerUpdate.mutate({
nodeId: node.id,
targetVersion: latest.version!,
downloadUrl: `https://github.com/${AGENT_REPO}/releases/download/${latest.tag}/vf-agent-linux-${arch}`,
checksum: `sha256:${latest.checksums[`vf-agent-linux-${arch}`] ?? ""}`,
});
```
How can I resolve this? If you propose a fix, please make it concise.The agent infers its channel from the version string prefix (dev- vs semver). Writing VF_CHANNEL to the env file served no purpose.
Previously, checksums fetched from GitHub were only held in memory. After a server restart within the 24h cache window, the version was loaded from the DB but checksums were empty, causing agent self-update checksum verification to fail. Store checksums as JSON in SystemSettings for both stable and dev channels, and load them on cache hits.
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: "1.22" | ||
| cache-dependency-path: agent/go.sum | ||
|
|
||
| - name: Build dev binaries | ||
| working-directory: agent | ||
| run: | | ||
| SHORT_SHA="${GITHUB_SHA::7}" | ||
| VERSION="dev-${SHORT_SHA}" | ||
| LDFLAGS="-s -w -X github.com/TerrifiedBug/vectorflow/agent/internal/agent.Version=${VERSION}" | ||
| CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -ldflags="${LDFLAGS}" -o ../vf-agent-linux-amd64 . | ||
| CGO_ENABLED=0 GOOS=linux GOARCH=arm64 go build -ldflags="${LDFLAGS}" -o ../vf-agent-linux-arm64 . | ||
| echo "${VERSION}" > ../dev-version.txt | ||
|
|
||
| - name: Generate checksums | ||
| run: sha256sum vf-agent-linux-* > checksums.txt | ||
|
|
||
| - name: Publish dev pre-release | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| # Delete existing dev release if present (gh errors if not found, ignore) | ||
| gh release delete dev --yes --cleanup-tag 2>/dev/null || true | ||
| # Create fresh pre-release pointing at current commit | ||
| gh release create dev \ | ||
| --title "Development Build" \ | ||
| --notes "Rolling dev build from \`${GITHUB_SHA::7}\` on main. Not for production use." \ | ||
| --target "${GITHUB_SHA}" \ | ||
| --prerelease \ | ||
| vf-agent-linux-amd64 \ | ||
| vf-agent-linux-arm64 \ | ||
| checksums.txt \ | ||
| dev-version.txt | ||
|
|
||
| agent-binaries: | ||
| name: Agent Binaries | ||
| needs: check |
There was a problem hiding this comment.
Dev release race condition on concurrent pushes
The agent-dev-binaries job has no concurrency group. If two commits land on main in quick succession, two workflow runs will overlap. The delete → create sequence is not atomic, so the following interleaving is possible:
- Run A (older SHA):
gh release delete dev← removes dev - Run A:
gh release create dev← publishes commit-A binaries - Run B (newer SHA):
gh release delete dev← removes commit-A binaries - Run A (still running): nothing left to do, exits cleanly
- Run B:
gh release create dev← publishes commit-B binaries ✓ (correct final state)
…or, if scheduling flips steps 2 and 3:
- Run A:
gh release delete dev - Run B:
gh release delete dev(already gone — silently ignored with|| true) - Run B:
gh release create dev← publishes commit-B binaries - Run A:
gh release create dev← FAILS with "release already exists"
In scenario 4, gh release create exits non-zero and the step fails, causing the newer commit's release to persist — but subsequent CI runs will surface a noisy failure that needs to be retried manually.
Add a job-level concurrency key to cancel in-progress runs:
agent-dev-binaries:
name: Agent Dev Binaries
needs: check
if: github.event_name == 'push' && !startsWith(github.ref, 'refs/tags/v')
runs-on: ubuntu-latest
concurrency:
group: dev-release
cancel-in-progress: trueWith cancel-in-progress: true, only the latest push's job ever reaches the delete/create steps, making the operation effectively atomic.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/ci.yml
Line: 156-195
Comment:
**Dev release race condition on concurrent pushes**
The `agent-dev-binaries` job has no `concurrency` group. If two commits land on `main` in quick succession, two workflow runs will overlap. The delete → create sequence is not atomic, so the following interleaving is possible:
1. Run A (older SHA): `gh release delete dev` ← removes dev
2. Run A: `gh release create dev` ← publishes commit-A binaries
3. Run B (newer SHA): `gh release delete dev` ← removes commit-A binaries
4. Run A (still running): nothing left to do, exits cleanly
5. Run B: `gh release create dev` ← publishes commit-B binaries ✓ (correct final state)
…or, if scheduling flips steps 2 and 3:
1. Run A: `gh release delete dev`
2. Run B: `gh release delete dev` (already gone — silently ignored with `|| true`)
3. Run B: `gh release create dev` ← publishes commit-B binaries
4. Run A: `gh release create dev` ← **FAILS** with "release already exists"
In scenario 4, `gh release create` exits non-zero and the step fails, causing the newer commit's release to persist — but subsequent CI runs will surface a noisy failure that needs to be retried manually.
Add a job-level `concurrency` key to cancel in-progress runs:
```yaml
agent-dev-binaries:
name: Agent Dev Binaries
needs: check
if: github.event_name == 'push' && !startsWith(github.ref, 'refs/tags/v')
runs-on: ubuntu-latest
concurrency:
group: dev-release
cancel-in-progress: true
```
With `cancel-in-progress: true`, only the latest push's job ever reaches the delete/create steps, making the operation effectively atomic.
How can I resolve this? If you propose a fix, please make it concise.
src/app/(dashboard)/fleet/page.tsx
Outdated
| if (node.agentVersion?.startsWith("dev-")) { | ||
| return { version: latestDevAgentVersion, checksums: devAgentChecksums, tag: "dev" }; | ||
| } | ||
| return { version: latestAgentVersion, checksums: agentChecksums, tag: `v${latestAgentVersion}` }; |
There was a problem hiding this comment.
Tag eagerly built with potentially-null version
latestAgentVersion is typed string | null (it's null when no stable release has been fetched yet). The template literal \v${latestAgentVersion}`is evaluated unconditionally, producing"vnull"when the version is absent. The downstream&&guard ongetNodeLatest(node).versionprevents the broken URL from being used in the current render logic, but this is a latent trap — any future code that reaches into.tagwithout checking.versionfirst will silently dispatch an update request to.../releases/download/vnull/vf-agent-linux-amd64`.
| return { version: latestAgentVersion, checksums: agentChecksums, tag: `v${latestAgentVersion}` }; | |
| return { version: latestAgentVersion, checksums: agentChecksums, tag: latestAgentVersion ? `v${latestAgentVersion}` : null }; |
Then narrow the type at the call site: latest.tag! → latest.tag ?? "" (or guard on it like version).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/fleet/page.tsx
Line: 79
Comment:
**Tag eagerly built with potentially-null version**
`latestAgentVersion` is typed `string | null` (it's `null` when no stable release has been fetched yet). The template literal `\`v${latestAgentVersion}\`` is evaluated unconditionally, producing `"vnull"` when the version is absent. The downstream `&&` guard on `getNodeLatest(node).version` prevents the broken URL from being used in the current render logic, but this is a latent trap — any future code that reaches into `.tag` without checking `.version` first will silently dispatch an update request to `.../releases/download/vnull/vf-agent-linux-amd64`.
```suggestion
return { version: latestAgentVersion, checksums: agentChecksums, tag: latestAgentVersion ? `v${latestAgentVersion}` : null };
```
Then narrow the type at the call site: `latest.tag!` → `latest.tag ?? ""` (or guard on it like `version`).
How can I resolve this? If you propose a fix, please make it concise.- Always update checkedAt when dev release fetch succeeds, even if dev-version.txt is missing, to prevent GitHub API rate limit exhaustion - Add concurrency group to agent-dev-binaries CI job to prevent race conditions on concurrent pushes to main - Guard stable tag construction against null version to avoid "vnull" URL
| if (needsCheck) { | ||
| const release = await fetchDevRelease(); | ||
| if (release) { | ||
| checkedAt = new Date(); | ||
| const versionString = await fetchDevVersionString(release); | ||
| if (versionString) { | ||
| latestVersion = versionString; | ||
| checksums = await fetchChecksums(release); | ||
| } | ||
| await prisma.systemSettings.upsert({ | ||
| where: { id: "singleton" }, | ||
| update: { | ||
| latestDevAgentRelease: latestVersion, | ||
| latestDevAgentReleaseCheckedAt: checkedAt, | ||
| ...(versionString ? { latestDevAgentChecksums: JSON.stringify(checksums) } : {}), | ||
| }, | ||
| create: { | ||
| id: "singleton", | ||
| latestDevAgentRelease: latestVersion, | ||
| latestDevAgentReleaseCheckedAt: checkedAt, | ||
| ...(versionString ? { latestDevAgentChecksums: JSON.stringify(checksums) } : {}), | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Outer release null case bypasses rate-limiting entirely
When fetchDevRelease() returns null (e.g., GitHub API is down, the dev release tag doesn't exist yet after the first push, or the request is rate-limited), the if (release) block is skipped, so latestDevAgentReleaseCheckedAt is never written to the database. lastChecked remains null or stale, and needsCheck evaluates to true on every subsequent call — causing unbounded GitHub API requests with no cooldown period.
This is distinct from the previously flagged versionString null case: when release is non-null but versionString is null, the upsert at line 226 still runs (updating latestDevAgentReleaseCheckedAt) because checkedAt = new Date() is set unconditionally inside if (release). The outer null case has no equivalent protection.
Consider updating checkedAt even when the release fetch fails, to enforce the 24-hour cooldown:
if (needsCheck) {
checkedAt = new Date(); // bump timestamp even on failed fetch
const release = await fetchDevRelease();
if (release) {
const versionString = await fetchDevVersionString(release);
if (versionString) {
latestVersion = versionString;
checksums = await fetchChecksums(release);
}
}
await prisma.systemSettings.upsert({
where: { id: "singleton" },
update: {
latestDevAgentRelease: latestVersion,
latestDevAgentReleaseCheckedAt: checkedAt,
...(checksums && Object.keys(checksums).length > 0
? { latestDevAgentChecksums: JSON.stringify(checksums) }
: {}),
},
create: { /* ... */ },
});
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/server/services/version-check.ts
Line: 217-240
Comment:
**Outer `release` null case bypasses rate-limiting entirely**
When `fetchDevRelease()` returns `null` (e.g., GitHub API is down, the `dev` release tag doesn't exist yet after the first push, or the request is rate-limited), the `if (release)` block is skipped, so `latestDevAgentReleaseCheckedAt` is never written to the database. `lastChecked` remains `null` or stale, and `needsCheck` evaluates to `true` on every subsequent call — causing unbounded GitHub API requests with no cooldown period.
This is distinct from the previously flagged `versionString` null case: when `release` is non-null but `versionString` is null, the upsert at line 226 still runs (updating `latestDevAgentReleaseCheckedAt`) because `checkedAt = new Date()` is set unconditionally inside `if (release)`. The outer null case has no equivalent protection.
Consider updating `checkedAt` even when the release fetch fails, to enforce the 24-hour cooldown:
```typescript
if (needsCheck) {
checkedAt = new Date(); // bump timestamp even on failed fetch
const release = await fetchDevRelease();
if (release) {
const versionString = await fetchDevVersionString(release);
if (versionString) {
latestVersion = versionString;
checksums = await fetchChecksums(release);
}
}
await prisma.systemSettings.upsert({
where: { id: "singleton" },
update: {
latestDevAgentRelease: latestVersion,
latestDevAgentReleaseCheckedAt: checkedAt,
...(checksums && Object.keys(checksums).length > 0
? { latestDevAgentChecksums: JSON.stringify(checksums) }
: {}),
},
create: { /* ... */ },
});
}
```
How can I resolve this? If you propose a fix, please make it concise.| --url) VF_URL="$2"; shift 2 ;; | ||
| --token) VF_TOKEN="$2"; shift 2 ;; | ||
| --version) VERSION="$2"; shift 2 ;; | ||
| --channel) CHANNEL="$2"; shift 2 ;; | ||
| --help) usage ;; | ||
| *) fatal "Unknown option: $1 (use --help for usage)" ;; | ||
| esac | ||
| done | ||
|
|
||
| if [ "${CHANNEL}" = "dev" ] && [ "${VERSION}" != "latest" ]; then | ||
| fatal "--channel dev and --version are mutually exclusive" | ||
| fi | ||
|
|
||
| # ───────────────────────────────────────────────── | ||
| # Preflight checks |
There was a problem hiding this comment.
Unknown --channel values silently fall through to stable
The --channel flag only rejects the combination of dev + --version, but does not validate that the channel value itself is one of the supported options (stable | dev). Passing --channel foo silently treats the install as a stable-channel install, which can confuse operators who mistype the channel name.
| --url) VF_URL="$2"; shift 2 ;; | |
| --token) VF_TOKEN="$2"; shift 2 ;; | |
| --version) VERSION="$2"; shift 2 ;; | |
| --channel) CHANNEL="$2"; shift 2 ;; | |
| --help) usage ;; | |
| *) fatal "Unknown option: $1 (use --help for usage)" ;; | |
| esac | |
| done | |
| if [ "${CHANNEL}" = "dev" ] && [ "${VERSION}" != "latest" ]; then | |
| fatal "--channel dev and --version are mutually exclusive" | |
| fi | |
| # ───────────────────────────────────────────────── | |
| # Preflight checks | |
| if [ "${CHANNEL}" = "dev" ] && [ "${VERSION}" != "latest" ]; then | |
| fatal "--channel dev and --version are mutually exclusive" | |
| fi | |
| if [ "${CHANNEL}" != "stable" ] && [ "${CHANNEL}" != "dev" ]; then | |
| fatal "Unknown channel '${CHANNEL}'. Valid values are: stable, dev" | |
| fi |
Prompt To Fix With AI
This is a comment left during a code review.
Path: agent/install.sh
Line: 68-82
Comment:
**Unknown `--channel` values silently fall through to stable**
The `--channel` flag only rejects the combination of `dev` + `--version`, but does not validate that the channel value itself is one of the supported options (`stable` | `dev`). Passing `--channel foo` silently treats the install as a stable-channel install, which can confuse operators who mistype the channel name.
```suggestion
if [ "${CHANNEL}" = "dev" ] && [ "${VERSION}" != "latest" ]; then
fatal "--channel dev and --version are mutually exclusive"
fi
if [ "${CHANNEL}" != "stable" ] && [ "${CHANNEL}" != "dev" ]; then
fatal "Unknown channel '${CHANNEL}'. Valid values are: stable, dev"
fi
```
How can I resolve this? If you propose a fix, please make it concise.
1 participant
Summary
--versionand--helpCLI flags to the agent binarydevpre-release on every push to main--channel devfor dev binary installsTest plan
vf-agent --versionprints version and exitsvf-agent --helpprints usage and exitsinstall.sh --channel devdownloads from dev pre-release