P3 [CRF-17] buildMCPServerResources converts MCPToolInfo to MCPTool, mapping Name, Description, and Schema (properties) but dropping Required []string. hashMCPServer covers only MCPTool fields, so when a tool changes which parameters are required without any other schema change, the server's ContentHash and the aggregate hash stay the same.

The sync loop does detect the change: mcpToolsChanged uses reflect.DeepEqual on the full MCPToolInfo (which includes Required), so a re-resolve fires and the snapshot version increments. But the hashes don't flip, so hash-based consumers (e.g. coderd's push path in follow-up PRs) would miss the update.

Fix: add Required []string to MCPTool, populate it here, and include it in hashMCPServer's hash computation.

(Meruem P3, Razor P3)

🤖

P3 [CRF-18] lastPaths = paths is committed before Reload at line 1623. If Reload fails (e.g. manager closing during a shutdown race), the next wake sees equal paths and short-circuits, permanently skipping the explicit reload for those paths. Self-healing requires a file edit on disk that changes the discovered path set.

In practice this bites only during shutdown races, and the agent is tearing down anyway. But the fix is trivial: set lastPaths after Reload succeeds, or clear it on error.

if !slices.Equal(paths, lastPaths) {
    if err := a.mcpManager.Reload(ctx, paths); err != nil {
        a.logger.Warn(ctx, "failed to reload workspace MCP servers", slog.Error(err))
    } else {
        lastPaths = paths
    }
}

(Hisoka)

🤖

P3 [CRF-19] contextSnapshotToProto and snapshotResponse at agentcontext/api.go:180 are structurally identical converters from agentcontext.Snapshot. The Name field has already drifted: contextSnapshotToProto maps it (line 327), but snapshotResponse's SnapshotResource type lacks it. Adding a field to Resource now requires updating two converters in two packages.

A shared resourceToWire helper or at minimum syncing the field sets would prevent further drift.

(Robin)

🤖

Nit [CRF-20] slices.Sorted(maps.Keys(byServer)) replaces these four lines. CRF-10 fixed this pattern in mcpConfigPaths (same PR), but this new instance uses the old manual form.

(Robin)

🤖

Nit [CRF-21] validateMCPConfig iterates shape.MCPServers in nondeterministic map order. When a .mcp.json has multiple invalid entries, the error message varies between resolves. Status (StatusInvalid) and the aggregate hash are unaffected (hash does not include Error), so this is cosmetic only. Sorting the keys before iterating would make the error stable.

(Komugi)

🤖