From 8c657116700172678a5afb17e9f634f37b4e08ad Mon Sep 17 00:00:00 2001 From: hongming-ceo-delegated Date: Wed, 24 Jun 2026 11:43:02 -0700 Subject: [PATCH] fix(concierge): consolidate identity prompt on system-prompt.md (kill filename split) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The platform-agent template shipped its system prompt as prompts/concierge.md (config.yaml prompt_files), but core assumed system-prompt.md in two places — a divergence that left every prod concierge with a broken identity: 1. substituteConciergeName keyed on configFiles["system-prompt.md"] → the real prompts/concierge.md never had {{CONCIERGE_NAME}} filled → the concierge ran with a literal placeholder (observed live on test2). 2. conciergeIdentityPresent probed only /configs/system-prompt.md → a correctly seeded concierge (prompts/concierge.md, no system-prompt.md) read as "vanilla" → MaybeProvisionPlatformAgentOnBoot restarted it on every CP boot. Consolidate on system-prompt.md (the SSOT filename ordinary workspaces, the runtime fallback, and core's probe already share) and make core robust so this cannot silently re-break: - substituteConciergeName now follows the {{CONCIERGE_NAME}} placeholder across ALL delivered config files, not a hardcoded filename (no-op where absent). - conciergeIdentityPresent checks the runtime's full candidate set (system-prompt.md, then legacy prompts/concierge.md) → transition-safe for existing concierges until they re-provision. Companion template change flips molecule-ai-workspace-template-platform-agent to ship system-prompt.md. Prove-fail tests added for both paths (fail against pre-fix source). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../internal/handlers/platform_agent.go | 71 +++++++++++----- .../internal/handlers/platform_agent_test.go | 83 +++++++++++++++++++ 2 files changed, 135 insertions(+), 19 deletions(-) diff --git a/workspace-server/internal/handlers/platform_agent.go b/workspace-server/internal/handlers/platform_agent.go index 29426c5f..f6a719d1 100644 --- a/workspace-server/internal/handlers/platform_agent.go +++ b/workspace-server/internal/handlers/platform_agent.go @@ -283,21 +283,32 @@ func (h *WorkspaceHandler) applyConciergeProvisionConfig( // 1. Platform-MCP env (org-admin token + platform URL + org id). conciergePlatformMCPEnv(envVars) - // 2. {{CONCIERGE_NAME}} substitution in the template-delivered - // system-prompt.md. The runtime's build_system_prompt does NOT - // template prompt files, so we do the minimal per-instance - // substitution here at provision time. The template's - // prompts/concierge.md carries the {{CONCIERGE_NAME}} placeholder - // where the per-instance name goes. Idempotent: a re-provision - // re-runs the substitution; the result is stable. + // 2. {{CONCIERGE_NAME}} substitution across EVERY template-delivered + // config file that carries the placeholder. The runtime's + // build_system_prompt does NOT template prompt files, so we do the + // per-instance substitution here at provision time. + // + // We follow the PLACEHOLDER, not a hardcoded filename. The template + // ships its system prompt as prompts/concierge.md (per config.yaml + // `prompt_files`), NOT system-prompt.md — so the old single-file + // ("system-prompt.md") substitution silently no-opped and every prod + // concierge ran with a literal {{CONCIERGE_NAME}} and a half-applied + // identity. Substituting wherever the marker appears cannot miss when + // a template renames or splits its prompt sources, subsumes the legacy + // system-prompt.md layout (no regression), and is a no-op on files + // without the placeholder (safe + idempotent across re-provision). if configFiles == nil { configFiles = map[string][]byte{} } - if prompt, ok := configFiles["system-prompt.md"]; ok { - configFiles["system-prompt.md"] = substituteConciergeName(prompt, name) + substituted := 0 + for path, content := range configFiles { + if strings.Contains(string(content), conciergeNamePlaceholder) { + configFiles[path] = substituteConciergeName(content, name) + substituted++ + } } - log.Printf("Provisioner: applied platform-agent env + {{CONCIERGE_NAME}} substitution for %s (name=%q, %d config file(s))", - workspaceID, name, len(configFiles)) + log.Printf("Provisioner: applied platform-agent env + {{CONCIERGE_NAME}} substitution for %s (name=%q, %d/%d config file(s) substituted)", + workspaceID, name, substituted, len(configFiles)) return configFiles } @@ -823,11 +834,27 @@ func MaybeProvisionPlatformAgentOnBoot(ctx context.Context, database *sql.DB, pr go restartByID(id) } +// conciergePromptCandidates are the on-box system-prompt files a seeded +// concierge may carry, in the SAME newest-first order the runtime's prompt +// resolver uses. The platform-agent template now ships system-prompt.md (the +// SSOT filename shared with ordinary workspaces); prompts/concierge.md is the +// legacy location existing concierges still carry until they re-provision. The +// identity probe checks ALL of them so a correctly-seeded concierge on either +// layout is never misjudged "vanilla" (which would trigger a spurious boot +// restart on every CP boot — the second symptom of the old filename split). +var conciergePromptCandidates = []string{"system-prompt.md", "prompts/concierge.md"} + +// conciergeIdentityMarker is the stable phrase the concierge system prompt +// carries ("… — the Org Concierge"). Its presence in any candidate file means +// the per-instance identity overlay has been applied. +const conciergeIdentityMarker = "Org Concierge" + // conciergeIdentityPresent reports whether the running concierge container -// already carries the seeded identity (a non-empty /configs/system-prompt.md). -// Used to decide whether a running-but-vanilla concierge needs a one-shot -// restart to pick up the overlay. Best-effort: on a probe error or an empty -// file it returns false (so the safe action — re-seed via restart — is taken). +// already carries the seeded identity (a system-prompt file bearing the +// concierge identity marker). Used to decide whether a running-but-vanilla +// concierge needs a one-shot restart to pick up the overlay. Best-effort: if +// NO candidate file carries the marker (probe error or vanilla content) it +// returns false (so the safe action — re-seed via restart — is taken). func conciergeIdentityPresent(ctx context.Context, prov localProvisionerIsRunning, id string) bool { reader, ok := prov.(interface { ExecRead(ctx context.Context, containerName, filePath string) ([]byte, error) @@ -837,11 +864,17 @@ func conciergeIdentityPresent(ctx context.Context, prov localProvisionerIsRunnin // that doesn't expose ExecRead. return true } - body, err := reader.ExecRead(ctx, provisioner.ContainerName(id), "/configs/system-prompt.md") - if err != nil { - return false + container := provisioner.ContainerName(id) + for _, name := range conciergePromptCandidates { + body, err := reader.ExecRead(ctx, container, "/configs/"+name) + if err != nil { + continue + } + if strings.Contains(string(body), conciergeIdentityMarker) { + return true + } } - return strings.Contains(string(body), "Org Concierge") + return false } // localProvisionerIsRunning is the minimal slice of the local Docker diff --git a/workspace-server/internal/handlers/platform_agent_test.go b/workspace-server/internal/handlers/platform_agent_test.go index 55e1cd81..776240f7 100644 --- a/workspace-server/internal/handlers/platform_agent_test.go +++ b/workspace-server/internal/handlers/platform_agent_test.go @@ -221,8 +221,50 @@ func (s *stubBootProvExec) ExecRead(_ context.Context, _ /*container*/, _ /*path return []byte(s.systemPrompt), nil } +// stubBootProvExecByPath is a path-aware ExecRead fake: it returns the body +// registered for a given /configs path (map zero-value "" for unregistered +// paths, which the probe treats as "no marker"). Lets the identity probe be +// exercised across the multiple candidate prompt filenames. +type stubBootProvExecByPath struct { + stubBootProv + files map[string]string // absolute /configs path -> content +} + +func (s *stubBootProvExecByPath) ExecRead(_ context.Context, _ /*container*/, path string) ([]byte, error) { + return []byte(s.files[path]), nil +} + const bootPlatformID = "11111111-2222-3333-4444-555555555555" +// TestConciergeIdentityPresent_ChecksBothPromptCandidates asserts the boot +// identity probe recognises the seeded identity in EITHER the new SSOT filename +// (system-prompt.md) or the legacy prompts/concierge.md location. The latter is +// the load-bearing transition case: before consolidation, the template shipped +// prompts/concierge.md and the probe only read system-prompt.md → it judged +// every seeded concierge "vanilla" → spurious restart on every boot. Fails +// against the pre-fix single-file probe. +func TestConciergeIdentityPresent_ChecksBothPromptCandidates(t *testing.T) { + const marker = "# You are Mia — the Org Concierge\n\nYou are an org orchestrator.\n" + cases := []struct { + name string + files map[string]string + want bool + }{ + {"new SSOT system-prompt.md carries identity", map[string]string{"/configs/system-prompt.md": marker}, true}, + {"legacy prompts/concierge.md carries identity (transition)", map[string]string{"/configs/prompts/concierge.md": marker}, true}, + {"vanilla system-prompt.md (no marker)", map[string]string{"/configs/system-prompt.md": "You are a helpful assistant.\n"}, false}, + {"nothing seeded on box", map[string]string{}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + prov := &stubBootProvExecByPath{files: tc.files} + if got := conciergeIdentityPresent(context.Background(), prov, bootPlatformID); got != tc.want { + t.Errorf("conciergeIdentityPresent = %v, want %v", got, tc.want) + } + }) + } +} + // TestMaybeProvisionPlatformAgentOnBoot_KicksOffWhenNotRunning: row present + // container not running ⇒ RestartByID is invoked with the platform agent's id. func TestMaybeProvisionPlatformAgentOnBoot_KicksOffWhenNotRunning(t *testing.T) { @@ -568,6 +610,47 @@ func TestApplyConciergeProvisionConfig_OnlyPlatformGetsOrgMCP(t *testing.T) { } }) + t.Run("substitutes {{CONCIERGE_NAME}} in the real prompts/concierge.md layout (not just system-prompt.md)", func(t *testing.T) { + // REGRESSION GUARD (prod incident 2026-06-24): the platform-agent + // template ships its system prompt as prompts/concierge.md (per + // config.yaml `prompt_files`), NOT system-prompt.md. The old + // substitution keyed on the literal "system-prompt.md" → it silently + // no-opped on every real concierge, which then ran with a literal + // {{CONCIERGE_NAME}} and a half-applied identity (observed live on + // test2). This asserts the placeholder is filled wherever it appears. + // Fails against the pre-fix single-filename code. + mock := setupTestDB(t) + mock.ExpectQuery(kindQuery).WithArgs("ws-concierge"). + WillReturnRows(sqlmock.NewRows([]string{"kind"}).AddRow("platform")) + mock.ExpectQuery(modelSelQuery).WithArgs("ws-concierge"). + WillReturnRows(sqlmock.NewRows([]string{"encrypted_value", "encryption_version"}). + AddRow([]byte("moonshot/kimi-k2.6"), 0)) + mock.ExpectQuery(providerSelQuery).WithArgs("ws-concierge"). + WillReturnRows(sqlmock.NewRows([]string{"encrypted_value", "encryption_version"})) + // recordDeclaredPlugin: privileged-plugin kind precheck (→platform) + declared INSERT. + mock.ExpectQuery(kindQuery).WithArgs("ws-concierge"). + WillReturnRows(sqlmock.NewRows([]string{"kind"}).AddRow("platform")) + mock.ExpectExec(declaredInsert). + WithArgs("ws-concierge", sqlmock.AnyArg(), sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(0, 1)) + env := map[string]string{} + cf := map[string][]byte{ + "config.yaml": []byte("runtime: claude-code\nprompt_files:\n - prompts/concierge.md\n"), + "prompts/concierge.md": []byte("# You are {{CONCIERGE_NAME}} — the Org Concierge\n\nYou are NOT a generic coding assistant; you are an org orchestrator.\n"), + } + out := h.applyConciergeProvisionConfig(context.Background(), "ws-concierge", "", cf, env, "Molecule AI Agent") + got := string(out["prompts/concierge.md"]) + if strings.Contains(got, conciergeNamePlaceholder) { + t.Errorf("{{CONCIERGE_NAME}} survived in prompts/concierge.md — the real template prompt was not substituted:\n%s", got) + } + if !strings.Contains(got, "Molecule AI Agent") { + t.Errorf("per-instance concierge name was not baked into prompts/concierge.md:\n%s", got) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } + }) + t.Run("idempotent re-provision on the platform agent (no double-substitution)", func(t *testing.T) { mock := setupTestDB(t) mock.ExpectQuery(kindQuery).WithArgs("ws-concierge"). -- 2.52.0