Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/agent/api.go`:
- Around line 191-193: The shared DAGRunWatchRegistry created in
api.dagRunWatcher via newDAGRunWatchRegistry(api.dagRunStore,
api.notifyDAGRunWatch, api.logger) lacks teardown handling so canceled/cleaned
sessions keep their watches; add explicit cleanup hooks to unregister watches
when a session ends (e.g., add a RemoveSessionWatches(sessionID) or
Clear(sessionID) method on DAGRunWatchRegistry and call it from the session
teardown/Cancel/Close path), ensure notifyDAGRunWatch or session lifecycle code
invokes that removal, and update DAGRunWatchRegistry to stop any background
pollers for that session to prevent stale polling against dagRunStore.

In `@internal/agent/dag_def_manage.go`:
- Around line 159-161: The loop over result.Items can panic because it
dereferences dag.Name without checking for nil; modify the for loop that builds
items to guard each dag: for each element in result.Items, first check if dag ==
nil and either skip it or construct a safe fallback entry, then call
summarizeDAGDefinition and store.IsSuspended only when dag is non-nil (using
dag.Name). Ensure the references to summarizeDAGDefinition and
store.IsSuspended(dag.Name) are moved inside the nil-check so nil items are not
dereferenced.

In `@internal/agent/dag_run_watch.go`:
- Around line 315-323: The polling loop currently calls readStatus(...) and
complete(...) with context.Background(), which prevents cancellation and can
leak poll goroutines; change both calls to use a cancellable/timeout context
derived from the watch's parent context (e.g., use the ctx variable for the
goroutine or create one with context.WithTimeout(parentCtx, timeout) and defer
cancel()) so store reads and completion respect cancellation/deadline; update
the calls to readStatus(ctx, req.DAGName, ...) and complete(ctx, watchID,
status) and ensure you cancel the derived context to avoid leaks.

---

Outside diff comments:
In `@ui/src/features/dags/components/chat-history/ChatHistoryTab.tsx`:
- Around line 1-5: The file ChatHistoryTab.tsx is missing the required GPL v3
license header; run the repository license tool (make addlicense) to insert the
standard header into this source file, or manually add the GPL v3 header comment
at the top of ChatHistoryTab.tsx so all exported identifiers (e.g., the
ChatHistoryTab component, imports like StepMessagesTable, isActiveNodeStatus,
and any top-level functions) are covered by the license; ensure the header
matches the project's standard used for .ts/.tsx files.

---

Nitpick comments:
In `@ui/src/features/dags/components/chat-history/ChatHistoryTab.tsx`:
- Around line 45-54: The current useEffect unconditionally sets selectedStep
from defaultStep on every change, which overwrites a user's manual choice; add a
boolean state (e.g., userSelected) and set it to true when the user calls
setSelectedStep manually, update the effect in ChatHistoryTab to only copy
defaultStep into selectedStep when userSelected is false (or when the active run
changes), and reset userSelected to false when dagRun.dagRunId (or whichever run
identifier is used) changes so defaultStep can apply for a new run.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/agent/dag_run_manage.go`:
- Around line 287-307: In dagRunManageList(), add a pre-validation that rejects
conflicting time flags by returning toolError when args.Last is set together
with args.From or args.To (e.g., if args.Last != "" && (args.From != "" ||
args.To != "")), mirroring cmd/history.go’s message "cannot use --last with
--from or --to (conflicting time range specifications)"; perform this check
before any parseDAGManageRelativeTime/parseDAGManageAbsoluteTime calls so you
avoid appending multiple exec.WithFrom() options and silent overrides.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/agent/dag_run_watch.go`:
- Around line 320-335: The notifyCtx created in poll is derived from the watch's
ctx so when complete calls entry.cancel() it cancels notifyCtx before r.notify
runs; change poll to create an independent timeout context for notification
(e.g., use context.WithTimeout(context.Background(), r.pollTimeout()) instead of
deriving from ctx) so r.notify(notifyCtx, ...) and AppendExternalMessage run
with a non-canceled root, or alternatively modify complete to delay calling
entry.cancel() until after r.notify returns; update the code around poll,
notifyCtx, complete, entry.cancel and the r.notify call accordingly.

In `@ui/src/features/dags/components/chat-history/ChatHistoryTab.tsx`:
- Around line 68-69: The selectedStep can become stale when historySteps
changes; ensure you guard against a missing selection by computing a
resolvedStep (e.g., check whether historySteps contains a step with name ===
selectedStep and if not fall back to defaultStep) or by resetting state (call
setSelectedStep(defaultStep) inside a useEffect when the selected name is
absent); use resolvedStep instead of selectedStep for the select value, for
deriving selectedNode (find by resolvedStep), for computing isSelectedActive
(use isActiveNodeStatus on the resolved node), and when rendering
StepMessagesTable so you never pass a non-existent step name downstream.

---

Outside diff comments:
In `@ui/src/features/dags/components/chat-history/ChatHistoryTab.tsx`:
- Line 28: Update the stale comment in ChatHistoryTab.tsx to reflect that the
step pool includes agent nodes now—replace "Determine default selected step:
last finished chat step" with a clearer note such as "Determine default selected
step: last finished step (includes chat and agent steps)" so the comment
accurately documents the selection logic for the step pool used by
ChatHistoryTab.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/agent/dag_run_manage.go`:
- Around line 379-386: The diagnose path currently only falls back to
node.ChatMessages when attempt.ReadStepMessages returns an empty slice; change
both call sites (the one around ReadStepMessages at the top of the snippet and
the second site around lines 431-435) so that any non-nil error from
attempt.ReadStepMessages also triggers the same fallback: if ReadStepMessages
returns an error or an empty slice, try status.NodeByName(args.StepName) and
assign node.ChatMessages to messages when available; update dagRunManageDiagnose
and the surrounding logic that uses messages to preserve chat history on read
failures.
- Around line 349-352: The code currently calls
normalizeDAGRunLogStream(args.Stream) which defaults an empty stream to
"scheduler", so a read_log request with a stepName but no stream will
incorrectly return the run/scheduler log; change the guard to fail fast when a
step-specific read is requested without an explicit stream: check the raw
args.Stream (not the normalized value) and if args.StepName is non-empty and
args.Stream is empty, return a toolError indicating that a stream is required
for step logs (e.g., in the same block that currently calls
normalizeDAGRunLogStream), otherwise continue to normalize and validate the
stream as before.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/agent/dag_manage_test.go`:
- Around line 182-195: The test reuses the map variable `got` across two
json.Unmarshal calls which can leave stale keys (like "messages") from the first
payload and yield false positives; fix it by resetting or re-declaring `got`
before the second Unmarshal (e.g., assign got = nil or declare a new var) so the
second call to json.Unmarshal into `got` (used after the second runJSONTool(...)
call) cannot see leftover keys from the prior parse; update the test in
internal/agent/dag_manage_test.go around the second json.Unmarshal to
clear/recreate `got`.