🐛 fix(edge): post-review cleanup — slog test race, doc truth-ups, pollInterval tolerance

COMPATIBILITY.md

@@ -41,7 +41,7 @@ usage as of the Portwing version in the row above:
 |---|---|
 | `app/configs/portwing.yaml` | Base preset: container lifecycle, image pull/inspect/remove, `/events`, narrow network/volume/distribution/service reads. No exec, no compose-stack network/volume creation, no build. |
 | `app/configs/portwing-with-exec.yaml` | `portwing.yaml` plus the exec/attach paths Portwing's interactive terminal feature needs. |
-| `app/configs/portwing-with-compose.yaml` | `portwing.yaml` plus `POST /networks/create`, `DELETE /networks/*`, `POST /networks/*/disconnect`, `POST /volumes/create`, `DELETE /volumes/*` — what compose-stack deploys through Portwing need. Still denies `/build`; BuildKit fallback needs `/session` + `/grpc`, which no preset here models yet. |
+| `app/configs/portwing-with-compose.yaml` | `portwing.yaml` plus `POST /networks/create`, `POST /networks/*/connect`, `DELETE /networks/*`, `POST /networks/*/disconnect`, `POST /volumes/create`, `DELETE /volumes/*` — what compose-stack deploys through Portwing need. Still denies `/build`; BuildKit fallback needs `/session` + `/grpc`, which no preset here models yet. |
 
 Portwing's `examples/sockguard.yaml` is a manually-synced copy of sockguard's
 `app/configs/portwing.yaml` (the no-exec, no-compose base preset) — update

README.md

@@ -611,13 +611,16 @@ live container state through this endpoint using their standard tool-call flow.
 | Tool | Description |
 |------|-------------|
 | `list_containers` | All containers — id, names, image, state, status, labels |
-| `inspect_container(id)` | State, image, env-var count (values never exposed), mounts, networks, restart policy |
+| `inspect_container(id)` | State, image, env-var count (values never exposed via this MCP tool), mounts, networks, restart policy |
 | `container_logs(id, tail)` | Last N lines of stdout/stderr (max 500) |
 | `host_metrics` | CPU, memory, disk, network, uptime snapshot |
 | `container_stats(id)` | One-shot CPU/memory/network stats for a container |
 
 **Credential hygiene:** `inspect_container` returns only the *count* of environment variables —
-values are never transmitted, preventing accidental secret leakage.
+values are never transmitted through this MCP tool, preventing accidental secret leakage. This
+scoping applies only to `inspect_container`: the sibling `GET /api/containers` REST endpoint (and
+the container inventory synced over the edge WebSocket) returns full container env var values by
+design — see [Security Model](docs/security-model.md) for details.
 
 #### Add to Claude Desktop (claude_desktop_config.json)
 

SPEC.md

@@ -142,6 +142,8 @@ The Drydock `/api/portwing/ws` endpoint requires the Ed25519 fields (`pubKeyId`,
 | `dd:trigger_response` | Agent -> Server | Trigger result |
 | `dd:container_log_request` | Server -> Agent | Request container logs |
 | `dd:container_log_response` | Agent -> Server | Container log data |
+| `dd:container_delete_request` | Server -> Agent | Request container removal |
+| `dd:container_delete_response` | Agent -> Server | Removal result (`success`/`error`) |
 
 ## 4. Standard Mode HTTP API
 

docs/content/docs/authentication.mdx

@@ -87,7 +87,7 @@ docker run -d --name portwing \
 ```
 
 <Callout type="info">
-`TOKEN_HASH` is the recommended choice when you need shared-secret token auth. The plaintext token exists only on the caller side; the agent stores and compares only the Argon2id hash.
+`TOKEN_HASH` is the recommended *token-based* option — among the three shared-secret mechanisms above, it's the one to reach for, since the plaintext token exists only on the caller side and the agent stores and compares only the Argon2id hash. [Ed25519 per-client keys](#4-ed25519-per-client-keys) below remain the recommended mechanism overall for production, since they avoid a shared secret entirely; use `TOKEN_HASH` when a caller can't do per-request signing.
 </Callout>
 
 ---
@@ -186,7 +186,7 @@ The hello carries `pubKeyId`, `timestamp`, `nonce`, and `signature` fields. The
 See [Connection Modes](/connection-modes) for a full description of edge mode.
 
 <Callout type="warn">
-Edge mode requires the Drydock controller to be started with `DD_EXPERIMENTAL_PORTWING=true` — otherwise `/api/portwing/ws` returns 404 and the agent loops reconnect failures indefinitely.
+Edge mode requires the Drydock controller to be started with `DD_EXPERIMENTAL_PORTWING=true` — otherwise `/api/portwing/ws` returns 404, which the agent treats as fatal and exits immediately (it does not retry).
 </Callout>
 
 <Callout type="warn">

docs/content/docs/drydock-integration.mdx

@@ -91,7 +91,7 @@ Sent on every new SSE connection. Drydock reads `version`, `os`, `arch`, `cpus`,
 <Callout type="info">
 `memoryGb` is read from `/proc/meminfo` (the binary stays cgo-free) and rounded to one decimal GiB; non-Linux hosts report `0`, which Drydock accepts. `pollInterval` is the agent's `DD_POLL_INTERVAL` rendered as a Go duration string — the field is informational and displayed as-is. Portwing 0.5.x and earlier sent `memoryGb: 0` and omitted `logLevel`/`pollInterval`.
 
-`pollInterval` actually takes three different string shapes today depending on the source — a Go duration (`"5m0s"`, Standard Mode `dd:ack`), a cron expression (Drydock's own legacy Node.js agent), and a bare integer string (`"300"`, Edge Mode's `welcome` frame — see below). Treat it as an opaque, display-only string in all three cases; do not parse it.
+`pollInterval` takes several different shapes depending on the source: a Go duration string (`"5m0s"`, Standard Mode `dd:ack`), a cron expression (Drydock's own legacy Node.js agent), and a bare integer string (`"300"`) on Drydock's REST `AgentInfo` surface. Edge Mode's `welcome` frame is different again — it sends `pollInterval` as a JSON *number* (e.g. `300`), not a string (see below). Treat it as an opaque, display-only value in all cases; do not parse it.
 </Callout>
 
 ### `dd:container-added` / `dd:container-updated`
@@ -208,7 +208,7 @@ The sequence below is the production handshake, implemented on the agent side in
 </Callout>
 
 <Callout type="warn">
-Requires the Drydock controller to be started with `DD_EXPERIMENTAL_PORTWING=true` — otherwise `/api/portwing/ws` returns 404 and the agent loops reconnect failures indefinitely.
+Requires the Drydock controller to be started with `DD_EXPERIMENTAL_PORTWING=true` — otherwise `/api/portwing/ws` returns 404, which the agent treats as fatal and exits immediately (it does not retry).
 </Callout>
 
 Handshake sequence:

docs/content/docs/security-model.mdx

@@ -319,6 +319,14 @@ Portwing is a privileged agent: an authenticated caller has full Docker API acce
 
 ---
 
+## Container env vars are not redacted on `/api/containers`
+
+`GET /api/containers` and the container inventory synced to Drydock over the edge WebSocket (`dd:container_sync` / `dd:container_added` / `_updated`) include each container's environment variables as plaintext key/value pairs (`RuntimeDetails.Env` in `internal/adapter/containers.go`). This is deliberate: Portwing does not redact or filter env values on this surface, and leaves that responsibility to Drydock (or another downstream consumer) if redaction before display or storage is required.
+
+This is distinct from the MCP `inspect_container` tool, which reports only an env-var *count* and never the values — see the [README](https://github.com/codeswhat/portwing#mcp--ai-assistant-integration). Any authenticated caller of `/api/containers`, and any system that receives the edge sync, sees full env var values. This is most relevant to standalone or no-Sockguard deployments, where the caller/consumer trust boundary is wider than "operator only."
+
+---
+
 ## Related pages
 
 - [Authentication](/authentication) — token, Argon2id hash, and Ed25519 key setup

docs/drydock-integration.md

@@ -89,7 +89,7 @@ The edge-mode hello is Ed25519-signed (`pubKeyId`/`timestamp`/`nonce`/`signature
 
 > **Drydock version:** the `/api/portwing/ws` controller endpoint and the `portwing/1.0` protocol string require a Drydock build that ships them. Drydock 1.5 is the first controller release with this endpoint, so edge mode needs Drydock 1.5+; older controllers do not expose it.
 >
-> **Requires `DD_EXPERIMENTAL_PORTWING=true`:** the Drydock controller must be started with this flag set — otherwise `/api/portwing/ws` returns 404 and the agent loops reconnect failures indefinitely.
+> **Requires `DD_EXPERIMENTAL_PORTWING=true`:** the Drydock controller must be started with this flag set — otherwise `/api/portwing/ws` returns 404, which the agent treats as fatal and exits immediately (it does not retry).
 
 ---
 
@@ -148,7 +148,7 @@ Portwing sends:
 
 Note: `memoryGb` is read from `/proc/meminfo` (no cgo) and rounded to one decimal GiB; non-Linux hosts report 0, which Drydock accepts. `pollInterval` is the agent's `DD_POLL_INTERVAL` as a Go duration string (Drydock's own agent sends a cron expression here — the field is informational, displayed as-is). Portwing 0.5.x and earlier sent `memoryGb: 0` and omitted `logLevel`/`pollInterval`.
 
-`pollInterval` actually appears in three different string shapes depending on the source, and none of them should be parsed as anything but an opaque, display-only string: a Go duration (`"5m0s"`, from Portwing's Standard-mode `dd:ack`), a cron expression (from Drydock's own legacy Node.js agent), and a bare integer string (`"300"`, from Drydock's Edge Mode `welcome` frame — see the Edge Mode section below).
+`pollInterval` appears in several shapes depending on the source, and none of them should be parsed as anything but opaque, display-only values: a Go duration string (`"5m0s"`, from Portwing's Standard-mode `dd:ack`), a cron expression (from Drydock's own legacy Node.js agent), and a bare integer string (`"300"`) on Drydock's REST `AgentInfo` surface. Drydock's Edge Mode `welcome` frame is different again — it sends `pollInterval` as a JSON *number* (e.g. `300`), not a string (see the Edge Mode section below).
 
 ### `dd:container-added` / `dd:container-updated`
 

docs/security-model.md

@@ -253,3 +253,19 @@ access. The controls above prevent *unauthenticated or improperly authorized*
 access and certain classes of *injection* attacks against the Compose path.
 They do not limit what an authenticated caller can do with the Docker API.
 Operators should treat the Portwing token as a root-equivalent credential.
+
+### Container Env Vars Are Not Redacted on `/api/containers`
+
+`GET /api/containers` and the container inventory synced to Drydock over the
+edge WebSocket (`dd:container_sync` / `dd:container_added` / `_updated`)
+include each container's environment variables as plaintext key/value pairs
+(`RuntimeDetails.Env` in `internal/adapter/containers.go`). This is a
+deliberate design decision — Portwing does not redact or filter env values on
+this surface; that responsibility belongs to Drydock (or another downstream
+consumer) if redaction before display or storage is required. This is
+distinct from the MCP `inspect_container` tool, which reports only an env-var
+*count* and never the values (see the [README](../README.md#mcp--ai-assistant-integration)).
+Any authenticated caller of `/api/containers`, and any system that receives
+the edge sync, sees full env var values — this is most relevant to
+standalone or no-Sockguard deployments where the caller/consumer trust
+boundary is wider than "operator only."

internal/edge/client.go

@@ -277,7 +277,9 @@ func (c *Client) connect(ctx context.Context) (bool, error) {
 	}
 	if env.Type == protocol.TypeError {
 		var errMsg protocol.ErrorMessage
-		if err := json.Unmarshal(env.Data, &errMsg); err == nil {
+		if err := json.Unmarshal(env.Data, &errMsg); err != nil {
+			slog.Warn("controller rejected hello with an unparseable error payload", "error", err, "raw", string(env.Data))
+		} else {
 			slog.Warn("controller rejected hello", "code", errMsg.Code, "message", errMsg.Message)
 		}
 		closeWebSocket(conn, "controller rejected hello")

internal/edge/wire_contract_test.go

@@ -267,7 +267,14 @@ func TestConnectFatal404(t *testing.T) {
 // rather than the generic "expected welcome, got ..." string, and logs both
 // fields.
 func TestConnectHelloRejectedByController(t *testing.T) {
-	t.Parallel()
+	// Deliberately NOT t.Parallel(): this test mutates the process-global
+	// slog default (slog.SetDefault) to capture log output, which races
+	// with any other test in this package that logs concurrently via the
+	// package-level slog.* helpers. Go only runs parallel subtests
+	// concurrently with each other; keeping this test sequential guarantees
+	// no other test observes or contends on the global logger while it's
+	// swapped out. Don't "fix" this by re-adding t.Parallel() and widening
+	// a sleep — that masks the race instead of removing it.
 
 	srv := newControllerServer(t, func(ctrl *websocket.Conn) {
 		readAndAckHello(t, ctrl)

internal/protocol/messages.go

@@ -1,6 +1,10 @@
 package protocol
 
-import "encoding/json"
+import (
+	"encoding/json"
+	"fmt"
+	"strconv"
+)
 
 // Core message types.
 const (
@@ -72,6 +76,45 @@ type WelcomeMessage struct {
 	Config       map[string]string `json:"config,omitempty"`
 }
 
+// UnmarshalJSON tolerates pollInterval arriving as either a JSON number (the
+// shape Drydock's Edge Mode welcome frame actually sends) or a numeric
+// string (a shape seen elsewhere in the ecosystem, e.g. Drydock's REST
+// AgentInfo surface). Without this, a numeric-string welcome frame would
+// fail to unmarshal entirely, silently dropping the serverCompatLevel
+// mismatch warning along with the poll interval.
+func (w *WelcomeMessage) UnmarshalJSON(data []byte) error {
+	type alias WelcomeMessage
+	aux := struct {
+		PollInterval json.RawMessage `json:"pollInterval"`
+		*alias
+	}{
+		alias: (*alias)(w),
+	}
+	if err := json.Unmarshal(data, &aux); err != nil {
+		return err
+	}
+	if len(aux.PollInterval) == 0 {
+		return nil
+	}
+
+	var n int
+	if err := json.Unmarshal(aux.PollInterval, &n); err == nil {
+		w.PollInterval = n
+		return nil
+	}
+
+	var s string
+	if err := json.Unmarshal(aux.PollInterval, &s); err != nil {
+		return fmt.Errorf("pollInterval: neither a number nor a string: %w", err)
+	}
+	n, err := strconv.Atoi(s)
+	if err != nil {
+		return fmt.Errorf("pollInterval: %q is not a valid integer: %w", s, err)
+	}
+	w.PollInterval = n
+	return nil
+}
+
 type RequestMessage struct {
 	RequestID string            `json:"requestId"`
 	Method    string            `json:"method"`

internal/protocol/welcome_test.go

@@ -0,0 +1,119 @@
+package protocol
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+// TestWelcomeMessagePollIntervalShapes verifies WelcomeMessage.PollInterval
+// decodes correctly whether the wire sends it as a JSON number (the actual
+// shape of Drydock's Edge Mode welcome frame) or as a numeric string (a
+// shape documented elsewhere in the ecosystem, e.g. Drydock's REST
+// AgentInfo surface). Before UnmarshalJSON was added, a numeric-string
+// pollInterval failed the whole WelcomeMessage decode.
+func TestWelcomeMessagePollIntervalShapes(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		json string
+		want int
+	}{
+		{
+			name: "numeric pollInterval (real welcome frame shape)",
+			json: `{"pollInterval":300}`,
+			want: 300,
+		},
+		{
+			name: "numeric-string pollInterval (compat shape)",
+			json: `{"pollInterval":"300"}`,
+			want: 300,
+		},
+		{
+			name: "numeric pollInterval with config",
+			json: `{"pollInterval":60,"config":{"serverCompatLevel":"1.5.0"}}`,
+			want: 60,
+		},
+		{
+			name: "numeric-string pollInterval with config",
+			json: `{"pollInterval":"60","config":{"serverCompatLevel":"1.5.0"}}`,
+			want: 60,
+		},
+		{
+			name: "missing pollInterval",
+			json: `{"config":{"serverCompatLevel":"1.5.0"}}`,
+			want: 0,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			var w WelcomeMessage
+			if err := json.Unmarshal([]byte(tt.json), &w); err != nil {
+				t.Fatalf("Unmarshal(%q) returned error: %v", tt.json, err)
+			}
+			if w.PollInterval != tt.want {
+				t.Errorf("PollInterval = %d, want %d", w.PollInterval, tt.want)
+			}
+		})
+	}
+}
+
+// TestWelcomeMessagePollIntervalInvalid verifies malformed pollInterval
+// shapes fail decoding cleanly (an error, not a panic or silent zero).
+func TestWelcomeMessagePollIntervalInvalid(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		json string
+	}{
+		{name: "non-numeric string", json: `{"pollInterval":"not-a-number"}`},
+		{name: "boolean", json: `{"pollInterval":true}`},
+		{name: "object", json: `{"pollInterval":{}}`},
+		{name: "array", json: `{"pollInterval":[]}`},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			var w WelcomeMessage
+			if err := json.Unmarshal([]byte(tt.json), &w); err == nil {
+				t.Errorf("Unmarshal(%q) = nil error, want an error", tt.json)
+			}
+		})
+	}
+}
+
+// TestWelcomeMessageRoundTrip verifies a WelcomeMessage encoded by this
+// package (always a JSON number) decodes back to the same value, so the
+// custom UnmarshalJSON doesn't break the common/agent-facing path.
+func TestWelcomeMessageRoundTrip(t *testing.T) {
+	t.Parallel()
+
+	want := WelcomeMessage{
+		PollInterval: 300,
+		Config:       map[string]string{"serverCompatLevel": "1.5.0"},
+	}
+
+	data, err := json.Marshal(want)
+	if err != nil {
+		t.Fatalf("Marshal: %v", err)
+	}
+
+	var got WelcomeMessage
+	if err := json.Unmarshal(data, &got); err != nil {
+		t.Fatalf("Unmarshal: %v", err)
+	}
+	if got.PollInterval != want.PollInterval {
+		t.Errorf("PollInterval = %d, want %d", got.PollInterval, want.PollInterval)
+	}
+	if got.Config["serverCompatLevel"] != want.Config["serverCompatLevel"] {
+		t.Errorf("Config[serverCompatLevel] = %q, want %q", got.Config["serverCompatLevel"], want.Config["serverCompatLevel"])
+	}
+}