diff --git a/engines/temporal/internal/temporal/workflows/agentrun_name_test.go b/engines/temporal/internal/temporal/workflows/agentrun_name_test.go new file mode 100644 index 0000000..32d0562 --- /dev/null +++ b/engines/temporal/internal/temporal/workflows/agentrun_name_test.go @@ -0,0 +1,62 @@ +package workflows + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// Internal (package workflows, not workflows_test) because newAgentRunName +// is unexported -- this pins the exact bug that reached production: a real +// agent ID ("claude-code-swe-agent", 21 bytes) combined with the +// "agentrun-"/uuid scaffolding is 67 bytes, over Kubernetes' 63-byte label +// limit, and core-controller's reconciler rejects the Job it tries to create +// with that name forever. e2e coverage never caught it because its stand-in +// agent ("stub-agent", 10 bytes) happens to fit. +func TestNewAgentRunName_FitsWithinK8sLabelLimit(t *testing.T) { + for _, agentID := range []string{ + "stub-agent", + "claude-code-swe-agent", + "opencode-swe-agent", + strings.Repeat("a", 100), // pathological: far longer than any real agent id + } { + name := newAgentRunName(agentID) + require.LessOrEqualf(t, len(name), k8sNameMaxBytes, "agentID=%q produced %q (%d bytes)", agentID, name, len(name)) + require.True(t, strings.HasPrefix(name, "agentrun-"), "name=%q", name) + } +} + +func TestNewAgentRunName_ShortIDUnaffected(t *testing.T) { + name := newAgentRunName("stub-agent") + require.True(t, strings.HasPrefix(name, "agentrun-stub-agent-"), "name=%q", name) + // uuid.NewString() is always 36 bytes -- the suffix must survive intact, + // only a too-long agent ID ever gets truncated. + require.Len(t, name, len("agentrun-stub-agent-")+36) +} + +func TestNewAgentRunName_LongIDTruncatesIDNotUUID(t *testing.T) { + agentID := "claude-code-swe-agent" + name := newAgentRunName(agentID) + require.LessOrEqual(t, len(name), k8sNameMaxBytes) + + // The trailing 36 bytes must be an intact uuid -- truncation must never + // eat into it, or two runs could collide. + suffix := name[len(name)-36:] + require.Len(t, suffix, 36) + require.Regexp(t, `^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`, suffix) + + // The agent ID portion was shortened, not the fixed scaffolding. + middle := strings.TrimSuffix(strings.TrimPrefix(name, "agentrun-"), "-"+suffix) + require.Less(t, len(middle), len(agentID)) + require.True(t, strings.HasPrefix(agentID, middle), "truncated portion %q must be a prefix of %q", middle, agentID) +} + +func TestNewAgentRunName_UniqueAcrossCalls(t *testing.T) { + agentID := "claude-code-swe-agent" + names := map[string]bool{} + for i := 0; i < 100; i++ { + names[newAgentRunName(agentID)] = true + } + require.Len(t, names, 100, "each call must produce a distinct name") +} diff --git a/engines/temporal/internal/temporal/workflows/bridged_agent_workflow.go b/engines/temporal/internal/temporal/workflows/bridged_agent_workflow.go index f7a78db..2dcda0d 100644 --- a/engines/temporal/internal/temporal/workflows/bridged_agent_workflow.go +++ b/engines/temporal/internal/temporal/workflows/bridged_agent_workflow.go @@ -82,7 +82,7 @@ func BridgedAgentWorkflow(ctx workflow.Context, in AgentWorkflowInput) error { var runID string if err := workflow.SideEffect(ctx, func(workflow.Context) any { // Also the protocol's agent_run_id, and therefore the NATS subjects. - return "agentrun-" + in.Agent.ID + "-" + uuid.NewString() + return newAgentRunName(in.Agent.ID) }).Get(&runID); err != nil { return fail("id_error", err.Error()) } @@ -224,6 +224,44 @@ func BridgedAgentWorkflow(ctx workflow.Context, in AgentWorkflowInput) error { } } +// k8sNameMaxBytes is the RFC 1123 label-value limit. This run name becomes a +// k8s AgentRun/Job name AND, via core-controller's reconciler (which uses it +// directly as the Job name), the API server's auto-added `job-name` label — +// so it must fit the LABEL limit (63 bytes), the stricter of the two, even +// though a bare resource name alone could run to 253. +const k8sNameMaxBytes = 63 + +// newAgentRunName builds this run's k8s-facing name (also the protocol's +// agent_run_id, and therefore the NATS subjects), truncating the agent ID — +// never the uuid — when the natural "agentrun--" form would +// exceed k8sNameMaxBytes. +// +// This is not a hypothetical: "agentrun-" (9) + "claude-code-swe-agent" (21) +// + "-" (1) + a uuid.NewString() (36) is 67 bytes, already over the limit +// with today's actual agent ID -- core-controller's reconciler then rejects +// the Job it tries to create with "must be no more than 63 bytes" and the run +// never progresses past a bare AgentRun CR, retrying forever. e2e coverage +// missed this because its stand-in agent ("stub-agent", 10 chars) happens to +// fit; nothing this short-sightedly assumes a length bound in production. +func newAgentRunName(agentID string) string { + suffix := uuid.NewString() + name := "agentrun-" + agentID + "-" + suffix + if len(name) <= k8sNameMaxBytes { + return name + } + // Shorten the agent ID portion only -- the uuid is what makes this + // unique, and truncating IT would reintroduce the collision risk the + // full form exists to avoid. + budget := k8sNameMaxBytes - len("agentrun-") - len("-") - len(suffix) + if budget < 0 { + budget = 0 + } + if budget > len(agentID) { + budget = len(agentID) + } + return "agentrun-" + agentID[:budget] + "-" + suffix +} + // handleBridgedToolCall runs a Tool on a sub-agent's behalf and answers the // correlated callId. // diff --git a/engines/temporal/internal/temporal/workflows/bridged_agent_workflow_test.go b/engines/temporal/internal/temporal/workflows/bridged_agent_workflow_test.go index f6e80cf..a053013 100644 --- a/engines/temporal/internal/temporal/workflows/bridged_agent_workflow_test.go +++ b/engines/temporal/internal/temporal/workflows/bridged_agent_workflow_test.go @@ -68,7 +68,12 @@ func TestBridgedAgentRunsToAFinalReply(t *testing.T) { require.Len(t, le.agentRunLaunches, 1) require.Equal(t, "claude-code-swe-agent", le.agentRunLaunches[0].AgentRef) require.Equal(t, "add retry logic to the fetcher", le.agentRunLaunches[0].Goal) - require.Contains(t, le.agentRunLaunches[0].RunID, "claude-code-swe-agent") + // A prefix, not the full id: newAgentRunName truncates it to keep + // "agentrun--" within Kubernetes' 63-byte label limit, and + // "claude-code-swe-agent" is already long enough that the full form (67 + // bytes) doesn't fit — see agentrun_name_test.go for the exact bound. + require.Contains(t, le.agentRunLaunches[0].RunID, "claude-code-swe") + require.LessOrEqual(t, len(le.agentRunLaunches[0].RunID), 63) // Narration reached the user. require.Contains(t, result.Meta.Narration, "clone: cloning the repo")