refactor(comments): remove plan/phase refs from agent identity hardening code

Rewrite inline comments added during the agent identity hardening so
they explain the code as it stands today, rather than tying to internal
plan terminology (phase numbers, FR/NFR/H/M/C codes, PR references,
trap zone labels). Commit history already carries the plan archaeology.
Comments now keep the non-obvious invariants (cache boundaries, bypass
gaps, silent-nil traps, dual-tenant semantics) and drop the scaffolding.
Comment-only — no runtime behavior change.

cmd/gateway_methods.go

@@ -43,7 +43,7 @@ func registerAllMethods(server *gateway.Server, agents *agent.Router, sessStore
 
 	// Phase 2: Heartbeat
 	heartbeatMethods := methods.NewHeartbeatMethods(heartbeatStore, msgBus)
-	// Wire cache-aware resolver (Phase 3): accepts agent_key or UUID input
+	// Wire cache-aware resolver so heartbeat can accept agent_key or UUID
 	// without a DB roundtrip on the hot path when the agent is router-cached.
 	heartbeatMethods.SetAgentRouter(agents)
 	heartbeatMethods.Register(router)

internal/agent/loop_tracing.go

@@ -24,12 +24,12 @@ func (l *Loop) emit(event AgentEvent) {
 
 // ID returns the agent's identifier (agent_key, e.g. "goctech-leader").
 // Use for logs, UI, filesystem paths. NEVER for DB FK or DomainEvent.AgentID.
-// See docs/agent-identity-conventions.md (Phase 6).
+// See docs/agent-identity-conventions.md.
 func (l *Loop) ID() string { return l.id }
 
 // UUID returns the agent's canonical UUID (DB primary key).
 // Use for SQL WHERE/JOIN, DomainEvent.AgentID, context propagation.
-// See docs/agent-identity-conventions.md (Phase 6).
+// See docs/agent-identity-conventions.md.
 func (l *Loop) UUID() uuid.UUID { return l.agentUUID }
 
 // Model returns the model identifier for this agent loop.

internal/agent/memoryflush.go

@@ -70,7 +70,7 @@ func ResolveMemoryFlushSettings(compaction *config.CompactionConfig) *MemoryFlus
 // buildMemoryFlushPromptConfig returns the SystemPromptConfig used by the
 // memory flush turn. Extracted as a pure function so tests can assert the
 // config shape (specifically, that AgentUUID is populated) without building
-// a full Loop fixture. See Phase 1 Fix A and M7 mitigation.
+// a full Loop fixture.
 func buildMemoryFlushPromptConfig(
 	agentID, agentUUID, model, workspace string,
 	toolNames []string,
@@ -131,9 +131,8 @@ func (l *Loop) runMemoryFlush(ctx context.Context, sessionKey string, settings *
 	flushSystemPrompt := strings.ReplaceAll(settings.SystemPrompt, "YYYY-MM-DD", today)
 
 	// System prompt: combine agent's normal system prompt context with flush system prompt.
-	// Config construction extracted to buildMemoryFlushPromptConfig for testability — the
-	// AgentUUID field here mirrors loop_history.go:199-201 and must stay in sync
-	// (regression: missing AgentUUID caused identity drift in DomainEvents — see PR #826).
+	// AgentUUID must stay in sync with loop_history.go's SystemPromptConfig —
+	// missing it here historically caused identity drift in downstream DomainEvents.
 	systemPrompt := BuildSystemPrompt(buildMemoryFlushPromptConfig(
 		l.id,
 		l.agentUUID.String(),

internal/agent/memoryflush_test.go

@@ -6,10 +6,11 @@ import (
 	"github.com/google/uuid"
 )
 
-// TestBuildMemoryFlushPromptConfig_AgentUUIDPopulated asserts Fix A — the
+// TestBuildMemoryFlushPromptConfig_AgentUUIDPopulated asserts the
 // SystemPromptConfig returned by buildMemoryFlushPromptConfig carries the
-// AgentUUID. Historically missing; loop_history.go set it but memoryflush.go
-// did not, leading to identity drift (see PR #826 / brainstorm trap zone 2).
+// AgentUUID. loop_history.go always set it but memoryflush.go historically
+// did not, which would have caused identity drift in downstream DomainEvents
+// if AgentUUID ever reached the stable cache prefix.
 func TestBuildMemoryFlushPromptConfig_AgentUUIDPopulated(t *testing.T) {
 	u := uuid.New()
 	cfg := buildMemoryFlushPromptConfig(

internal/agent/router.go

@@ -194,10 +194,10 @@ func (r *Router) ListInfo() []AgentInfo {
 	return infos
 }
 
-// IsRunning checks if a specific agent is currently running (cached in router).
-// Ctx carries tenant scope — pre-fix (C6) this function did a bare lookup which
-// always returned false in tenant-scoped deployments, causing `agents.list` to
-// incorrectly report every live agent as idle.
+// IsRunning checks if a specific agent is currently running (cached in
+// router). Ctx carries tenant scope — without it, a bare lookup returns false
+// in tenant-scoped deployments because cache keys are stored as
+// `tenantID:agentKey` after resolution.
 func (r *Router) IsRunning(ctx context.Context, agentID string) bool {
 	cacheKey := agentCacheKey(ctx, agentID)
 	r.mu.RLock()

internal/agent/router_cache_canonicalize_test.go

@@ -36,10 +36,10 @@ func stubResolver(agentKey string) ResolverFunc {
 	}
 }
 
-// TestRouterGet_UUIDInputStoresCanonicalKey — Phase 2 FR-1.
-// When the caller passes a UUID-like string to Get(), the cache entry must
-// land under tenantID:agentKey (canonical), NOT tenantID:uuidStr.
-// Exercises the canonicalization path via a real resolver call.
+// TestRouterGet_UUIDInputStoresCanonicalKey verifies that when the caller
+// passes a UUID-like string to Get(), the cache entry lands under
+// tenantID:agentKey (canonical), NOT tenantID:uuidStr. Exercises the
+// canonicalization path via a real resolver call.
 func TestRouterGet_UUIDInputStoresCanonicalKey(t *testing.T) {
 	r := NewRouter()
 	r.SetResolver(stubResolver("goctech-leader"))
@@ -72,9 +72,9 @@ func TestRouterGet_UUIDInputStoresCanonicalKey(t *testing.T) {
 	}
 }
 
-// TestRouterGet_IdempotentCacheUnderKeyOrUUID — Phase 2 FR-4.
-// Calling Get() first with agent_key then with UUID should produce exactly
-// ONE cache entry — the canonical tenantID:agent_key.
+// TestRouterGet_IdempotentCacheUnderKeyOrUUID verifies that calling Get()
+// first with agent_key then with UUID produces exactly ONE cache entry — the
+// canonical tenantID:agent_key.
 func TestRouterGet_IdempotentCacheUnderKeyOrUUID(t *testing.T) {
 	r := NewRouter()
 	var resolveCount atomic.Int32
@@ -107,10 +107,10 @@ func TestRouterGet_IdempotentCacheUnderKeyOrUUID(t *testing.T) {
 	}
 }
 
-// TestRouterGet_UUIDCallerResolvesEveryTime — Phase 2 honest cost (H1).
-// A caller that keeps passing the UUID form never hits the canonical key on
-// read, so the resolver runs on every call. Document this behavior so future
-// refactors don't pretend the cache covers UUID inputs.
+// TestRouterGet_UUIDCallerResolvesEveryTime documents the honest cost of
+// canonicalization: a caller that keeps passing the UUID form never hits the
+// canonical key on read, so the resolver runs on every call. Pin this
+// behavior so future refactors don't pretend the cache covers UUID inputs.
 func TestRouterGet_UUIDCallerResolvesEveryTime(t *testing.T) {
 	r := NewRouter()
 	var resolveCount atomic.Int32

internal/agent/router_invalidate_collision_test.go

@@ -4,10 +4,10 @@ import (
 	"testing"
 )
 
-// TestInvalidateAgent_NoSubstringCollision — Phase 2 FR-2.
-// Before the fix, Router.Remove / InvalidateAgent used strings.HasSuffix
-// which would match "tenantX:sub-foo" when invalidating "foo". Verify
-// exact-segment match rejects substring collisions.
+// TestInvalidateAgent_NoSubstringCollision verifies exact-segment match
+// rejects substring collisions. A previous HasSuffix-based matcher would have
+// removed "tenantX:sub-foo" when invalidating "foo" — this regression guards
+// against that.
 func TestInvalidateAgent_NoSubstringCollision(t *testing.T) {
 	r := NewRouter()
 	r.agents["tenantX:foo"] = &agentEntry{}

internal/agent/router_isrunning_test.go

@@ -10,11 +10,11 @@ import (
 	"github.com/nextlevelbuilder/goclaw/internal/store"
 )
 
-// TestIsRunning_TenantScopedLookup — Phase 2 FR-5 (C6).
-// Router.IsRunning must accept ctx and lookup under tenant-scoped cache key.
-// Pre-fix: bare `r.agents[agentID]` lookup always returned false for any
-// tenant-scoped deployment, so the WS `agents.list` response incorrectly
-// showed `isRunning: false` for every live agent.
+// TestIsRunning_TenantScopedLookup asserts that Router.IsRunning accepts ctx
+// and looks up under the tenant-scoped cache key. A previous bare
+// `r.agents[agentID]` lookup always returned false for any tenant-scoped
+// deployment, so the WS `agents.list` response incorrectly showed every live
+// agent as `isRunning: false`.
 func TestIsRunning_TenantScopedLookup(t *testing.T) {
 	r := NewRouter()
 	tenantA := uuid.New()

internal/eventbus/bus_impl.go

@@ -64,7 +64,6 @@ func (b *busImpl) Publish(event DomainEvent) {
 		return
 	}
 	// Publish-time observer: warns on non-UUID AgentID drift without blocking.
-	// See validate_agent_id.go for rationale (PR #826 regression safety net).
 	validateAgentID(event)
 	select {
 	case b.queue <- event:

internal/eventbus/validate_agent_id.go

@@ -6,19 +6,17 @@ import (
 	"github.com/google/uuid"
 )
 
-// validateAgentID is a publish-time observer that logs a warning when a DomainEvent
-// carries a non-UUID AgentID. It does NOT block the publish — observability only.
+// validateAgentID is a publish-time observer that logs a warning when a
+// DomainEvent carries a non-UUID AgentID. It does NOT block the publish —
+// observability only. Acts as a safety net catching future drift before the
+// event reaches a consumer that parses the field as a UUID and queries the DB
+// with it.
 //
-// Motivation: PR #826 fixed multiple call sites that passed agent_key strings
-// (e.g. "goctech-leader") into DomainEvent.AgentID, silently corrupting downstream
-// consumers. This helper acts as a safety net to catch any future drift BEFORE
-// the event reaches a consumer that parses the field as a UUID.
+// Log field name is `non_uuid_agent_id` — intentionally distinct from the
+// standard `agent_id` field used elsewhere — to avoid collision with
+// observability tooling that parses `agent_id` as a UUID.
 //
-// Log field name: `non_uuid_agent_id` (intentionally distinct from the standard
-// `agent_id` field used elsewhere) to avoid collision with observability tooling
-// that parses `agent_id` as a UUID. See red-team finding H6.
-//
-// See docs/agent-identity-conventions.md (Phase 6) for the convention.
+// See docs/agent-identity-conventions.md for the full convention.
 func validateAgentID(event DomainEvent) {
 	if event.AgentID == "" {
 		return // legitimate team-owned, tenant-scoped, or anonymous event

internal/eventbus/validate_agent_id_test.go

@@ -76,8 +76,9 @@ func TestValidateAgentID_NonUUIDWarns(t *testing.T) {
 }
 
 func TestValidateAgentID_DistinctFieldName_NoCollision(t *testing.T) {
-	// H6 mitigation: the field name must NOT be `agent_id` (which downstream
-	// observability tooling parses as UUID) — it must be `non_uuid_agent_id`.
+	// The log field name must NOT be `agent_id` (which downstream
+	// observability tooling parses as UUID) — it must be `non_uuid_agent_id`
+	// so the warning never collides with valid UUID-typed agent_id fields.
 	buf, restore := captureSlog(t)
 	defer restore()
 

internal/gateway/methods/agent_links.go

@@ -412,7 +412,7 @@ func (m *AgentLinksMethods) invalidateLinkAgentsByID(ctx context.Context, source
 // its canonical UUID via a DB lookup. Tenant-aware via
 // store.TenantIDFromContext(ctx) inside agentStore.GetByID/GetByKey. Prefer
 // resolveAgentUUIDCached in hot-path handlers to avoid the extra DB roundtrip.
-// See docs/agent-identity-conventions.md trap zone 5.
+// See docs/agent-identity-conventions.md.
 func resolveAgentUUID(ctx context.Context, agentStore store.AgentStore, keyOrID string) (uuid.UUID, error) {
 	if id, err := uuid.Parse(keyOrID); err == nil {
 		ag, err := agentStore.GetByID(ctx, id)
@@ -441,7 +441,7 @@ type agentUUIDProvider interface {
 // on cache miss or when the input is a UUID string (router cache keys are
 // canonicalized to `tenantID:agentKey`, so UUID inputs never hit the cache).
 // If router is nil, delegates straight to resolveAgentUUID.
-// See docs/agent-identity-conventions.md trap zone 5 and section 8.
+// See docs/agent-identity-conventions.md.
 func resolveAgentUUIDCached(ctx context.Context, router *agent.Router, agentStore store.AgentStore, keyOrID string) (uuid.UUID, error) {
 	// Fast path: input is agent_key and the agent is cached in the router.
 	if router != nil {

internal/gateway/methods/agent_resolve_cached_test.go

@@ -59,10 +59,11 @@ func TestResolveAgentUUIDCached_CacheMissFallsBack(t *testing.T) {
 	}
 }
 
-// TestResolveAgentUUIDCached_UUIDInputTakesDBPath — per Phase 2 H1, a caller
-// passing the UUID form is never cached under the raw UUID key, so the helper
-// must fall through to the DB path. Verifies this by checking the stub's
-// sentinel error surfaces.
+// TestResolveAgentUUIDCached_UUIDInputTakesDBPath verifies that a caller
+// passing the UUID form falls through to the DB path. Router cache entries
+// are canonicalized to `tenantID:agentKey`, so the raw UUID input never hits
+// the cache and the helper must delegate to the store stub (whose sentinel
+// error surfaces here).
 func TestResolveAgentUUIDCached_UUIDInputTakesDBPath(t *testing.T) {
 	r := agent.NewRouter()
 	stub := &errorAgentStore{err: errSentinelMiss}

internal/gateway/methods/channel_instances.go

@@ -23,10 +23,8 @@ var channelInstanceAllowed = map[string]bool{
 }
 
 // ChannelInstancesMethods handles channel instance CRUD via WebSocket RPC.
-//
-// NFR-2 exception (Phase 3 C2): agentStore is a new struct field required to
-// support agent_key input via resolveAgentUUIDCached. Constructor signature
-// expanded — single caller at cmd/gateway_channels_setup.go.
+// agentStore is held so the create/update handlers can resolve agent_key or
+// UUID input via resolveAgentUUIDCached.
 type ChannelInstancesMethods struct {
 	store      store.ChannelInstanceStore
 	agentStore store.AgentStore
@@ -127,9 +125,9 @@ func (m *ChannelInstancesMethods) handleCreate(ctx context.Context, client *gate
 		return
 	}
 
-	// Accept both agent_key and UUID via resolveAgentUUIDCached (Phase 3 FR-1).
-	// nil router: channel_instances methods are not wired to the router — falls
-	// back to a pure DB lookup which is acceptable given create is a rare op.
+	// Accept both agent_key and UUID via resolveAgentUUIDCached. Router is nil
+	// here because channel_instances methods are not wired to the router —
+	// falls back to a pure DB lookup, acceptable given create is a rare op.
 	agentID, err := resolveAgentUUIDCached(ctx, nil, m.agentStore, params.AgentID)
 	if err != nil {
 		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidID, "agent_id")))

internal/gateway/methods/teams_members.go

@@ -52,8 +52,8 @@ func (m *TeamsMethods) handleAddMember(ctx context.Context, client *gateway.Clie
 		return
 	}
 
-	// Resolve agent — accepts agent_key or UUID. H10 fix: return i18n error instead
-	// of leaking raw store error (was: "agent: " + err.Error()).
+	// Resolve agent — accepts agent_key or UUID. Return an i18n error on
+	// failure; never leak the raw store error string to WS clients.
 	ag, err := resolveAgentInfo(ctx, m.agentStore, params.Agent)
 	if err != nil {
 		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidID, "agent")))
@@ -135,7 +135,7 @@ func (m *TeamsMethods) handleRemoveMember(ctx context.Context, client *gateway.C
 		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidID, "teamId")))
 		return
 	}
-	// Phase 3 FR-1: accept agent_key or UUID via cache-aware resolver.
+	// Accept agent_key or UUID via cache-aware resolver.
 	agentID, err := resolveAgentUUIDCached(ctx, m.agentRouter, m.agentStore, params.AgentID)
 	if err != nil {
 		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidID, "agentId")))

internal/gateway/methods/teams_tasks_mutations.go

@@ -151,7 +151,7 @@ func (m *TeamsMethods) handleTaskAssign(ctx context.Context, client *gateway.Cli
 		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidID, "taskId")))
 		return
 	}
-	// Phase 3 FR-1: accept agent_key or UUID for assignee.
+	// Accept agent_key or UUID for assignee.
 	agentID, err := resolveAgentUUIDCached(ctx, m.agentRouter, m.agentStore, params.AgentID)
 	if err != nil {
 		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidID, "agentId")))

internal/http/vault_handler_upload.go

@@ -41,10 +41,11 @@ func (h *VaultHandler) handleUpload(w http.ResponseWriter, r *http.Request) {
 	agentIDStr := r.FormValue("agent_id")
 	teamIDStr := r.FormValue("team_id")
 
-	// Boundary UUID validation — Fix B (Phase 1, H9 mitigation).
-	// validateTeamMembership short-circuits on owner role + lite edition (nil teamAccess),
-	// leaving downstream `parseUUIDOrNil(*doc.TeamID)` as a silent-nil trap. Validate at
-	// the HTTP boundary so bad form input is rejected before any store call or event publish.
+	// Boundary UUID validation. validateTeamMembership below short-circuits
+	// on owner role + lite edition (nil teamAccess), which would leave a
+	// downstream parseUUIDOrNil(*doc.TeamID) call as a silent-nil trap.
+	// Validate at the HTTP boundary so bad form input is rejected before any
+	// store call or event publish.
 	// See docs/agent-identity-conventions.md.
 	if agentIDStr != "" {
 		if _, err := uuid.Parse(agentIDStr); err != nil {

internal/http/vault_handler_upload_test.go

@@ -60,10 +60,10 @@ func assertBadRequest(t *testing.T, rr *httptest.ResponseRecorder, wantFragment
 	}
 }
 
-// TestHandleUpload_InvalidAgentIDReturns400 asserts Phase 1 Fix B — bad form
-// `agent_id` is rejected at the HTTP boundary before any store call.
-// Closes the owner/lite edition gap where validateTeamMembership would skip
-// the UUID check.
+// TestHandleUpload_InvalidAgentIDReturns400 asserts bad form `agent_id` is
+// rejected at the HTTP boundary before any store call. Closes the
+// owner / lite edition gap where validateTeamMembership would skip the UUID
+// check.
 func TestHandleUpload_InvalidAgentIDReturns400(t *testing.T) {
 	h := &VaultHandler{} // no store wired — boundary check runs first
 	req := buildUploadRequest(t, map[string]string{
@@ -76,9 +76,10 @@ func TestHandleUpload_InvalidAgentIDReturns400(t *testing.T) {
 	assertBadRequest(t, rr, "invalid agent_id")
 }
 
-// TestHandleUpload_InvalidTeamIDReturns400 asserts Phase 1 Fix B for team_id.
-// This is the hole validateTeamMembership leaves open: it short-circuits on
-// owner role (line 57) and nil teamAccess (line 60), never parsing the UUID.
+// TestHandleUpload_InvalidTeamIDReturns400 asserts bad form `team_id` is
+// rejected at the HTTP boundary. validateTeamMembership short-circuits on
+// owner role and on nil teamAccess (lite edition), never parsing the UUID —
+// the boundary check closes that hole.
 func TestHandleUpload_InvalidTeamIDReturns400(t *testing.T) {
 	h := &VaultHandler{} // lite edition: teamAccess is nil
 	req := buildUploadRequest(t, map[string]string{
@@ -91,10 +92,9 @@ func TestHandleUpload_InvalidTeamIDReturns400(t *testing.T) {
 	assertBadRequest(t, rr, "invalid team_id")
 }
 
-// TestHandleUpload_InvalidAgentID_OwnerContext asserts the boundary check fires
-// regardless of role. Pre-Fix-B, validateTeamMembership skipped the check for
-// owner role — but agent_id was never validated at all upstream, so the UUID
-// hole also existed for admins. Boundary check closes it for every caller.
+// TestHandleUpload_InvalidAgentID_OwnerContext asserts the boundary check
+// fires regardless of role. The UUID hole existed for every caller — the
+// boundary check ignores role entirely.
 func TestHandleUpload_InvalidAgentID_OwnerContext(t *testing.T) {
 	h := &VaultHandler{}
 	req := buildUploadRequest(t, map[string]string{

internal/store/pg/memory_docs.go

@@ -482,7 +482,7 @@ func (s *PGMemoryStore) Close() error { return nil }
 // either corrupt data or hide bugs as empty reads / zero-row updates. FK
 // constraints reject bad writes at the driver layer, but errors there come
 // back as cryptic PG 23503 — parseUUID catches them upstream with a clean
-// Go error. See docs/agent-identity-conventions.md trap zone 3.
+// Go error. See docs/agent-identity-conventions.md.
 func parseUUID(s string) (uuid.UUID, error) {
 	id, err := uuid.Parse(s)
 	if err != nil {
@@ -496,7 +496,7 @@ func parseUUID(s string) (uuid.UUID, error) {
 // SELECT WHERE paths where a no-match (empty result) is the correct
 // semantics on bad input. Do NOT use for writes, updates, deletes, or any
 // SELECT where an empty result would hide a bug. Prefer parseUUID for new
-// code. See docs/agent-identity-conventions.md trap zone 3.
+// code. See docs/agent-identity-conventions.md.
 func parseUUIDOrNil(s string) uuid.UUID {
 	id, err := uuid.Parse(s)
 	if err != nil {

internal/store/pg/vault_documents.go

@@ -57,9 +57,8 @@ func (s *PGVaultStore) Close() error { return nil }
 // optAgentUUID converts a nullable *string agent_id to *uuid.UUID for SQL.
 // Returns (nil, nil) when the input is nil or empty — a legitimate SQL NULL.
 // Returns (nil, error) on a non-empty, non-UUID input — propagating the error
-// prevents silent-nil writes that would otherwise corrupt data (pre-Phase-4
-// this helper silently returned nil on garbage input).
-// See docs/agent-identity-conventions.md (Phase 6).
+// prevents silent-nil writes that would otherwise corrupt data.
+// See docs/agent-identity-conventions.md.
 func optAgentUUID(agentID *string) (*uuid.UUID, error) {
 	if agentID == nil || *agentID == "" {
 		return nil, nil