fix(ui,process): dashboard agent lifecycle feedback + Windows daemon stop

[lucifer78907]

Type of Change

• Bug fix
• New feature
• Enhancement / refactor
• New skill
• Documentation
• CI / build

Summary

• Stop showed no stopping state — the card fully renders that state
  (dot, spinner, disabled buttons) but nothing ever assigned it, and the
  one broadcast fired after the blocking stop rather than before.
• A stopped card came back wrong — the broadcast sent a stub struct, so
  Go's zero values ("", 0, null) overwrote the real model/tools/
  channels tags, while port,omitempty was omitted and left a stale
  port. Only a refresh gave a correct card.
• Queued events could be rewritten — broadcasts shared a live pointer
  that handleSSE marshals on drain, so a later mutation meant the
  stopping event would serialize as stopped.
• forge serve stop failed on Windows even when it succeeded — IsAlive
  treated "can I open a handle?" as liveness, but the stop path holds a
  handle itself, which is exactly what keeps a dead process resolvable. It
  killed the daemon, then spent 10s convinced it was alive, then failed
  trying to kill it twice. Previously invisible: the UI discarded the error.
• Switching agents kept the previous chat — useState survives a prop
  change and ChatPage wasn't keyed, so the transcript, sessionId (would
  post the wrong session) and streaming (left the new input dead) all
  stayed behind.
• A new agent needed a refresh to appear — EventSource dispatches by
  event name and only agent_status had a listener, so agent_created was
  silently discarded.

Description

Four dashboard bugs, all variations on "the UI never learns what happened".
The last one turned out to be a Windows process bug that the first fix
merely made visible.

1. Stop showed no stopping state, and repainted the card wrong
(forge-ui/process.go, handlers.go)

StateStopping was defined and fully rendered by the card (status dot,
spinner, isBusy button disabling) but never assigned anywhere. The
only broadcast also happened after the blocking forge serve stop, and
handleStopAgent is synchronous, so nothing reached the browser until the
child exited.

That broadcast then sent a stub &AgentInfo{ID, Directory, Status}. A
partial struct is not a partial JSON object — version/framework/
model/tools/channels/skills have no omitempty and marshal as zero
values, which the dashboard's object-spread merge wrote over the real
values, while port,omitempty was omitted and left the stale running
port in place. Hence "only a refresh gives a correct card".

Stop now takes the full *AgentInfo, announces stopping before
cmd.Run() (SSE is served on its own goroutine, so it delivers while Stop
is still blocked), clears Port, and rolls status/port back with the
stderr message on failure so the card can't wedge on stopping with both
buttons disabled.

All six broadcasts now go through broadcastStatus, which sends a
snapshot rather than the pointer: SSEBroker queues events and
handleSSE marshals on drain, so sharing the live pointer let a later
mutation rewrite an already-queued event — the stopping event would
serialize as stopped. Start had the same latent aliasing.

2. forge serve stop failed on Windows even when it succeeded
(forge-core/util/process/process_windows.go, forge-cli/cmd/serve.go)

Surfaced by fix 1 — previously the UI caught this and discarded it, so it
had been failing silently. It reported
sending SIGKILL: TerminateProcess: Access is denied plus the whole cobra
help text.

IsAlive used OpenProcess success as the liveness test, but Windows
keeps a terminated process's kernel object alive while any handle to it
stays open (so callers can read its exit code), and OpenProcess succeeds
against that object. serveStopRun holds exactly such a handle — from
os.FindProcess, needed to do the killing — for the whole function, so the
stop path pinned its own dead child and then asked whether it was running:

1. sendTermSignal → proc.Kill(), daemon actually dies
2. IsAlive keeps reporting alive: the held handle keeps the object resolvable
3. full 10s poll loop burns
4. sendKillSignal on the dead process → ERROR_ACCESS_DENIED
5. non-zero exit — the stop succeeded but reported failure

Now opens with SYNCHRONIZE and waits on the handle with a zero timeout. A
process handle becomes signaled exactly when the process exits, which is
unambiguous. (GetExitCodeProcess was the alternative, but its
STILL_ACTIVE sentinel is 259 and collides with a genuine exit code of
259.)

Also: serve stop/status/logs were missing SilenceUsage (serve and
start already had it), and a kill that fails because the target is
already gone now counts as a successful stop — correct on Unix too, where
killing a dead pid returns ESRCH.

Windows-only (//go:build windows). process_unix.go is untouched and
Unix keeps its Signal(0) probe. The serve.go changes reach all
platforms but are either cosmetic (SilenceUsage) or inside the error
branch only, so the normal Unix path is unchanged.

3. Switching agents kept the previous agent's chat (static/app.js)

useChatStream holds messages/sessionId/streaming in useState.
Preact reuses the mounted ChatPage on an agentId change, and useState
survives a prop change — so the header (prop) and session list
(agentId-keyed effect) re-rendered correctly while the transcript did
not.

Worse than cosmetic: sessionId was stale too, so the first message to the
new agent would post the previous agent's session id; and stale
streaming left the new agent's input dead while the old agent's
setMessages calls appended into it.

Fixed by keying ChatPage on the agent id, which resets all of it at once,
plus an unmount abort so the previous stream doesn't keep running into a
discarded component.

4. A newly created agent needed a refresh to appear (static/app.js)

EventSource dispatches strictly by event: name and useSSE registered
only agent_status, so the agent_created event handleCreateAgent
broadcasts reached the browser and was silently discarded. The status
merge also deliberately ignores unknown ids, so a creation could never be
handled there either. Only the 60s poll or a manual refresh surfaced it.

Now registers an agent_created listener and refetches on it — the event
carries only {id, directory}, not the record a card renders from.
handleCreateAgent broadcasts after CreateFunc has written to disk, so
the refetch is guaranteed to see the new forge.yaml.

General Checklist

• Tests pass for affected modules (go test ./...)
• Code is formatted (gofmt -w)
• Linter passes (golangci-lint run)
• go vet reports no issues
• No new egress domains added without justification

Notes on the unchecked boxes:

• gofmt -w deliberately not run. The working copy is CRLF, so it
  would rewrite all 35 files in the module and bury the change. Verified
  instead that the diff contains no line-ending churn and that gofmt
  reports nothing beyond line endings on the touched files. Please confirm
  in CI.
• golangci-lint not installed locally, so it hasn't been run.
• Tests were run on Windows, where these pre-existing failures are
  environmental (Unix permission bits, CRLF fixtures) and were confirmed to
  fail on a clean tree with the changes stashed: forge-ui
TestForgeSkillBuilderMD_MirrorsPrompt, uiconfig
TestSetEnvFileValue_* / TestLoad_UserFallbackWhenNoWorkspace,
  forge-cli/cmd TestAuthMintToken_StoresWithCorrectPermissions /
  TestDeriveEgressDomains / TestMCPLogout_DeletesTokens.
• The changes have not been cross-compiled for linux/darwin locally;
  platform isolation was verified by build tags and diff review.

Tests added

• TestProcessManagerStopBroadcastsStoppingFirst — asserts the first event
  is stopping, carries full metadata, and is not the caller's pointer
• TestProcessManagerStopExecError — gains rollback assertions
• TestIsAlive_FalseWhileHandleHeld — holds a handle across the kill and
  fails against the old implementation (verified both ways). The
  existing TestIsAlive_ChildAfterExit could not catch this because
  cmd.Wait() closes Go's handle, releasing the object before the check
  runs.

Frontend changes are manually verified in the dashboard; there is no JS
test harness in the repo.

Related Issues

[lucifer78907]

@initializ-mk please review these bug fixes

[initializ-mk]

Thanks for a genuinely excellent contribution — every one of these fixes traces a real bug to its actual root cause, and the commit messages are some of the clearest I have read. The Windows handle-openability-≠-liveness diagnosis in particular is a great catch. I verified all of it against your branch; the core is correct. Requesting changes on scope + two small hardening items below, none of which are about the correctness of the fixes themselves.

Verified correct

• process.go — broadcastStatus snapshot fixes the queued-event aliasing (the rewritten field is Status, a scalar, so the shallow copy fully decouples the stopping→stopped rewrite); stopping announced before the blocking cmd.Run(); rollback-on-error prevents a card wedged on stopping.
• process_windows.go — OpenProcess(QUERY_LIMITED_INFO|SYNCHRONIZE) + WaitForSingleObject(h, 0) returning event == waitTimeout for alive is the right liveness test; handle closed; the new test fails against the old implementation.
• serve.go — SilenceUsage, and treating a kill that fails because the target is already gone as success correctly closes the liveness/kill race (right on Unix ESRCH too).
• app.js — keying ChatPage is the correct fix for the stale useState across an agent switch; unmount abort, agent_created listener + refetch, and optimistic stop + rollback are all sound. The new useSSE((type, data) => …) signature has exactly one caller, so nothing else breaks.

Changes requested

1. Split the code-agent prompt change into its own PR (see inline on runner.go). The codeAgentDirective + SKILL.md change is an LLM-behavior change unrelated to the dashboard / Windows-stop theme and is not in the PR summary. It is a good fix on its own merits, but bundling a system-prompt behavioral change with UI/process bugfixes makes both harder to review and to revert independently, and there is no eval pinning the new greeting→text behavior. Please extract it into a separate PR (or, if it must stay, describe it in the PR body and add a note on how the conversation carve-out was verified).
2. Document the shallow-copy assumption in broadcastStatus (see inline on process.go).
3. Client stop-rollback hardcodes running. In handleStop's catch you set status: 'running', but the true prior state could differ (e.g. the agent was already errored). The server's authoritative agent_status SSE event reconciles it so it self-corrects, but the card can briefly show the wrong state. Prefer mirroring the server's rollback-to-previous, or lean on the SSE event and drop the optimistic rollback. Low severity.

Note

Appreciated the transparency on the unchecked boxes (gofmt/CRLF, golangci-lint, Windows-only testing) — CI confirms Lint + gofmt are clean and all 10 checks pass, so those are resolved.

Really nice work — once the prompt change is split out and the two small items are addressed, this is good to go.

[initializ-mk]

Change requested (scope): this codeAgentDirective rework — and the paired SKILL.md edit — is an LLM-behavior change, unrelated to the dashboard-feedback / Windows-daemon-stop bugs this PR is about, and it is not mentioned in the PR summary. The fix itself is sound and well-reasoned (the old unconditional "every response MUST include tool calls" left a plain "Hi" no legal move, so the model saved its greeting to a file — scoping the mandate to coding work and carving out conversation is the right call). But please lift it into its own PR: a system-prompt behavioral change deserves independent review and a clean revert path, and ideally a small eval demonstrating greeting→text vs the old greeting→file-write. If it truly must ride along, at minimum call it out in the description.

[initializ-mk]

Change requested (document the assumption): snapshot := *info is a shallow copy. It correctly fixes the bug because the fields mutated between broadcasts (Status/Port/Error) are scalars, so the copy fully decouples them — that is exactly right. The subtlety is that AgentInfo's slice fields (Tools/Channels/Skills) still share the caller's backing arrays, so this only stays correct as long as those slices are reassigned at load and never mutated in place after a broadcast. Please add a one-line comment here recording that assumption, so a future in-place slice mutation doesn't silently reintroduce the aliasing for those fields.