Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api/v1/api.yaml`:
- Around line 8213-8215: Remove the archiveDir field from the public API schema
(the archiveDir property in api.yaml) so the coordinator/worker filesystem path
is not exposed to clients; instead add a server-side capability flag (e.g.,
artifactsAvailable or artifactsEnabled) to the same DAG-run response object to
indicate whether artifacts exist or are supported, and ensure any code that
serializes/deserializes archiveDir (responses, examples, mocks) is updated to
stop emitting it while server storage and the run-scoped artifact endpoints
remain unchanged.
- Around line 2249-2259: The current GET /dag-runs/{name}/{dagRunId}/artifacts
endpoint forces full recursive expansion of directory children causing expensive
full-tree walks; add an optional query parameter (e.g., ?recursive=true or
?depth=N or ?shallow=true) to api.yaml for operationId getDAGRunArtifacts (and
the other affected artifacts-list operation) and update the handler in
internal/service/frontend/api/v1/dagruns.go (the listing logic around the
current children expansion in the function handling getDAGRunArtifacts and the
similar block at the other range) to honor the new parameter: perform a shallow
single-directory listing by default (no recursive children expansion), support
depth-limited or explicit recursive=true behavior only when requested, and
ensure response schema/documentation reflects the new param and behavior to
avoid large payloads and unnecessary filesystem walks.

In `@internal/cmd/enqueue.go`:
- Around line 100-103: Move the ctx.GenArtifactDir(dag, dagRunID) call so it
runs after the duplicate-run validation block that checks whether the dagRunID
already exists; currently artifactDir is created before that validation and can
create/touch paths for runs that will be rejected. Specifically, remove the
GenArtifactDir invocation from before the duplicate-run check and reinsert it
immediately after the validation succeeds (keeping the same error handling: wrap
and return fmt.Errorf("failed to generate artifact directory: %w", err) on
failure), referencing ctx.GenArtifactDir, dag and dagRunID to locate the call to
move.

In `@internal/cmn/fileutil/safe_path.go`:
- Around line 77-79: The pathWithinBase function fails for root bases because
base+string(filepath.Separator) becomes "//"; to fix, normalize inputs with
filepath.Clean and handle the root case explicitly: in pathWithinBase(path,
base) call filepath.Clean on both path and base, then return true if cleanedPath
== cleanedBase or (cleanedBase == string(filepath.Separator) &&
strings.HasPrefix(cleanedPath, cleanedBase)) or strings.HasPrefix(cleanedPath,
cleanedBase+string(filepath.Separator)); update only the pathWithinBase function
accordingly.

In `@internal/persis/filedagrun/dagrun.go`:
- Around line 308-317: The cleanup loop currently calls os.RemoveAll on entries
from uniqueArtifactDirs without validating the paths; restrict deletions to a
trusted artifact root by resolving and cleaning each dir (use filepath.Abs +
filepath.Clean), compute rel := filepath.Rel(trustedRoot, dir) and only call
os.RemoveAll if rel does not start with ".." (i.e., dir is inside trustedRoot);
log and skip otherwise; update the same validation when processing persisted
ArchiveDir entries (the block handling ArchiveDir around lines 409-440) and
ensure parentDirs population only happens for validated, accepted dirs.

In `@internal/runtime/agent/agent.go`:
- Around line 851-867: The artifact finalization is using the cancelable run
context (ctx) which allows aborted runs to skip uploads; change the Finalize
call to use a detached context (like context.WithoutCancel(ctx) or a
background/detached context) so artifactFinalizer.Finalize(ctx,
finishedStatus.AttemptID, a.artifactDir) becomes
artifactFinalizer.Finalize(detachedCtx, finishedStatus.AttemptID,
a.artifactDir); update the call site in the block that references
a.artifactFinalizer, a.artifactDir, finishedStatus and lastErr and ensure the
detached context variable is created just before the Finalize call.

In `@internal/service/coordinator/artifact_handler.go`:
- Around line 38-85: When the streaming loop in handleStream exits early (on
io.EOF or any stream.Recv error), ensure any open buffered writers in h.writers
are closed to avoid stale state: before returning on io.EOF or on a non-nil recv
error, iterate over the entries in the h.writers map and call h.closeWriter(ctx,
<corresponding chunk/identifier>) or add and call a helper like
closeAllWriters(ctx) that closes each writer; update handleStream to invoke this
cleanup path prior to returning so getOrCreateWriter and future writes won't
reuse orphaned writers.
- Around line 96-109: The cached writer returned by getOrCreateWriter can be
stale because the code returns an existing writer before revalidating the
current attempt; ensure you revalidate the attempt before reusing a cached
writer by calling the same check used for new writers (e.g., call
validateCurrentAttempt which calls archiveDir(ctx, chunk) or directly call
h.archiveDir(ctx, chunk)) immediately after acquiring the writer from h.writers
and before returning it; if the validation fails, treat it as a miss and proceed
with artifactFilePath and writer creation as originally implemented.

In `@internal/service/coordinator/handler.go`:
- Around line 1477-1485: The code expands the artifact base directory into
baseDir (from h.artifactDir or dag.Artifacts.Dir) using eval.String but does not
reject an empty result; after env expansion an empty baseDir will make
filepath.Join produce a relative path and MkdirAll write to the process CWD.
After the eval.String call that sets baseDir (and checks err), add a validation
that baseDir is not empty (and optionally trim whitespace) and return a
descriptive error (e.g., "artifact directory is empty after expansion") if it
is; reference the variables and calls baseDir, dag.Artifacts.Dir, h.artifactDir,
and the eval.String expansion so the check occurs before any use in
filepath.Join/MkdirAll.
- Around line 1459-1464: The current early return checks "h.artifactDir == \"\"
|| incoming == nil || incoming.ArchiveDir == \"\"" which prevents reusing
latestStatus.ArchiveDir and allows a later empty incoming.ArchiveDir to clear
the persisted path; change the guard to only return when h.artifactDir == "" or
incoming == nil, then if latestStatus != nil && latestStatus.ArchiveDir != ""
copy latestStatus.ArchiveDir into incoming.ArchiveDir before any write/clear
logic so that missing ArchiveDir in subsequent updates preserves the previously
resolved ArchiveDir (refer to symbols h.artifactDir, incoming, latestStatus,
ArchiveDir).

In `@internal/service/frontend/api/v1/dagruns.go`:
- Around line 811-818: The code incorrectly maps every error from
getDAGRunArtifactStatus()/getSubDAGRunArtifactStatus() to a 404; change the
error handling in GetDAGRunArtifacts (and the similar blocks at the other
ranges) to distinguish "not found" from other failures: detect the actual
not-found sentinel (e.g. errors.Is(err, os.ErrNotExist) or the package-specific
sentinel returned by those helpers) and only return the 404/NotFound response
for that case, otherwise return an internal/server-error response (appropriate
api.*500JSONResponse or api.ErrorCodeInternal) including the original error
message for debugging; update the branches in GetDAGRunArtifacts, the
preview/download handlers, and other occurrences you noted to use this pattern
with getDAGRunArtifactStatus and getSubDAGRunArtifactStatus.

In `@internal/service/worker/remote_handler_test.go`:
- Around line 1883-1892: The test uses an unprotected reported
[]exec.DAGRunStatus which is appended to inside the mock client's
ReportStatusFunc and can race with executeDAGRun; fix by adding a sync.Mutex (or
sync.RWMutex) near the reported declaration and lock it around all
accesses/modifications to reported inside ReportStatusFunc and when later
reading the slice for assertions (referencing reported, ReportStatusFunc,
newMockRemoteCoordinatorClient, convert.ProtoToDAGRunStatus, require.NoError),
and apply the same mutex-guarding change to the other occurrence mentioned (the
block around lines 1942-1951).

In `@ui/src/api/v1/schema.ts`:
- Around line 3322-3328: The public schema currently exposes node-local
filesystem paths (DAGArtifactsConfig.dir and archiveDir) which leak host layout
and are useless to browsers; remove these fields from the browser-visible types
and replace them with a capability flag (e.g., hasArtifacts or
artifactsAvailable) on DAGArtifactsConfig and any related run artifact types,
keep the actual path values strictly server-side and only return them via
protected list/preview/download endpoints, and update any serializers/DTOs and
clients that used dir or archiveDir to instead call the server endpoints for
artifact operations.

In `@ui/src/features/dags/components/artifacts/ArtifactsTab.tsx`:
- Around line 1-20: This file (ArtifactsTab.tsx) is missing the repository's GPL
v3 license header; add the standard GPL v3 banner to the top of the file (above
all imports) matching the project's header template and license text, then run
the repository tool to ensure consistency (e.g., run make addlicense) so the
header is applied exactly like other source files.
- Around line 255-278: The fetchTree async flow only handles a resolved response
shape and can leave treeLoading true if requestArtifactTree() throws; wrap the
await requestArtifactTree() call and its subsequent handling in a try/catch
inside fetchTree, and in the catch ensure you call setTreeLoading(false),
setTree([]), and setTreeError(error.message || 'Failed to load artifacts');
apply the same pattern to the preview-fetching logic (the previewLoading state
handling around the request used in the preview path around lines 311-325) so
any thrown rejections clear loading state and set an error.
- Around line 280-292: When refreshing the tree we currently preserve
selectedPath if it still exists, but that prevents the preview effect from
re-running and leaves stale content; update the selection logic around
findFirstFile/ setSelectedPath so that if you return the existing current path
(i.e., keep the selection because nextNodes.some(...) is true) you also clear
the preview (call setPreview(null)) to force the preview effect to reload,
otherwise set the selection to firstFile.path as before; reference
setSelectedPath, setPreview, findFirstFile, nextNodes and the
selectedPath/current callback when making the change.

---

Outside diff comments:
In `@internal/runtime/agent/agent.go`:
- Around line 800-805: This goroutine can write a terminal status too early;
change the logic inside execWithRecovery so after the sleep you 1) obtain status
via a.Status(ctx), 2) re-check a.finished.Load(), and 3) consult
a.shouldDelayTerminalStatus(attempt, status) and skip calling a.writeStatus(ctx,
attempt, status) when that returns true (i.e., only write if not finished and
not delayed); use the existing symbols execWithRecovery, a.finished, a.Status,
a.shouldDelayTerminalStatus, and a.writeStatus to locate and modify the code.

---

Nitpick comments:
In `@api/v1/api.yaml`:
- Around line 6751-6758: The ArtifactPath query parameter currently allows
absolute paths and `..` segments; update the ArtifactPath schema to enforce a
safe relative-path constraint by adding a regex pattern that disallows a leading
slash and any `..` path segments (for example a negative lookahead that forbids
leading '/' and any '/..' or '..' segments), keeping type: string and minLength:
1; reference ArtifactPath in the spec and ensure the description stays, so
generated clients/docs and validation will reject absolute paths and
parent-directory traversals early.

In `@internal/cmd/context.go`:
- Around line 947-950: GenArtifactDir is being called with its error ignored;
change the call to capture the error (e.g., artifactDir, err :=
c.GenArtifactDir(dag, dagRunID)), and if err != nil log the error (using the
existing logger in this package) so path/config issues are visible, then
continue to call statusBuilder.Create(dagRunID, core.Failed, 0, time.Now(),
transform.WithLogFilePath(logPath), transform.WithArchiveDir(artifactDir))
without turning the status write into a blocking failure; ensure you reference
the artifactDir and err variables and use the same log facility used elsewhere
in this file to emit the error.

In `@internal/cmd/local_execution.go`:
- Around line 109-115: GenLogFileName and GenArtifactDir are currently called
with their errors discarded; capture their returned errors, log them (including
error details and context: dag, dagRunID) before building the opts slice, and
still pass the paths (possibly empty) to transform.WithLogFilePath and
transform.WithArchiveDir so the failure-recording flow continues; specifically
update the call sites around ctx.GenLogFileName(dag, dagRunID) and
ctx.GenArtifactDir(dag, dagRunID) to check the err, log via the existing logger
(or ctx logger) with clear messages, and then construct opts with
transform.WithLogFilePath(logPath) and transform.WithArchiveDir(artifactDir) as
before.

In `@internal/cmn/fileutil/safe_path_test.go`:
- Around line 30-60: Modify the two tests
TestResolveExistingPathWithinBaseRejectsSymlinkEscape and
TestResolveExistingPathWithinBaseAllowsSafeSymlink to skip rather than fail when
the platform cannot create symlinks: after the os.Symlink call (or immediately
before attempting it) check the returned error and call t.Skipf("skipping
symlink test: %v", err) when symlink creation fails (e.g., permission denied or
not supported), so tests only assert behavior when os.Symlink succeeds.

In `@internal/service/coordinator/artifact_handler.go`:
- Around line 99-125: The current writersMu lock around artifactFilePath,
os.MkdirAll and fileutil.OpenOrCreateFile serializes unrelated disk/DB work;
change to a double-checked locking pattern: under h.writersMu check
h.writers[key] and return if present, otherwise release the lock and perform
artifactFilePath, MkdirAll and OpenOrCreateFile, then re-acquire h.writersMu,
re-check h.writers[key] (in case another goroutine created it), and only then
set h.writers[key] = w and return; reference functions/vars: writersMu,
h.writers, artifactFilePath, fileutil.OpenOrCreateFile, logWriter to locate and
adjust the code.

In `@internal/service/coordinator/handler_test.go`:
- Around line 327-352: Add a new test variant of
TestTransformArtifactPathsCreatesDirectory that uses a DAG name containing
separators/traversal tokens (e.g., "../weird/..-dag--name") to assert
transformArtifactPaths sanitizes the DAG name; call
handler.transformArtifactPaths (same as the existing test) and compute the
expected sanitized directory using fileutil.SafeName on the DAG name, then
assert incoming.ArchiveDir is under baseDir and equals filepath.Join(baseDir,
fileutil.SafeName(dag.Name), <archive-suffix-from-incoming>) and that the
directory exists; reuse mockDAGRunAttempt, Handler, and exec.DAGRunStatus from
the current test setup to locate the change near transformArtifactPaths and
fileutil.SafeName usage.

In `@internal/service/frontend/api/v1/dagruns.go`:
- Around line 3284-3289: The comparator passed to sort.Slice can return false
both ways when names differ only by case, making ordering nondeterministic;
update the anonymous comparator used with sort.Slice over nodes so that after
comparing Type and after comparing strings.ToLower(nodes[i].Name) vs
strings.ToLower(nodes[j].Name) you add a tiebreaker for case-differing names
(e.g., if strings.EqualFold(nodes[i].Name, nodes[j].Name) or if lowercased names
are equal, return nodes[i].Name < nodes[j].Name) so the comparison becomes
deterministic; reference the existing symbols sort.Slice, nodes, nodes[i].Name,
nodes[j].Name, and api.ArtifactNodeTypeDirectory.

In `@internal/service/scheduler/enqueue_test.go`:
- Line 39: Two tests call the function with th.Config.Paths.ArtifactDir but
never assert it was persisted; update the tests that pass
th.Config.Paths.ArtifactDir to assert that the job/archive status metadata
contains that artifact path. Locate the assertions after the enqueue/creation
call (look for uses of th.Config.Paths.ArtifactDir in enqueue_test.go and the
variables holding returned status or archive objects, e.g., status, job, or
archive) and add an assertion that status.Metadata (or archive.Status.Metadata)
includes the artifact dir value (compare to th.Config.Paths.ArtifactDir) and/or
the specific metadata key used by the implementation (e.g., "artifact_dir" or
"artifactDir"). Ensure you add the same assertion in both call sites (the one at
the earlier location and the one referenced as also applies to 103-103).

In `@internal/service/scheduler/enqueue.go`:
- Around line 73-83: The nil check on dagCopy.Artifacts is redundant because
dagCopy.ArtifactsEnabled() already guarantees Artifacts != nil; remove the inner
"if dagCopy.Artifacts != nil" block and directly read dagCopy.Artifacts.Dir into
dagArtifactDir before calling logpath.GenerateDir so artifactDir is computed the
same way (preserve artifactDir, dagArtifactDir variables and the GenerateDir
call and error handling around it).

In `@ui/src/api/v1/schema.ts`:
- Around line 4812-4813: The ArtifactPath property is currently typed too
loosely; update the OpenAPI parameter/schema that defines ArtifactPath to
enforce a safe relative-path constraint (e.g., minLength: 1 and a pattern that
disallows a leading slash and any '..' segments such as using a
negative-lookahead regex like one that forbids '^/' and forbids '..'), then
regenerate frontend types with pnpm gen:api so the generated ArtifactPath in
ui/src/api/v1/schema.ts reflects the no-leading-slash/no-`..` rule; locate the
OpenAPI parameter/schema that maps to ArtifactPath (the same schema/parameter
name used to produce the ArtifactPath property) and modify it accordingly before
running the generator.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api/v1/api.yaml`:
- Around line 6753-6765: The ArtifactPath query parameter schema currently
forbids Unix-style traversal but allows Windows-style inputs; update the
ArtifactPath schema (the component named ArtifactPath) to also reject
backslash-based traversal and Windows absolute/drive paths by adding additional
validation rules: disallow any backslash characters, disallow leading
backslashes, disallow backslash-based parent segments like "..\" and disallow
drive-letter prefixes like "C:\" (e.g., add complementary not: pattern checks
for backslashes and for ^[A-Za-z]:\\ or ^\\). Ensure these new patterns are
combined with the existing patterns so both Unix and Windows traversal/absolute
paths are rejected.

In `@internal/cmd/artifacts_regression_test.go`:
- Around line 103-107: The YAML writer builds the test config using fmt.Sprintf
into the variable content with unquoted path interpolations (dagRunsDir and
artifactDir), which can produce invalid YAML for Windows-style paths; change the
interpolation to quote those values (use quoted formatting for dagRunsDir and
artifactDir in the fmt.Sprintf call that assigns content) so the generated YAML
contains proper string literals for the paths.

In `@internal/runtime/remote/artifact_uploader.go`:
- Around line 57-67: The upload routine reads u.attemptID for every chunk which
allows a concurrent SetAttemptID to change the value mid-stream; modify
UploadDir (or its call sites like Finalize) to capture the attempt ID once into
a local immutable variable (e.g., localAttemptID := u.attemptID with u.mu held)
at the start of the upload and then use that localAttemptID for all
chunks/stream messages instead of reading u.attemptID repeatedly; ensure
SetAttemptID still acquires u.mu and updates u.attemptID, but UploadDir uses the
snapshot to prevent mixed AttemptId values across RPC chunks (apply same change
where chunk streaming occurs, e.g., the code paths noted around lines 70-74 and
137-163).
- Around line 117-121: The current UploadDir callback defers file.Close(), which
only runs when UploadDir returns and leaks FDs for each file; change the
per-file handling so the file is closed immediately after its upload work
completes (e.g., move the open/upload logic into a short helper or closure
inside UploadDir or call file.Close() explicitly right after the
upload/processing and before returning from the callback). Update the code
around the os.Open(filepath.Clean(path)) usage in UploadDir to remove the
deferred close and ensure file.Close() is invoked in all success and error paths
for that specific file (including after any uploadToRemote or io.Copy calls).

In `@internal/service/coordinator/artifact_handler.go`:
- Around line 97-103: The streamKey function builds a cache key from chunk
fields but uses chunk.RelativePath raw; canonicalize/normalize RelativePath
before using it so semantically equivalent paths (e.g., "foo/./bar.txt" vs
"foo/bar.txt" or leading/trailing slashes) map to the same writer. In
artifactHandler.streamKey, call the path normalization (e.g., path.Clean and
trim any leading "./" or leading "/" as your project expects) on
chunk.RelativePath and use that cleaned value in the fmt.Sprintf to produce the
key.
- Around line 69-75: The handler currently skips writing when chunk.IsFinal is
true, dropping trailing bytes; modify the branch handling chunk.IsFinal so it
still obtains the writer via h.getOrCreateWriter(ctx, chunk), writes the chunk's
payload to that writer (checking and returning any write error), then closes the
writer via h.closeWriterByKey(ctx, key) and deletes activeKeys[key]; ensure you
reference chunk.IsFinal, h.getOrCreateWriter, h.closeWriterByKey, key and handle
errors from the write before cleanup.

In `@internal/service/coordinator/handler.go`:
- Around line 1460-1492: The early return currently checks h.artifactDir == ""
|| incoming == nil which prevents using a DAG-level artifacts dir when
Handler.artifactDir is empty; remove the h.artifactDir check so the guard only
returns when incoming == nil, then keep the existing logic in resolve (call
attempt.ReadDAG, use dag.Artifacts.Dir if set, otherwise fall back to
h.artifactDir, run eval.String with eval.WithOSExpansion, trim and validate
baseDir) so DAG-level artifact directories are honored even when
Handler.artifactDir is empty (refer to h.artifactDir, incoming.ArchiveDir,
latestStatus, attempt.ReadDAG, dag.Artifacts.Dir, baseDir and eval.String).

In `@internal/service/frontend/api/v1/dagruns.go`:
- Around line 3386-3400: The current checks only reject directories using
info.IsDir(), but they should also reject non-regular files (FIFOs, sockets,
devices) to avoid hanging or exposing special files; update the logic around
os.Stat(absPath) so that after getting info you return an error (e.g.,
os.ErrNotExist) when !info.Mode().IsRegular() instead of only when info.IsDir(),
and keep the rest of the flow (opening filepath.Clean(absPath) and deferring
Close) unchanged; apply the same change to the other occurrence referenced (the
block around the later open at lines 3453-3461) so both preview and download
paths consistently reject non-regular files.

In `@internal/service/scheduler/ha_health_test.go`:
- Around line 99-102: The test constructs dagRunStore via filedagrun.New and
wires WithArtifactDir(cfg.Paths.ArtifactDir) but never sets
cfg.Paths.ArtifactDir, so the fixture passes an empty path; set
cfg.Paths.ArtifactDir to a valid non-empty path (for example a t.TempDir() or
the same base used for cfg.Paths.DAGRunsDir) before calling filedagrun.New so
WithArtifactDir receives a real artifact directory and the test exercises
artifact-dir behavior correctly.

In `@ui/src/features/dags/components/artifacts/ArtifactsTab.tsx`:
- Around line 236-257: The buildArtifactDownloadUrl/fetchArtifactDownload flow
uses a direct fetch and duplicates auth/base-URL handling; replace this with the
generated shared API client by calling client.GET (or client.POST if required)
for the artifacts/download endpoint and pass remoteNode and path as query
params, ensuring you use the client instance that already handles base URL and
auth tokens; update fetchArtifactDownload to return the client response (and
propagate AbortSignal if client supports it) and remove
buildArtifactDownloadUrl, and make the same change for the other similar
handlers referenced (lines 404-422).
- Around line 259-312: fetchTree can write state after async awaits and
overwrite newer runs; add a per-call request identity (e.g., a serial requestId
or AbortController) at the top of fetchTree and capture it in a local variable
before any await, then check that identity (or !aborted) before every state
mutation (setTree, setOpenDirs, setSelectedPath, setPreview, setTreeError,
setTreeLoading, setPreviewVersion) in the try, catch and finally blocks; also
wire that identity/abort to the useEffect so each invocation creates a fresh
id/controller and cancels or ignores prior requests when
dagRun.artifactsAvailable, dagRun.dagRunId, dagRun.rootDAGRunId or remoteNode
change, and ensure requestArtifactTree is cancellable or its result is ignored
when the id/controller no longer matches.

---

Outside diff comments:
In `@api/v1/api.yaml`:
- Around line 2249-3011: The OpenAPI spec added several new endpoints/operations
(e.g., operationId getDAGRunArtifacts, getDAGRunArtifactPreview,
downloadDAGRunArtifact, getDAGRunOutputs, retryDAGRun, terminateDAGRun,
getDAGRunStepLog, downloadDAGRunStepLog, getDAGRunStepMessages,
updateDAGRunStepStatus, approveDAGRunStep, rejectDAGRunStep, pushBackDAGRunStep,
getSubDAGRunDetails, getSubDAGRunSpec, getSubDAGRunLog, downloadSubDAGRunLog,
getSubDAGRunArtifacts, getSubDAGRunArtifactPreview, downloadSubDAGRunArtifact
and schemas like
ArtifactTreeResponse/ArtifactPreviewResponse/DAGRunOutputs/ApproveStepRequest
etc.), but the generated server code wasn’t updated; regenerate the REST server
types and handlers from the OpenAPI spec using the project’s oapi-codegen flow
(run the provided make/api generation step), then commit the updated generated
types/handlers so the codebase reflects the new operationIds and response
schemas.

---

Duplicate comments:
In `@ui/src/api/v1/schema.ts`:
- Around line 3322-3328: The public DTO exposes a node-local path via
DAGArtifactsConfig.dir; remove the dir field from the OpenAPI model (the source
that generates DAGArtifactsConfig) so the generated ui/src/api/v1/schema.ts no
longer includes DAGArtifactsConfig.dir, update the backend/OpenAPI spec (where
DAGArtifactsConfig is defined) to only include capability flags (e.g.,
enabled/artifactsAvailable), then regenerate the frontend types with pnpm
gen:api rather than editing the generated file directly; ensure references to
DAGArtifactsConfig.dir are removed or handled server-side behind artifact
endpoints.

---

Nitpick comments:
In `@internal/cmn/config/loader_test.go`:
- Around line 687-692: The test currently uses the self-referential value
cfg.Paths.ArtifactDir in the expected struct which can hide regressions; change
the expected ArtifactDir to the explicit derived value from the DataDir instead
of reusing cfg.Paths.ArtifactDir—e.g., compute it with
filepath.Join(cfg.Paths.DataDir, "artifacts") or assert the literal
"/var/dagu/data/artifacts" (matching DataDir "/var/dagu/data") when building the
expected value so the test verifies the derived-path logic (update the expected
variable where ArtifactDir is set and reference cfg.Paths.DataDir, not
cfg.Paths.ArtifactDir).

In `@internal/core/spec/dag_test.go`:
- Around line 2164-2202: Add a new table test case to cover the explicit-disable
path: insert a test entry in the tests slice (same style as existing entries)
named e.g. "DisabledExplicit" with yaml set to:
artifacts:
  enabled: false
and expected set to &core.ArtifactsConfig{Enabled: false}; place it alongside
the other cases so the test verifies explicit disabling is parsed into
core.ArtifactsConfig.Enabled == false (refer to the tests variable and
core.ArtifactsConfig type in dag_test.go).

In `@internal/service/coordinator/client.go`:
- Around line 676-716: The StreamLogs/StreamArtifacts methods duplicate the
coordinator selection + health check + failover loop; extract that loop into a
single helper on clientImpl (e.g., call it withStreamFailover or
tryCoordinatorStream) that takes context and a callback which, given a
coordinator client, attempts to open the specific stream (so it can invoke
client.client.StreamLogs or client.client.StreamArtifacts). Move logic that
calls getCoordinatorMembers, rand.Shuffle, getOrCreateClient, isHealthy,
recordFailure, recordSuccess, and returns the opened stream into that helper,
and have StreamLogs and StreamArtifacts call the helper with the appropriate
stream-opening callback so both reuse the same failover/retry flow.