Skip to content

Commit cb77004

Browse files
committed
fix: materialize-skills uses template name for pool instances
The stage-2 PreStart hook for skill materialization passed the pool instance's qualified name (e.g. rig/furiosa from polecat's namepool) to `gc internal materialize-skills --agent`. That command calls resolveAgentIdentity, which cannot map a namepool member back to its pool template — it treats rig/furiosa as an unknown agent and exits with code 1, failing pre_start[1] on every polecat start. Fix: pass templateNameFor(cfgAgent, qualifiedName) which returns cfgAgent.PoolName (the template's qualified name) for pool instances and qualifiedName for singletons. Skills are per-template, not per-instance; all members of a pool share the template's catalog. Regression exposed by tier-C acceptance: TestGastown_PolecatImplementsRefineryMerges on v0.15.1-rc1. Adds TestResolveTemplatePoolInstanceMaterializeUsesTemplateName as a focused unit regression so a future refactor can't silently reintroduce the bug without tripping CI in milliseconds rather than 20 minutes.
1 parent 6f957ab commit cb77004

2 files changed

Lines changed: 121 additions & 1 deletion

File tree

cmd/gc/template_resolve.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,14 @@ func resolveTemplate(p *agentBuildParams, cfgAgent *config.Agent, qualifiedName
351351
if len(desired) > 0 {
352352
fpExtra = mergeSkillFingerprintEntries(fpExtra, desired)
353353
if canonWorkDir != scopeRoot {
354-
expandedPreStart = appendMaterializeSkillsPreStart(expandedPreStart, qualifiedName, workDir)
354+
// Pool instances inherit their skill catalog from the
355+
// template, not the instance — namepool members (e.g.
356+
// repo/furiosa from polecat) are not resolvable as
357+
// standalone agents by `gc internal materialize-skills`.
358+
// templateNameFor returns cfgAgent.PoolName for pool
359+
// instances and qualifiedName for singletons.
360+
materializeAgent := templateNameFor(cfgAgent, qualifiedName)
361+
expandedPreStart = appendMaterializeSkillsPreStart(expandedPreStart, materializeAgent, workDir)
355362
}
356363
}
357364
}

cmd/gc/template_resolve_skills_test.go

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,3 +316,116 @@ func TestResolveTemplateAppendsAssignedSkillsPrompt(t *testing.T) {
316316
}
317317
})
318318
}
319+
320+
// TestResolveTemplatePoolInstanceMaterializeUsesTemplateName is the
321+
// v0.15.1-rc1 → rc2 regression. Pool instances (especially namepool-
322+
// themed ones like polecat → furiosa) must route the stage-2 PreStart
323+
// `gc internal materialize-skills --agent` flag at the TEMPLATE's
324+
// qualified name, not the instance's. The materialize-skills command
325+
// resolves the agent via resolveAgentIdentity, which cannot map a
326+
// namepool member (`rig/furiosa`) back to its template (`rig/polecat`)
327+
// — it treats `rig/furiosa` as an unknown agent and exits with code 1,
328+
// failing pre_start[1] on every polecat start in tier C. See
329+
// TestGastown_PolecatImplementsRefineryMerges.
330+
func TestResolveTemplatePoolInstanceMaterializeUsesTemplateName(t *testing.T) {
331+
cityPath := t.TempDir()
332+
writeTemplateResolveCityConfig(t, cityPath, "file")
333+
if err := os.WriteFile(filepath.Join(cityPath, "pack.toml"),
334+
[]byte("[pack]\nname = \"s\"\nversion = \"0.1.0\"\nschema = 2\n"), 0o644); err != nil {
335+
t.Fatal(err)
336+
}
337+
skillDir := filepath.Join(cityPath, "skills", "plan")
338+
if err := os.MkdirAll(skillDir, 0o755); err != nil {
339+
t.Fatal(err)
340+
}
341+
if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"),
342+
[]byte("---\nname: plan\ndescription: test\n---\nbody\n"), 0o644); err != nil {
343+
t.Fatal(err)
344+
}
345+
sharedCat, err := materialize.LoadCityCatalog(filepath.Join(cityPath, "skills"))
346+
if err != nil {
347+
t.Fatal(err)
348+
}
349+
350+
// Namepool-themed instance: template is "rig/polecat" (PoolName),
351+
// concrete instance name is "furiosa", Dir="rig".
352+
// WorkDir != scope root so stage-2 PreStart injection fires.
353+
instance := &config.Agent{
354+
Name: "furiosa",
355+
Dir: "rig",
356+
Scope: "rig",
357+
Provider: "claude",
358+
WorkDir: ".gc/worktrees/rig/polecats/furiosa",
359+
PoolName: "rig/polecat",
360+
}
361+
362+
params := &agentBuildParams{
363+
cityName: "city",
364+
cityPath: cityPath,
365+
workspace: &config.Workspace{Provider: "claude"},
366+
providers: map[string]config.ProviderSpec{
367+
"claude": {Command: "echo", PromptMode: "none"},
368+
},
369+
lookPath: func(string) (string, error) { return "/bin/echo", nil },
370+
fs: fsys.OSFS{},
371+
rigs: []config.Rig{{Name: "rig", Path: filepath.Join(cityPath, "rig")}},
372+
beaconTime: time.Unix(0, 0),
373+
beadNames: make(map[string]string),
374+
stderr: io.Discard,
375+
skillCatalog: &sharedCat,
376+
sessionProvider: "tmux",
377+
}
378+
379+
tp, err := resolveTemplate(params, instance, instance.QualifiedName(), nil)
380+
if err != nil {
381+
t.Fatalf("resolveTemplate: %v", err)
382+
}
383+
384+
var materializeCmd string
385+
for _, entry := range tp.Hints.PreStart {
386+
if strings.Contains(entry, "internal materialize-skills") {
387+
materializeCmd = entry
388+
break
389+
}
390+
}
391+
if materializeCmd == "" {
392+
t.Fatalf("expected stage-2 materialize-skills PreStart entry; PreStart=%v", tp.Hints.PreStart)
393+
}
394+
395+
// The --agent flag must carry the TEMPLATE qualified name, not the
396+
// instance. `gc internal materialize-skills --agent rig/furiosa`
397+
// exits 1 with "unknown agent" because resolveAgentIdentity can't
398+
// walk a namepool member back to its pool template.
399+
// shellquote.Join emits bare (unquoted) tokens when no escaping is
400+
// needed, so match on the raw substring after --agent.
401+
if !strings.Contains(materializeCmd, "--agent rig/polecat") {
402+
t.Errorf("materialize-skills --agent flag should carry template name rig/polecat; got: %q", materializeCmd)
403+
}
404+
if strings.Contains(materializeCmd, "--agent rig/furiosa") {
405+
t.Errorf("materialize-skills --agent flag must NOT carry namepool instance name rig/furiosa; got: %q", materializeCmd)
406+
}
407+
408+
// Non-pool singleton: cfgAgent.PoolName is empty, so the cmd carries
409+
// the agent's own qualified name. Guards against over-correction
410+
// where templateNameFor's fallback breaks non-pool cases.
411+
singleton := &config.Agent{
412+
Name: "mayor",
413+
Scope: "city",
414+
Provider: "claude",
415+
WorkDir: ".gc/agents/mayor",
416+
}
417+
tp2, err := resolveTemplate(params, singleton, singleton.QualifiedName(), nil)
418+
if err != nil {
419+
t.Fatalf("resolveTemplate singleton: %v", err)
420+
}
421+
var singletonCmd string
422+
for _, entry := range tp2.Hints.PreStart {
423+
if strings.Contains(entry, "internal materialize-skills") {
424+
singletonCmd = entry
425+
break
426+
}
427+
}
428+
if singletonCmd != "" && !strings.Contains(singletonCmd, "--agent mayor") {
429+
t.Errorf("singleton materialize-skills should carry own qualified name mayor; got: %q", singletonCmd)
430+
}
431+
}

0 commit comments

Comments
 (0)