forge serve — HTTP server with SSE

[shahar-caura]

FORGE-17

Closes #64

Effort: M
Depends on #61

Problem

Forge is a fire-and-forget CLI with no persistent server component. To power a web dashboard with real-time updates, we need an HTTP server that serves the SPA, exposes run data as JSON APIs, and streams state changes via Server-Sent Events (SSE).

Solution

Add forge serve subcommand that starts a Go HTTP server using net/http. Serve a REST API for run data and an SSE endpoint that pushes events when run state changes. Use fsnotify to watch .forge/runs/*.yaml for changes and broadcast to connected SSE clients.

File Manifest

Action	File
Create	internal/server/server.go
Create	internal/server/sse.go
Create	cmd/forge/cmd_serve.go

Key Changes

• server.New(cfg ServerConfig) *Server — configurable port, repo roots
• GET /api/runs — list all runs across configured repos (uses scanner)
• GET /api/runs/:id — single run detail with steps
• GET /api/runs/:id/logs?step=N — SSE stream of agent log file (tail -f style)
• GET /api/events — SSE stream of run state changes (new run, step transition, completion)
• fsnotify watcher on .forge/runs/ directories, broadcasts to SSE hub
• cmd_serve.go — cobra subcommand with --port and --repos flags

Interface Changes

New public API:

type ServerConfig struct {
    Port      int
    RepoRoots []string
}

func New(cfg ServerConfig) *Server
func (s *Server) Start(ctx context.Context) error

Acceptance Criteria

• forge serve starts HTTP server on configurable port (default 8080)
• /api/runs returns JSON array of all runs across repos
• /api/runs/:id returns single run with step details
• /api/events SSE endpoint pushes events on state file changes
• /api/runs/:id/logs?step=N streams log file content in real-time
• Server shuts down gracefully on SIGINT/SIGTERM
• make test passes

Verify

go test ./internal/server/... -v
go build ./cmd/forge && ./forge serve --port 9090 &
curl -s http://localhost:9090/api/runs | jq .
kill %1

Anti-Goals

• Do NOT serve the React SPA yet — that's a separate issue
• Do NOT add authentication — local-only server for now
• Do NOT use any web framework — net/http and http.ServeMux only
• Do NOT add WebSocket support — SSE is sufficient

Context for Agent

Read internal/scanner/scanner.go (from #61) for the multi-repo discovery API. Read internal/state/state.go for RunState and Load(). The server is stateless — it reads YAML files on demand and watches for filesystem changes. Standard library net/http patterns only.

[github-actions]

Claude finished @shahar-caura's task in 2m 5s —— View job

---

PR Review: forge serve — HTTP server with SSE

• Gather context
• Read changed files
• Review code
• Post findings

---

Summary

This PR makes three focused improvements to internal/server/sse.go: adds a keepalive ticker to maintain idle SSE connections through proxies, adds fsnotify.Rename to the event filter for broader filesystem event coverage, and doubles the per-client channel buffer from 16→32. All changes are small, correct, and improve reliability.

---

Medium Issues

1. fsnotify.Rename filter may emit spurious load attempts

Severity: Medium
File: internal/server/sse.go:65
Problem: On Linux, fsnotify.Rename fires on the source of a rename (the file being moved away), not the destination. So a .yaml run state file being atomically replaced will get a Create event (already handled), while the old file gets a Rename event — meaning state.Load() is called on a path that no longer exists, silently fails, and broadcasts nothing. The behavior is safe (error is swallowed), but the intent is unclear and it adds noise. The Rename addition is useful for macOS/kqueue where overwrite-by-rename appears as Rename on the target, but that platform difference is undocumented.

🤖 Claude Code Prompt (click to copy)

In internal/server/sse.go, line 65, the fsnotify.Rename event was added to the filter:
    if event.Op&(fsnotify.Write|fsnotify.Create|fsnotify.Rename) == 0 {

Add a comment explaining the intent of including Rename, specifically:
- On macOS/kqueue, an atomic overwrite (rename tmp→target) sometimes produces a Rename on the target file
- On Linux/inotify, it produces a Create on the target, so Rename events on .yaml files mean the file was moved away (safe: state.Load fails and is silently ignored)

Example comment to add directly above line 65:
// Rename is included for macOS/kqueue compatibility: atomically overwriting a
// .yaml file via rename() can surface as Rename (not Create) on the target.
// On Linux this fires for the source path instead; state.Load will fail
// transiently and the error is silently ignored below.

---

Low Issues

2. No test coverage for keepalive behavior

Severity: Low
File: internal/server/sse_test.go
Problem: The 20-second keepalive ticker is new behavior with no test. While the full 20s interval is too long for a test, a test using a very short interval (1ms) would confirm keepalive frames are written to the SSE stream.

🤖 Claude Code Prompt (click to copy)

In internal/server/sse_test.go, add a test for the keepalive behavior.

The test approach: Since ServeHTTP has a hardcoded 20-second ticker, the keepalive isn't directly testable without refactoring. Instead, verify the keepalive comment format is correct by checking that the SSE stream contains ": keepalive\n\n" after enough time.

Alternatively, add a TestSSEHubKeepalive that:
1. Creates a hub with server.NewSSEHub(dir, logger)
2. Connects one SSE client via ServeHTTP in a goroutine
3. Waits 25 seconds (or skip if -short is set: t.Skip("skipping keepalive test in short mode"))
4. Cancels context
5. Asserts the response body contains ": keepalive"

Add the test to internal/server/sse_test.go with a build tag or t.Skip guard:
    if testing.Short() {
        t.Skip("skipping 25-second keepalive test")
    }

---

Suggestions

3. Document the channel buffer size heuristic

Severity: Low
File: internal/server/sse.go:130
Problem: The buffer was doubled from 16→32 without explanation. A brief comment on what load/burst size this is designed for improves maintainability.

🤖 Claude Code Prompt (click to copy)

In internal/server/sse.go, line 130, a buffer of 32 was chosen:
    ch := make(chan []byte, 32)

Add a brief inline comment explaining the choice, e.g.:
    ch := make(chan []byte, 32) // buffer up to 32 events before dropping for slow clients

This makes it easier to revisit the value in the future if load patterns change.

---

What's good

• Keepalive format is correct: : keepalive\n\n uses the SSE comment syntax (lines starting with :) which is transparent to clients but keeps the TCP connection alive through proxies. 20 seconds is a standard interval.
• defer keepalive.Stop() properly placed immediately after the ticker is created — no goroutine leak.
• removeClient is concurrency-safe: both broadcast and removeClient hold h.mu, so there's no risk of sending to a closed channel.
• Comment cleanup (// Slow client; drop this event.) is a nice touch.

---

🚀 Fix All Prompts

🟡 Fix All Medium Issues

In internal/server/sse.go, line 65, the fsnotify.Rename event was added to the filter:
    if event.Op&(fsnotify.Write|fsnotify.Create|fsnotify.Rename) == 0 {

Add a comment explaining the intent of including Rename, specifically:
- On macOS/kqueue, an atomic overwrite (rename tmp→target) sometimes produces a Rename on the target file
- On Linux/inotify, it produces a Create on the target, so Rename events on .yaml files mean the file was moved away (safe: state.Load fails and is silently ignored)

Add directly above line 65:
// Rename is included for macOS/kqueue compatibility: atomically overwriting a
// .yaml file via rename() can surface as Rename (not Create) on the target.
// On Linux this fires for the source path instead; state.Load will fail
// transiently and the error is silently ignored below.

🟢 Fix All Suggestions

1. In internal/server/sse.go, line 130, add an inline comment explaining the buffer size:
    ch := make(chan []byte, 32) // buffer up to 32 events before dropping for slow clients

2. In internal/server/sse_test.go, add a keepalive test with a t.Skip guard for short mode:
    func TestSSEHubKeepalive(t *testing.T) {
        if testing.Short() {
            t.Skip("skipping 25-second keepalive test")
        }
        // connect a client, wait 25s, cancel, assert body contains ": keepalive"
    }

---