Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 39 additions & 8 deletions dashboard/src/components/StatesByRunId.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
'use client';

import React, { useState, useEffect, useMemo } from 'react';
import { formatDistanceToNow } from 'date-fns';
import { apiService } from '@/services/api';
import {
CurrentStatesResponse,
StatesByRunIdResponse,
StateListItem
StateListItem,
RunSummary
} from '@/types/state-manager';
import { GraphVisualization } from './GraphVisualization';
import {
Expand All @@ -25,7 +27,7 @@ interface StatesByRunIdProps {
namespace: string;
apiKey: string;
}

// my new function
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.

medium

This comment appears to be a temporary note. It's best to remove such comments from the final code to keep it clean.

export const StatesByRunId: React.FC<StatesByRunIdProps> = ({
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.

🧹 Nitpick (assertive)

Remove personal/commentary noise.

Drop “my new function” from production code.

-// my new function 
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const StatesByRunId: React.FC<StatesByRunIdProps> = ({
export const StatesByRunId: React.FC<StatesByRunIdProps> = ({
🤖 Prompt for AI Agents
In dashboard/src/components/StatesByRunId.tsx around line 31, there is a stray
personal/commentary phrase ("my new function") present in the production code;
remove that text so the component signature and surrounding code contain only
functional identifiers and comments relevant to the implementation, then run a
quick lint/type check to ensure no leftover stray tokens or formatting issues
remain.

namespace,
apiKey
Expand All @@ -43,6 +45,13 @@ export const StatesByRunId: React.FC<StatesByRunIdProps> = ({
return m;
}, [currentStates]);

const sortedRunIds = React.useMemo(() => {
if (!currentStates?.run_ids) return [];
return [...currentStates.run_ids].sort((a, b) =>
new Date(b.created_at).getTime() - new Date(a.created_at).getTime()
);
}, [currentStates?.run_ids]);
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.

medium

The dependency array for useMemo includes currentStates?.run_ids. While this might work, it's unconventional. The standard practice is to list state variables that the memoized value depends on. Using [currentStates] would be clearer and safer, ensuring the value is recomputed whenever the currentStates object is updated.

Suggested change
}, [currentStates?.run_ids]);
}, [currentStates]);


Comment on lines +48 to +54
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.

🧹 Nitpick (assertive)

Memoization is fine; guard against invalid dates.

If created_at can be null/invalid from the API, add a fallback to avoid NaN comparisons.

-  return [...currentStates.run_ids].sort((a, b) => 
-    new Date(b.created_at).getTime() - new Date(a.created_at).getTime()
-  );
+  return [...currentStates.run_ids].sort((a, b) => {
+    const tb = Date.parse(b.created_at ?? '') || 0;
+    const ta = Date.parse(a.created_at ?? '') || 0;
+    return tb - ta;
+  });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const sortedRunIds = React.useMemo(() => {
if (!currentStates?.run_ids) return [];
return [...currentStates.run_ids].sort((a, b) =>
new Date(b.created_at).getTime() - new Date(a.created_at).getTime()
);
}, [currentStates?.run_ids]);
const sortedRunIds = React.useMemo(() => {
if (!currentStates?.run_ids) return [];
return [...currentStates.run_ids].sort((a, b) => {
const tb = Date.parse(b.created_at ?? '') || 0;
const ta = Date.parse(a.created_at ?? '') || 0;
return tb - ta;
});
}, [currentStates?.run_ids]);
🤖 Prompt for AI Agents
In dashboard/src/components/StatesByRunId.tsx around lines 48 to 54, the sort
callback assumes created_at is always a valid date which can yield NaN and
unpredictable ordering; update the comparator to defensively parse created_at
for each run_id, fall back to a safe numeric timestamp (e.g., 0) when new
Date(created_at).getTime() is NaN or created_at is null/undefined, and use those
safe timestamps for subtraction so invalid dates consistently sort to the end.

const loadCurrentStates = async () => {
setIsLoading(true);
setError(null);
Expand All @@ -53,7 +62,7 @@ export const StatesByRunId: React.FC<StatesByRunIdProps> = ({

// Auto-select the first run ID if available
if (data.run_ids.length > 0 && !selectedRunId) {
setSelectedRunId(data.run_ids[0]);
setSelectedRunId(data.run_ids[0].run_id);
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.

high

This line selects the first run ID from the API response (data.run_ids[0]), which might not be the latest one. To ensure the latest run ID is selected by default, you should sort the run_ids by created_at in descending order before picking the first one.

Suggested change
setSelectedRunId(data.run_ids[0].run_id);
setSelectedRunId([...data.run_ids].sort((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime())[0].run_id);

}
Comment on lines 64 to 66
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.

⚠️ Potential issue

Auto-select the latest run, not the first array element.

Current code assumes server returns latest first. Sort locally to pick the newest.

-      // Auto-select the first run ID if available
-      if (data.run_ids.length > 0 && !selectedRunId) {
-        setSelectedRunId(data.run_ids[0].run_id);
-      }
+      // Auto-select the latest run ID if available
+      if (data.run_ids.length > 0 && !selectedRunId) {
+        const latest = [...data.run_ids].sort(
+          (a, b) => Date.parse(b.created_at) - Date.parse(a.created_at)
+        )[0];
+        setSelectedRunId(latest.run_id);
+      }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (data.run_ids.length > 0 && !selectedRunId) {
setSelectedRunId(data.run_ids[0]);
setSelectedRunId(data.run_ids[0].run_id);
}
// Auto-select the latest run ID if available
if (data.run_ids.length > 0 && !selectedRunId) {
const latest = [...data.run_ids].sort(
(a, b) => Date.parse(b.created_at) - Date.parse(a.created_at)
)[0];
setSelectedRunId(latest.run_id);
}
🤖 Prompt for AI Agents
In dashboard/src/components/StatesByRunId.tsx around lines 64-66, the code picks
the first element of data.run_ids assuming the server returns newest first;
instead, sort a copy of data.run_ids locally by the run timestamp (e.g.,
created_at/started_at) in descending order — if no timestamp exists, fall back
to a deterministic comparison of run_id — then pick the first element of the
sorted array and call setSelectedRunId; ensure you don't mutate the original
array and handle the empty-array guard.

} catch (err) {
setError(err instanceof Error ? err.message : 'Failed to load current states');
Expand Down Expand Up @@ -177,6 +186,28 @@ export const StatesByRunId: React.FC<StatesByRunIdProps> = ({
</button>
</div>
</div>
{/* my react component for latest run id's */}
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.

medium

This comment appears to be a temporary note. It should be removed to maintain code cleanliness.

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.

🧹 Nitpick (assertive)

Comment tone.

“my react component for latest run id's” → concise, neutral comment or remove. Also fix plural/possessive: “IDs”.

-{/* my react component for latest run id's */}
+{/* Latest Run IDs */}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{/* my react component for latest run id's */}
{/* Latest Run IDs */}
🤖 Prompt for AI Agents
In dashboard/src/components/StatesByRunId.tsx around line 189, the inline JSX
comment reads “my react component for latest run id's”; change this to a
concise, neutral comment (e.g. "Component showing latest run IDs") or remove it
entirely, and correct the possessive/plural error by using "IDs" (capital I, no
apostrophe).

<div className="mt-4">
<h3 className="text-lg font-semibold mb-2">Latest Run ID</h3>
<div className="max-h-60 overflow-y-auto border rounded-lg p-2">
{sortedRunIds.length > 0 ? (
<ul className="divide-y divide-gray-200">
{sortedRunIds.map((run) => (
<li key={run.run_id} className="py-2">
<div className="flex justify-between items-center">
<span className="font-mono text-sm">{run.run_id}</span>
<span className="text-xs text-gray-500">
{formatDistanceToNow(new Date(run.created_at), { addSuffix: true })}
</span>
</div>
</li>
))}
</ul>
) : (
<p className="text-sm text-gray-500">No run IDs found</p>
)}
</div>
</div>
Comment on lines +190 to +210
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.

🧹 Nitpick (assertive)

Make “Latest Run ID” list actionable.

Let users click a recent item to select it; improves UX.

-        {sortedRunIds.map((run) => (
-          <li key={run.run_id} className="py-2">
+        {sortedRunIds.map((run) => (
+          <li
+            key={run.run_id}
+            className="py-2 cursor-pointer hover:bg-gray-50 rounded"
+            onClick={() => setSelectedRunId(run.run_id)}
+          >

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In dashboard/src/components/StatesByRunId.tsx around lines 190 to 210, the
"Latest Run ID" list is static; make each list item actionable by wiring a
click/select handler: add an onSelectRun (prop or local state setter) and attach
it to each list item (use a button or clickable div) so clicking a run calls
onSelectRun(run.run_id). Also ensure keyboard accessibility (onKeyDown handling
Enter/Space), update UI to indicate the active/selected run (apply a selected
CSS class), and prevent event propagation if needed so selection triggers
without side effects.


{/* Run ID Selector */}
{currentStates && currentStates.run_ids.length > 0 && (
Expand All @@ -189,19 +220,19 @@ export const StatesByRunId: React.FC<StatesByRunIdProps> = ({
<div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-4">
{currentStates.run_ids.map((runId) => (
<button
key={runId}
onClick={() => setSelectedRunId(runId)}
key={runId.run_id}
onClick={() => setSelectedRunId(runId.run_id)}
className={`p-4 rounded-lg border-2 transition-colors ${
selectedRunId === runId
selectedRunId === runId.run_id
? 'border-[#031035] bg-[#031035]/5'
: 'border-gray-200 hover:border-gray-300'
}`}
>
<div className="text-sm font-medium text-gray-900 truncate">
{runId}
{runId.run_id}
</div>
<div className="text-xs text-gray-500 mt-1">
{countsByRunId.get(runId) ?? 0} states
{countsByRunId.get(runId.run_id) ?? 0} states
</div>
</button>
))}
Expand Down
46 changes: 27 additions & 19 deletions dashboard/src/services/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,34 @@ import {
} from '@/types/state-manager';

const API_BASE_URL = process.env.NEXT_PUBLIC_EXOSPHERE_STATE_MANAGER_URL || 'http://localhost:8000';

const DEFAULT_API_KEY = process.env.NEXT_PUBLIC_EXOSPHERE_API_KEY || '';
class ApiService {
Comment on lines 20 to 22
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.

🧹 Nitpick (assertive)

Optional: remove DEFAULT_API_KEY duplication.

You use DEFAULT_API_KEY in methods but read env again in makeRequest. Rely on a single source (the constant) to avoid drift.

-const DEFAULT_API_KEY = process.env.NEXT_PUBLIC_EXOSPHERE_API_KEY || '';
+const DEFAULT_API_KEY = process.env.NEXT_PUBLIC_EXOSPHERE_API_KEY ?? '';

And in makeRequest, use only DEFAULT_API_KEY (as shown above).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const API_BASE_URL = process.env.NEXT_PUBLIC_EXOSPHERE_STATE_MANAGER_URL || 'http://localhost:8000';
const DEFAULT_API_KEY = process.env.NEXT_PUBLIC_EXOSPHERE_API_KEY || '';
class ApiService {
const API_BASE_URL = process.env.NEXT_PUBLIC_EXOSPHERE_STATE_MANAGER_URL || 'http://localhost:8000';
const DEFAULT_API_KEY = process.env.NEXT_PUBLIC_EXOSPHERE_API_KEY ?? '';
class ApiService {
🤖 Prompt for AI Agents
In dashboard/src/services/api.ts around lines 20 to 22, the code reads the API
key from process.env in multiple places which duplicates DEFAULT_API_KEY; remove
the duplicate env read inside makeRequest and any other methods and reference
the single DEFAULT_API_KEY constant defined at the top instead, ensuring all
calls use that constant so there’s a single source of truth for the API key.

private async makeRequest<T>(
endpoint: string,
options: RequestInit = {}
): Promise<T> {
const url = `${API_BASE_URL}${endpoint}`;

const headers = new Headers(options.headers);
headers.set('Content-Type', 'application/json');
headers.set('x-api-key', process.env.NEXT_PUBLIC_EXOSPHERE_API_KEY || '');

const response = await fetch(url, {
headers: {
'Content-Type': 'application/json',
...options.headers,
},
...options,
headers,
});
Comment on lines +29 to 36
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.

⚠️ Potential issue

Bug: env key overrides provided apiKey; header case inconsistencies.

makeRequest unconditionally sets x-api-key from env, overwriting any caller-provided header (and per-method headers use X-API-Key). This breaks per-user apiKey flows and duplicates config.

Fix: only set a default if not already provided; standardize on x-api-key.

-    const headers = new Headers(options.headers);
-    headers.set('Content-Type', 'application/json');
-    headers.set('x-api-key', process.env.NEXT_PUBLIC_EXOSPHERE_API_KEY || '');
+    const headers = new Headers(options.headers);
+    headers.set('Content-Type', 'application/json');
+    if (!headers.has('x-api-key') && DEFAULT_API_KEY) {
+      headers.set('x-api-key', DEFAULT_API_KEY);
+    }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In dashboard/src/services/api.ts around lines 29-36, the code unconditionally
sets the x-api-key header from the environment (overwriting any caller-provided
header) and there's inconsistent header casing; change the logic to first detect
if an API key header already exists (case-insensitive check for
'x-api-key'/'X-API-Key') and only set the environment default when none is
provided, and standardize the header name to 'x-api-key' for both reads and
writes so callers' apiKey values are preserved and casing is consistent.


if (!response.ok) {
throw new Error(`API request failed: ${response.status} ${response.statusText}`);
const errorData = await response.json().catch(() => ({}));
console.error('API Error:', {
status: response.status,
statusText: response.statusText,
url,
errorData
});
throw new Error(errorData.detail || `API request failed: ${response.statusText}`);
}
Comment on lines 38 to 47
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.

🧹 Nitpick (assertive)

Error body may be non-JSON.

Good JSON fallback. Consider capturing response.text() if JSON parse fails to improve diagnostics.

-      const errorData = await response.json().catch(() => ({}));
+      let errorData: any = {};
+      try { errorData = await response.json(); } catch {
+        try { errorData = { detail: await response.text() }; } catch {}
+      }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In dashboard/src/services/api.ts around lines 38 to 47, the error handling
assumes response.json() will succeed and only falls back to an empty object on
parse failure; update it to attempt response.text() when JSON parsing fails and
include that rawText (and parsed JSON if available) in the console.error payload
and in the thrown Error message. Specifically: try parsing JSON, if that throws
call response.text(), set errorData to the parsed object when possible and
rawBody to the text, log both status/statusText/url/errorData/rawBody, and throw
an Error that uses errorData.detail || rawBody || a generic message so non-JSON
responses are surfaced for diagnostics.


return response.json();
}

Expand All @@ -52,7 +60,7 @@ class ApiService {
{
method: 'PUT',
headers: {
'X-API-Key': apiKey,
'X-API-Key': DEFAULT_API_KEY,
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.

high

This change to use DEFAULT_API_KEY makes the apiKey parameter of the registerNodes function redundant. This pattern is repeated across most methods in ApiService, but not all, creating inconsistencies and dead code.

I recommend the following improvements for maintainability:

  • Remove unused parameters: Remove the apiKey parameter from the signatures of all functions where it's no longer used (e.g., registerNodes, getCurrentStates, etc.) and update their call sites.
  • Ensure consistency: The upsertGraphTemplate method still uses the apiKey parameter. It should be updated to use DEFAULT_API_KEY to be consistent with other methods.
  • Avoid redundancy: The makeRequest function now sets the x-api-key header. To avoid duplication, you can remove the X-API-Key header from the options passed by each method.

},
Comment on lines +63 to 64
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.

🛠️ Refactor suggestion

Use caller apiKey (fallback to default) consistently.

Apply across all methods for consistency and to honor the passed apiKey.

-          'X-API-Key': DEFAULT_API_KEY,
+          'x-api-key': apiKey || DEFAULT_API_KEY,

Make the same change in every method setting the header (registerNodes, getGraphTemplate, createStates, enqueueStates, executeState, getSecrets, listRegisteredNodes, listGraphTemplates, getCurrentStates, getStatesByRunId, getGraphStructure). After this, makeRequest remains a safe default when a method doesn’t pass a header.

Also applies to: 99-100, 117-118, 134-135, 152-153, 169-170, 185-186, 200-201, 216-217, 232-233, 248-249

body: JSON.stringify(request),
}
Expand Down Expand Up @@ -88,7 +96,7 @@ class ApiService {
{
method: 'GET',
headers: {
'X-API-Key': apiKey,
'X-API-Key': DEFAULT_API_KEY,
},
}
);
Expand All @@ -106,7 +114,7 @@ class ApiService {
{
method: 'POST',
headers: {
'X-API-Key': apiKey,
'X-API-Key': DEFAULT_API_KEY,
},
body: JSON.stringify(request),
}
Expand All @@ -123,7 +131,7 @@ class ApiService {
{
method: 'POST',
headers: {
'X-API-Key': apiKey,
'X-API-Key': DEFAULT_API_KEY,
},
body: JSON.stringify(request),
}
Expand All @@ -141,7 +149,7 @@ class ApiService {
{
method: 'POST',
headers: {
'X-API-Key': apiKey,
'X-API-Key': DEFAULT_API_KEY,
},
body: JSON.stringify(request),
}
Expand All @@ -158,7 +166,7 @@ class ApiService {
{
method: 'GET',
headers: {
'X-API-Key': apiKey,
'X-API-Key': DEFAULT_API_KEY,
},
}
);
Expand All @@ -174,7 +182,7 @@ class ApiService {
{
method: 'GET',
headers: {
'X-API-Key': apiKey,
'X-API-Key': DEFAULT_API_KEY,
},
}
);
Expand All @@ -189,7 +197,7 @@ class ApiService {
{
method: 'GET',
headers: {
'X-API-Key': apiKey,
'X-API-Key': DEFAULT_API_KEY,
},
}
);
Expand All @@ -205,7 +213,7 @@ class ApiService {
{
method: 'GET',
headers: {
'X-API-Key': apiKey,
'X-API-Key': DEFAULT_API_KEY,
},
}
);
Expand All @@ -221,7 +229,7 @@ class ApiService {
{
method: 'GET',
headers: {
'X-API-Key': apiKey,
'X-API-Key': DEFAULT_API_KEY,
},
}
);
Expand All @@ -237,7 +245,7 @@ class ApiService {
{
method: 'GET',
headers: {
'X-API-Key': apiKey,
'X-API-Key': DEFAULT_API_KEY,
},
}
);
Expand Down
6 changes: 5 additions & 1 deletion dashboard/src/types/state-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,15 @@ export interface StatesByRunIdResponse {
states: StateListItem[];
}

export interface RunSummary {
run_id: string;
created_at: string;
}
export interface CurrentStatesResponse {
namespace: string;
count: number;
states: StateListItem[];
run_ids: string[];
run_ids: RunSummary[];
}

export interface WorkflowStep {
Expand Down
24 changes: 22 additions & 2 deletions state-manager/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,14 +1,34 @@
FROM python:3.12-slim-bookworm
COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/

# Install curl for downloading uv
RUN apt-get update && apt-get install -y curl ca-certificates \
&& rm -rf /var/lib/apt/lists/*
Comment on lines +4 to +5
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.

🛠️ Refactor suggestion

Harden apt install.

Use --no-install-recommends and set set -eux for reliability.

-RUN apt-get update && apt-get install -y curl ca-certificates \
-    && rm -rf /var/lib/apt/lists/*
+RUN set -eux; \
+    apt-get update; \
+    apt-get install -y --no-install-recommends curl ca-certificates; \
+    rm -rf /var/lib/apt/lists/*
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN apt-get update && apt-get install -y curl ca-certificates \
&& rm -rf /var/lib/apt/lists/*
RUN set -eux; \
apt-get update; \
apt-get install -y --no-install-recommends curl ca-certificates; \
rm -rf /var/lib/apt/lists/*
🧰 Tools
🪛 Hadolint (2.12.0)

[warning] 4-4: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>

(DL3008)


[info] 4-4: Avoid additional packages by specifying --no-install-recommends

(DL3015)

🤖 Prompt for AI Agents
In state-manager/Dockerfile around lines 4 to 5, the apt install step should be
hardened: update the RUN to enable strict shell flags (e.g., start with set
-eux) and install packages with apt-get install -y --no-install-recommends to
avoid unnecessary recommended packages; keep cleaning apt caches (rm -rf
/var/lib/apt/lists/*) and ensure update and install are combined in the same RUN
so failures are caught and the step is reliable.


# Download uv binary based on target architecture
ARG TARGETARCH
RUN case "$TARGETARCH" in \
amd64) ARCH=x86_64 ;; \
arm64) ARCH=aarch64 ;; \
*) echo "Unsupported architecture: $TARGETARCH" && exit 1 ;; \
esac && \
curl -L "https://github.com/astral-sh/uv/releases/latest/download/uv-${ARCH}-unknown-linux-musl.tar.gz" \
| tar -xz && \
mv uv-${ARCH}-unknown-linux-musl/uv /usr/local/bin/ && \
mv uv-${ARCH}-unknown-linux-musl/uvx /usr/local/bin/ && \
rm -rf uv-${ARCH}-unknown-linux-musl
Comment on lines +9 to +18
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.

🧹 Nitpick (assertive)

🛠️ Refactor suggestion

Ensure pipefail and tighten the download/extract step.

Without pipefail, a failed curl can still pass the build. Also quote variables.

-ARG TARGETARCH
-RUN case "$TARGETARCH" in \
+ARG TARGETARCH
+SHELL ["/bin/bash", "-o", "pipefail", "-c"]
+RUN set -eux; case "${TARGETARCH}" in \
       amd64)  ARCH=x86_64 ;; \
       arm64)  ARCH=aarch64 ;; \
       *) echo "Unsupported architecture: $TARGETARCH" && exit 1 ;; \
     esac && \
-    curl -L "https://github.com/astral-sh/uv/releases/latest/download/uv-${ARCH}-unknown-linux-musl.tar.gz" \
-    | tar -xz && \
-    mv uv-${ARCH}-unknown-linux-musl/uv /usr/local/bin/ && \
-    mv uv-${ARCH}-unknown-linux-musl/uvx /usr/local/bin/ && \
-    rm -rf uv-${ARCH}-unknown-linux-musl
+    curl -fsSL "https://github.com/astral-sh/uv/releases/latest/download/uv-${ARCH}-unknown-linux-musl.tar.gz" \
+    | tar -xz && \
+    install -m 0755 "uv-${ARCH}-unknown-linux-musl/uv" /usr/local/bin/uv && \
+    install -m 0755 "uv-${ARCH}-unknown-linux-musl/uvx" /usr/local/bin/uvx && \
+    rm -rf "uv-${ARCH}-unknown-linux-musl"

Optionally verify checksum/signature of the downloaded tarball for supply-chain safety.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN case "$TARGETARCH" in \
amd64) ARCH=x86_64 ;; \
arm64) ARCH=aarch64 ;; \
*) echo "Unsupported architecture: $TARGETARCH" && exit 1 ;; \
esac && \
curl -L "https://github.com/astral-sh/uv/releases/latest/download/uv-${ARCH}-unknown-linux-musl.tar.gz" \
| tar -xz && \
mv uv-${ARCH}-unknown-linux-musl/uv /usr/local/bin/ && \
mv uv-${ARCH}-unknown-linux-musl/uvx /usr/local/bin/ && \
rm -rf uv-${ARCH}-unknown-linux-musl
ARG TARGETARCH
SHELL ["/bin/bash", "-o", "pipefail", "-c"]
RUN set -eux; case "${TARGETARCH}" in \
amd64) ARCH=x86_64 ;; \
arm64) ARCH=aarch64 ;; \
*) echo "Unsupported architecture: $TARGETARCH" && exit 1 ;; \
esac && \
curl -fsSL "https://github.com/astral-sh/uv/releases/latest/download/uv-${ARCH}-unknown-linux-musl.tar.gz" \
| tar -xz && \
install -m 0755 "uv-${ARCH}-unknown-linux-musl/uv" /usr/local/bin/uv && \
install -m 0755 "uv-${ARCH}-unknown-linux-musl/uvx" /usr/local/bin/uvx && \
rm -rf "uv-${ARCH}-unknown-linux-musl"
🧰 Tools
🪛 Hadolint (2.12.0)

[info] 9-9: Double quote to prevent globbing and word splitting.

(SC2086)


[warning] 9-9: Set the SHELL option -o pipefail before RUN with a pipe in it. If you are using /bin/sh in an alpine image or if your shell is symlinked to busybox then consider explicitly setting your SHELL to /bin/ash, or disable this check

(DL4006)

🤖 Prompt for AI Agents
In state-manager/Dockerfile around lines 9 to 18, the download-and-extract step
is brittle: it lacks pipefail (so curl failures can be masked), doesn’t quote
variables, and extracts in a subshell without tighter controls; update the RUN
line to enable set -euo pipefail, use curl with --fail and -S/L, quote $ARCH and
other vars, stream the tar to tar -xzf - into a temporary directory (or use
--directory) and move files from that directory, quote paths when
moving/removing, and (optionally) add checksum or signature verification of the
downloaded tarball before extraction for supply-chain safety.


WORKDIR /api-server

# Copy dependency files first (for caching)
COPY pyproject.toml uv.lock ./

# Install dependencies using uv
RUN uv sync --locked
Comment on lines +22 to 26
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.

🧹 Nitpick (assertive)

Consider deterministic, offline-friendly sync.

If CI relies on network, uv sync --locked is fine. If you need hermetic builds, add UV_INDEX_STRATEGY=offline and vendor the lock/requirements.

🤖 Prompt for AI Agents
In state-manager/Dockerfile around lines 22 to 26, the uv sync step is
network-dependent; make the build hermetic by setting UV_INDEX_STRATEGY=offline
before running uv and ensuring vendored artifacts are copied into the image. Add
a Dockerfile line to set ENV UV_INDEX_STRATEGY=offline (or prefix the RUN with
it), copy the vendor directory or pre-generated wheels/requirements into the
image alongside pyproject.toml and uv.lock, then run uv sync --locked so the
sync uses the local index; also update CI to produce and provide the vendor
bundle if not already present.


# Copy app source code
COPY . .

EXPOSE 8000

CMD ["uv", "run", "run.py", "--mode", "production", "--workers", "4"]
# Start the app
CMD ["uv", "run", "run.py", "--mode", "production", "--workers", "4"]
Loading