Compare commits
7 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| ebdeaeff73 | |||
| 13073cdedd | |||
| d79f28ace0 | |||
| 0655d5acf0 | |||
| 488018b156 | |||
| 8f9b6a73f9 | |||
| 3fc585b939 |
@@ -11,13 +11,21 @@ import { render, screen, fireEvent, cleanup, act } from "@testing-library/react"
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { TestConnectionButton } from "../ui/TestConnectionButton";
|
||||
import type { SecretGroup } from "@/types/secrets";
|
||||
import { validateSecret } from "@/lib/api/secrets";
|
||||
import { validateSecret, ApiError } from "@/lib/api/secrets";
|
||||
|
||||
// ─── Mock validateSecret ──────────────────────────────────────────────────────
|
||||
// vi.mock is hoisted, so validateSecret (imported above) refers to the mocked
|
||||
// namespace value once vi.mock runs. Use vi.mocked() to access it in tests.
|
||||
vi.mock("@/lib/api/secrets", () => ({
|
||||
validateSecret: vi.fn(),
|
||||
ApiError: class ApiError extends Error {
|
||||
status: number;
|
||||
constructor(status: number, message: string) {
|
||||
super(message);
|
||||
this.name = "ApiError";
|
||||
this.status = status;
|
||||
}
|
||||
},
|
||||
}));
|
||||
|
||||
// SecretGroup is a string literal type: 'github' | 'anthropic' | 'openrouter' | 'custom'
|
||||
@@ -102,7 +110,7 @@ describe("TestConnectionButton — state machine", () => {
|
||||
expect(screen.getByText("Permission denied")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("shows generic error message on unexpected exception", async () => {
|
||||
it("shows a connectivity message on a genuine network exception", async () => {
|
||||
vi.mocked(validateSecret).mockRejectedValue(new Error("timeout"));
|
||||
render(<TestConnectionButton provider={toGroup("anthropic")} secretValue="sk-..." />);
|
||||
|
||||
@@ -110,8 +118,23 @@ describe("TestConnectionButton — state machine", () => {
|
||||
await act(async () => { /* flush */ });
|
||||
|
||||
expect(screen.getByRole("alert")).toBeTruthy();
|
||||
// The error detail is hardcoded to "Connection timed out. Service may be down."
|
||||
expect(document.body.querySelector('[role="alert"]')?.textContent).toMatch(/timed out/i);
|
||||
// A real thrown network error → honest connectivity message (not a
|
||||
// fabricated "service down"); see internal#492.
|
||||
expect(document.body.querySelector('[role="alert"]')?.textContent).toMatch(
|
||||
/could not reach the validation service/i,
|
||||
);
|
||||
});
|
||||
|
||||
it("does not claim a timeout when the validate endpoint 404s (internal#492)", async () => {
|
||||
vi.mocked(validateSecret).mockRejectedValue(new ApiError(404, "Not Found"));
|
||||
render(<TestConnectionButton provider={toGroup("anthropic")} secretValue="sk-..." />);
|
||||
|
||||
fireEvent.click(screen.getByRole("button"));
|
||||
await act(async () => { /* flush */ });
|
||||
|
||||
const alert = document.body.querySelector('[role="alert"]')?.textContent ?? "";
|
||||
expect(alert).not.toMatch(/timed out/i);
|
||||
expect(alert).toMatch(/not available/i);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
import { useState, useCallback, useRef, useEffect } from 'react';
|
||||
import type { TestConnectionState, SecretGroup } from '@/types/secrets';
|
||||
import { validateSecret } from '@/lib/api/secrets';
|
||||
import { validateSecret, ApiError } from '@/lib/api/secrets';
|
||||
|
||||
interface TestConnectionButtonProps {
|
||||
provider: SecretGroup;
|
||||
@@ -55,9 +55,23 @@ export function TestConnectionButton({
|
||||
}
|
||||
onResult?.(result.valid);
|
||||
resetTimerRef.current = setTimeout(() => setState('idle'), RESET_DELAYS[nextState]!);
|
||||
} catch {
|
||||
} catch (err) {
|
||||
// Distinguish a real failure shape rather than always claiming a
|
||||
// timeout. A reachable server that answered with an HTTP status
|
||||
// (ApiError) did NOT time out — most commonly the validation route
|
||||
// is not available (404/501), which must not masquerade as
|
||||
// "service down". Only an actual thrown network/abort error is a
|
||||
// connectivity failure.
|
||||
setState('failure');
|
||||
setErrorDetail('Connection timed out. Service may be down.');
|
||||
if (err instanceof ApiError) {
|
||||
setErrorDetail(
|
||||
err.status === 404 || err.status === 501
|
||||
? 'Key validation is not available for this service yet. The key was not tested.'
|
||||
: `Could not verify key (server returned ${err.status}). Saving is unaffected.`,
|
||||
);
|
||||
} else {
|
||||
setErrorDetail('Could not reach the validation service. Check your connection and try again.');
|
||||
}
|
||||
onResult?.(false);
|
||||
resetTimerRef.current = setTimeout(() => setState('idle'), RESET_DELAYS.failure);
|
||||
}
|
||||
|
||||
@@ -28,8 +28,20 @@ const mockValidateSecret = vi.fn();
|
||||
|
||||
vi.mock("@/lib/api/secrets", () => ({
|
||||
validateSecret: (...args: unknown[]) => mockValidateSecret(...args),
|
||||
ApiError: class ApiError extends Error {
|
||||
status: number;
|
||||
constructor(status: number, message: string) {
|
||||
super(message);
|
||||
this.name = "ApiError";
|
||||
this.status = status;
|
||||
}
|
||||
},
|
||||
}));
|
||||
|
||||
// Re-import the mocked ApiError so test cases construct the same class the
|
||||
// component's `instanceof` check sees.
|
||||
import { ApiError } from "@/lib/api/secrets";
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
vi.clearAllMocks();
|
||||
@@ -201,8 +213,27 @@ describe("TestConnectionButton — failure path", () => {
|
||||
});
|
||||
|
||||
describe("TestConnectionButton — catch path", () => {
|
||||
it("shows 'Connection timed out' on network error", async () => {
|
||||
mockValidateSecret.mockRejectedValue(new Error("timeout"));
|
||||
it("does NOT claim a timeout when the validate endpoint 404s (regression: internal#492)", async () => {
|
||||
// The validate route is unimplemented on the server and returns a fast
|
||||
// 404. Before the fix this rendered the misleading hardcoded string
|
||||
// "Connection timed out. Service may be down." It must instead state
|
||||
// honestly that validation isn't available and the key was not tested.
|
||||
mockValidateSecret.mockRejectedValue(new ApiError(404, "Not Found"));
|
||||
render(
|
||||
<TestConnectionButton provider="anthropic" secretValue="sk-ant-xxx" />,
|
||||
);
|
||||
fireEvent.click(document.querySelector('button[type="button"]')!);
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
});
|
||||
expect(document.body.textContent).not.toContain("Connection timed out");
|
||||
expect(document.body.textContent).not.toContain("Service may be down");
|
||||
expect(document.body.textContent).toContain("not available");
|
||||
expect(document.body.textContent).toContain("not tested");
|
||||
});
|
||||
|
||||
it("reports a non-404 server error with its status, not a timeout", async () => {
|
||||
mockValidateSecret.mockRejectedValue(new ApiError(500, "Internal Server Error"));
|
||||
render(
|
||||
<TestConnectionButton provider="github" secretValue="ghp_xxx" />,
|
||||
);
|
||||
@@ -210,7 +241,20 @@ describe("TestConnectionButton — catch path", () => {
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
});
|
||||
expect(document.body.textContent).toContain("Connection timed out");
|
||||
expect(document.body.textContent).toContain("500");
|
||||
expect(document.body.textContent).not.toContain("Connection timed out");
|
||||
});
|
||||
|
||||
it("shows a connectivity message on a genuine network error", async () => {
|
||||
mockValidateSecret.mockRejectedValue(new Error("network down"));
|
||||
render(
|
||||
<TestConnectionButton provider="github" secretValue="ghp_xxx" />,
|
||||
);
|
||||
fireEvent.click(document.querySelector('button[type="button"]')!);
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
});
|
||||
expect(document.body.textContent).toContain("Could not reach the validation service");
|
||||
});
|
||||
|
||||
it("calls onResult(false) on network error", async () => {
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
package handlers
|
||||
|
||||
import "encoding/json"
|
||||
|
||||
// agent_card_reconcile.go — server-side repair for the fleet-wide
|
||||
// agent-card identity gap.
|
||||
//
|
||||
// Root cause: the runtime builds its AgentCard from config.name
|
||||
// (workspace/main.py:198), and config.name is read from the
|
||||
// CP-regenerated /configs/config.yaml whose `name:` field is the raw
|
||||
// workspace UUID — NOT the friendly name the operator sees. The friendly
|
||||
// name IS captured: POST /workspaces and PATCH /workspaces/:id (the
|
||||
// canvas Details tab) write it to the trusted workspaces.name DB column.
|
||||
// But /registry/register stores the runtime-supplied card verbatim
|
||||
// (registry.go: `agent_card = EXCLUDED.agent_card`), so the stored card
|
||||
// served at /.well-known/agent-card.json and returned to peers via
|
||||
// agent_card_url ends up with name = UUID, description = "", role = null.
|
||||
//
|
||||
// Fix shape (deliberately minimal, no contract weakening): when the
|
||||
// runtime-supplied card's `name` is empty or equals the workspace UUID
|
||||
// (the placeholder the runtime had no better value for), the PLATFORM —
|
||||
// not the agent — substitutes the friendly value from the trusted
|
||||
// workspaces row. Identity stays platform-controlled: the agent never
|
||||
// gains the ability to self-set its own name/role; the platform sources
|
||||
// it from the operator-controlled DB column. We only ever FILL gaps
|
||||
// (empty / UUID-placeholder); a card that already carries a real
|
||||
// friendly name is never downgraded.
|
||||
//
|
||||
// list_peers / the /registry/:id/peers endpoint already resolve display
|
||||
// names from workspaces.name directly (discovery.go / mcp_tools.go
|
||||
// `SELECT w.id, w.name, ...`), so peer_name in delivered message tags
|
||||
// was already correct — this fix closes the remaining surface: the
|
||||
// agent_card blob itself (canvas Agent Card / Skills view, peer
|
||||
// agent_card_url fetches, the well-known card).
|
||||
//
|
||||
// description / role degrade discovery the same way: an empty
|
||||
// description and null role give peers nothing to reason about. We
|
||||
// default description from the (now reconciled) name when blank and
|
||||
// role from workspaces.role when the operator set one.
|
||||
|
||||
// reconcileAgentCardIdentity patches identity gaps in a runtime-supplied
|
||||
// agent card from the trusted workspace DB row. It returns the
|
||||
// (possibly rewritten) card bytes and whether anything changed. On any
|
||||
// failure (malformed JSON, nothing to fill) it returns the input bytes
|
||||
// unchanged with changed=false so the caller can store them verbatim —
|
||||
// this is strictly no-worse-than-before, never a regression.
|
||||
//
|
||||
// Pure function: no DB / HTTP / globals, so it is exhaustively
|
||||
// unit-testable (agent_card_reconcile_test.go) without booting the
|
||||
// handler or a sqlmock.
|
||||
func reconcileAgentCardIdentity(card json.RawMessage, workspaceID, dbName, dbRole string) (json.RawMessage, bool) {
|
||||
var m map[string]any
|
||||
if err := json.Unmarshal(card, &m); err != nil || m == nil {
|
||||
// Malformed card — not this function's job to reject it (the
|
||||
// upsert stores it as-is and downstream readers handle bad
|
||||
// JSON). Return verbatim so byte-for-byte behaviour is
|
||||
// preserved on the failure path.
|
||||
return card, false
|
||||
}
|
||||
|
||||
changed := false
|
||||
|
||||
// name: fill only when empty or the UUID placeholder. A dbName that
|
||||
// is itself the UUID is a placeholder row (registry.go INSERT seeds
|
||||
// name = id before the canvas sets a friendly one) — not a friendly
|
||||
// name, so it is not an eligible source.
|
||||
cardName, _ := m["name"].(string)
|
||||
if (cardName == "" || cardName == workspaceID) &&
|
||||
dbName != "" && dbName != workspaceID {
|
||||
m["name"] = dbName
|
||||
changed = true
|
||||
}
|
||||
|
||||
// description: when blank, default to the (reconciled) name so peers
|
||||
// and the canvas Agent Card view have a non-empty human label
|
||||
// instead of "". Mirrors the runtime's own
|
||||
// `config.description or config.name` fallback (main.py:199) but
|
||||
// applied to the registry copy where the runtime's fallback was the
|
||||
// UUID.
|
||||
if desc, _ := m["description"].(string); desc == "" {
|
||||
if n, _ := m["name"].(string); n != "" && n != workspaceID {
|
||||
m["description"] = n
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
|
||||
// role: surface the operator-set workspaces.role when the card
|
||||
// carries none. Discovery (peer_role) and the canvas Role row read
|
||||
// workspaces.role directly; this just makes the standalone card
|
||||
// self-describing too. Never overwrite a role the card already has.
|
||||
if dbRole != "" {
|
||||
if r, ok := m["role"].(string); !ok || r == "" {
|
||||
m["role"] = dbRole
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
|
||||
if !changed {
|
||||
// No-op: return the original bytes untouched so callers that
|
||||
// compare/store get byte-identical input (re-marshalling would
|
||||
// reorder keys for no reason).
|
||||
return card, false
|
||||
}
|
||||
|
||||
out, err := json.Marshal(m)
|
||||
if err != nil {
|
||||
// Re-marshal of a map we just unmarshalled should never fail;
|
||||
// if it somehow does, fall back to the verbatim input rather
|
||||
// than storing nothing.
|
||||
return card, false
|
||||
}
|
||||
return out, true
|
||||
}
|
||||
@@ -0,0 +1,166 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestReconcileAgentCardIdentity covers the server-side backfill that
|
||||
// repairs the fleet-wide agent-card identity gap (internal#XXX): the
|
||||
// runtime POSTs /registry/register with agent_card.name = the workspace
|
||||
// UUID (because the CP-regenerated /configs/config.yaml sets name: <uuid>)
|
||||
// while the trusted workspaces.name DB column — the value the canvas
|
||||
// Details tab shows and lets the operator edit — holds the friendly
|
||||
// name ("Claude Code Agent"). The platform reconciles them from the DB
|
||||
// row (NOT from the agent — identity stays platform-controlled, not
|
||||
// self-mutable).
|
||||
func TestReconcileAgentCardIdentity(t *testing.T) {
|
||||
const wsID = "3b81321b-1ec7-488c-96f7-72c42a968da6"
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
card string
|
||||
dbName string
|
||||
dbRole string
|
||||
wantName string
|
||||
wantDesc string
|
||||
wantRole string
|
||||
wantChanged bool
|
||||
}{
|
||||
{
|
||||
name: "name is the workspace UUID — backfill from DB",
|
||||
card: `{"name":"3b81321b-1ec7-488c-96f7-72c42a968da6","description":"","capabilities":{"streaming":true}}`,
|
||||
dbName: "Claude Code Agent",
|
||||
dbRole: "",
|
||||
wantName: "Claude Code Agent",
|
||||
wantDesc: "Claude Code Agent",
|
||||
wantRole: "",
|
||||
wantChanged: true,
|
||||
},
|
||||
{
|
||||
name: "empty name — backfill from DB",
|
||||
card: `{"name":"","description":"x"}`,
|
||||
dbName: "ops-agent",
|
||||
dbRole: "sre",
|
||||
wantName: "ops-agent",
|
||||
wantDesc: "x",
|
||||
wantRole: "sre",
|
||||
wantChanged: true,
|
||||
},
|
||||
{
|
||||
name: "role null in card, DB has role — backfill role only",
|
||||
card: `{"name":"Reviewer","description":"Senior reviewer"}`,
|
||||
dbName: "Reviewer",
|
||||
dbRole: "code-reviewer",
|
||||
wantName: "Reviewer",
|
||||
wantDesc: "Senior reviewer",
|
||||
wantRole: "code-reviewer",
|
||||
wantChanged: true,
|
||||
},
|
||||
{
|
||||
name: "card already has a real friendly name — do NOT clobber it",
|
||||
// A richer card (e.g. an external channel agent) must win;
|
||||
// the platform only fills gaps, never downgrades.
|
||||
card: `{"name":"Claude Code (channel)","description":"Local Claude Code session bridged","role":"assistant"}`,
|
||||
dbName: "hongming-pc",
|
||||
dbRole: "operator",
|
||||
wantName: "Claude Code (channel)",
|
||||
wantDesc: "Local Claude Code session bridged",
|
||||
wantRole: "assistant",
|
||||
wantChanged: false,
|
||||
},
|
||||
{
|
||||
name: "no DB name available — leave UUID name untouched (no worse than before)",
|
||||
card: `{"name":"3b81321b-1ec7-488c-96f7-72c42a968da6","description":""}`,
|
||||
dbName: "",
|
||||
dbRole: "",
|
||||
wantName: "3b81321b-1ec7-488c-96f7-72c42a968da6",
|
||||
wantDesc: "",
|
||||
wantRole: "",
|
||||
wantChanged: false,
|
||||
},
|
||||
{
|
||||
name: "dbName equals UUID (placeholder row) — not a friendly name, leave untouched",
|
||||
card: `{"name":"3b81321b-1ec7-488c-96f7-72c42a968da6"}`,
|
||||
dbName: "3b81321b-1ec7-488c-96f7-72c42a968da6",
|
||||
dbRole: "",
|
||||
wantName: "3b81321b-1ec7-488c-96f7-72c42a968da6",
|
||||
wantDesc: "",
|
||||
wantRole: "",
|
||||
wantChanged: false,
|
||||
},
|
||||
{
|
||||
name: "malformed card JSON — return unchanged, no panic",
|
||||
card: `{not json`,
|
||||
dbName: "Claude Code Agent",
|
||||
dbRole: "",
|
||||
wantChanged: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
out, changed := reconcileAgentCardIdentity(
|
||||
json.RawMessage(tc.card), wsID, tc.dbName, tc.dbRole,
|
||||
)
|
||||
if changed != tc.wantChanged {
|
||||
t.Fatalf("changed = %v, want %v", changed, tc.wantChanged)
|
||||
}
|
||||
if !tc.wantChanged {
|
||||
// Unchanged path must return the input bytes verbatim.
|
||||
if string(out) != tc.card {
|
||||
t.Fatalf("unchanged path mutated bytes:\n got %s\n want %s", out, tc.card)
|
||||
}
|
||||
return
|
||||
}
|
||||
var got map[string]any
|
||||
if err := json.Unmarshal(out, &got); err != nil {
|
||||
t.Fatalf("output not valid JSON: %v (%s)", err, out)
|
||||
}
|
||||
if g, _ := got["name"].(string); g != tc.wantName {
|
||||
t.Errorf("name = %q, want %q", g, tc.wantName)
|
||||
}
|
||||
if g, _ := got["description"].(string); g != tc.wantDesc {
|
||||
t.Errorf("description = %q, want %q", g, tc.wantDesc)
|
||||
}
|
||||
if tc.wantRole != "" {
|
||||
if g, _ := got["role"].(string); g != tc.wantRole {
|
||||
t.Errorf("role = %q, want %q", g, tc.wantRole)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestReconcileAgentCardIdentity_PreservesOtherFields ensures the
|
||||
// reconcile is a minimal in-place patch — capabilities, version,
|
||||
// skills and any unknown future fields survive untouched.
|
||||
func TestReconcileAgentCardIdentity_PreservesOtherFields(t *testing.T) {
|
||||
card := `{"name":"ws-uuid","description":"","version":"1.0.0",` +
|
||||
`"capabilities":{"streaming":true,"pushNotifications":true},` +
|
||||
`"skills":[{"id":"a","name":"a"}],"configuration_status":"ready"}`
|
||||
out, changed := reconcileAgentCardIdentity(
|
||||
json.RawMessage(card), "ws-uuid", "Friendly Name", "",
|
||||
)
|
||||
if !changed {
|
||||
t.Fatal("expected changed = true")
|
||||
}
|
||||
var got map[string]any
|
||||
if err := json.Unmarshal(out, &got); err != nil {
|
||||
t.Fatalf("invalid JSON: %v", err)
|
||||
}
|
||||
if got["version"] != "1.0.0" {
|
||||
t.Errorf("version not preserved: %v", got["version"])
|
||||
}
|
||||
if got["configuration_status"] != "ready" {
|
||||
t.Errorf("configuration_status not preserved: %v", got["configuration_status"])
|
||||
}
|
||||
caps, ok := got["capabilities"].(map[string]any)
|
||||
if !ok || caps["streaming"] != true {
|
||||
t.Errorf("capabilities not preserved: %v", got["capabilities"])
|
||||
}
|
||||
skills, ok := got["skills"].([]any)
|
||||
if !ok || len(skills) != 1 {
|
||||
t.Errorf("skills not preserved: %v", got["skills"])
|
||||
}
|
||||
}
|
||||
@@ -1,8 +1,12 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"crypto/tls"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
// TestExternalTemplates_NoMoleculeOrgIDPlaceholder pins the invariant
|
||||
@@ -118,3 +122,153 @@ func TestExternalTemplates_NoBrokenMoleculeAIGitHubURLs(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// BuildExternalConnectionPayload
|
||||
// =============================================================================
|
||||
|
||||
func TestBuildExternalConnectionPayload_HappyPath(t *testing.T) {
|
||||
payload := BuildExternalConnectionPayload(
|
||||
"https://platform.example.com/",
|
||||
"ws-123",
|
||||
"tok-secret-abc",
|
||||
)
|
||||
|
||||
if payload["workspace_id"] != "ws-123" {
|
||||
t.Errorf("workspace_id = %v; want ws-123", payload["workspace_id"])
|
||||
}
|
||||
if payload["platform_url"] != "https://platform.example.com" {
|
||||
t.Errorf("platform_url = %v; want https://platform.example.com", payload["platform_url"])
|
||||
}
|
||||
if payload["auth_token"] != "tok-secret-abc" {
|
||||
t.Errorf("auth_token = %v; want tok-secret-abc", payload["auth_token"])
|
||||
}
|
||||
if payload["registry_endpoint"] != "https://platform.example.com/registry/register" {
|
||||
t.Errorf("registry_endpoint = %v", payload["registry_endpoint"])
|
||||
}
|
||||
if payload["heartbeat_endpoint"] != "https://platform.example.com/registry/heartbeat" {
|
||||
t.Errorf("heartbeat_endpoint = %v", payload["heartbeat_endpoint"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildExternalConnectionPayload_TrailingSlashStripped(t *testing.T) {
|
||||
// TrimSuffix only removes one trailing slash; multiple slashes stay.
|
||||
// This is intentional — the function documents this behavior.
|
||||
payload := BuildExternalConnectionPayload(
|
||||
"https://platform.example.com/",
|
||||
"ws-456",
|
||||
"tok",
|
||||
)
|
||||
if payload["platform_url"] != "https://platform.example.com" {
|
||||
t.Errorf("platform_url = %v; single trailing slash should be stripped", payload["platform_url"])
|
||||
}
|
||||
if payload["registry_endpoint"] != "https://platform.example.com/registry/register" {
|
||||
t.Errorf("registry_endpoint should not have double slash")
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildExternalConnectionPayload_EmptyAuthToken(t *testing.T) {
|
||||
// Empty token is valid for "show instructions again" read-only path
|
||||
payload := BuildExternalConnectionPayload(
|
||||
"https://platform.example.com",
|
||||
"ws-789",
|
||||
"",
|
||||
)
|
||||
if payload["workspace_id"] != "ws-789" {
|
||||
t.Errorf("workspace_id = %v", payload["workspace_id"])
|
||||
}
|
||||
if payload["auth_token"] != "" {
|
||||
t.Errorf("auth_token = %v; want empty string", payload["auth_token"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildExternalConnectionPayload_SnippetsStamped(t *testing.T) {
|
||||
payload := BuildExternalConnectionPayload(
|
||||
"https://platform.example.com",
|
||||
"ws-test",
|
||||
"tok-test",
|
||||
)
|
||||
|
||||
for _, key := range []string{
|
||||
"curl_register_template",
|
||||
"python_snippet",
|
||||
"claude_code_channel_snippet",
|
||||
"universal_mcp_snippet",
|
||||
"hermes_channel_snippet",
|
||||
"codex_snippet",
|
||||
"openclaw_snippet",
|
||||
"kimi_snippet",
|
||||
} {
|
||||
v, ok := payload[key].(string)
|
||||
if !ok {
|
||||
t.Errorf("%s is not a string", key)
|
||||
continue
|
||||
}
|
||||
if strings.Contains(v, "{{PLATFORM_URL}}") {
|
||||
t.Errorf("%s still contains unsubstituted {{PLATFORM_URL}}", key)
|
||||
}
|
||||
if strings.Contains(v, "{{WORKSPACE_ID}}") {
|
||||
t.Errorf("%s still contains unsubstituted {{WORKSPACE_ID}}", key)
|
||||
}
|
||||
// Should contain the stamped values
|
||||
if !strings.Contains(v, "https://platform.example.com") {
|
||||
t.Errorf("%s does not contain stamped platform URL", key)
|
||||
}
|
||||
if !strings.Contains(v, "ws-test") {
|
||||
t.Errorf("%s does not contain stamped workspace ID", key)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// externalPlatformURL
|
||||
// =============================================================================
|
||||
|
||||
func TestExternalPlatformURL_EnvVarTakesPrecedence(t *testing.T) {
|
||||
t.Setenv("EXTERNAL_PLATFORM_URL", "https://external.example.com")
|
||||
c, _ := gin.CreateTestContext(httptest.NewRecorder())
|
||||
c.Request = httptest.NewRequest("GET", "/", nil)
|
||||
|
||||
got := externalPlatformURL(c)
|
||||
if got != "https://external.example.com" {
|
||||
t.Errorf("got %q; want EXTERNAL_PLATFORM_URL value", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExternalPlatformURL_XForwardedProtoAndHost(t *testing.T) {
|
||||
c, _ := gin.CreateTestContext(httptest.NewRecorder())
|
||||
c.Request = httptest.NewRequest("GET", "/", nil)
|
||||
c.Request.Header.Set("X-Forwarded-Proto", "https")
|
||||
c.Request.Header.Set("X-Forwarded-Host", "tenant.example.com")
|
||||
|
||||
got := externalPlatformURL(c)
|
||||
if got != "https://tenant.example.com" {
|
||||
t.Errorf("got %q; want https://tenant.example.com", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExternalPlatformURL_HTTPSNoHeaders(t *testing.T) {
|
||||
c, _ := gin.CreateTestContext(httptest.NewRecorder())
|
||||
c.Request = httptest.NewRequest("GET", "/", nil)
|
||||
c.Request.Host = "platform.example.com"
|
||||
// TLS set → https scheme (regardless of host)
|
||||
c.Request.TLS = &tls.ConnectionState{}
|
||||
|
||||
got := externalPlatformURL(c)
|
||||
if got != "https://platform.example.com" {
|
||||
t.Errorf("got %q; want https://platform.example.com (TLS set)", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExternalPlatformURL_HTTPNoTLS(t *testing.T) {
|
||||
c, _ := gin.CreateTestContext(httptest.NewRecorder())
|
||||
c.Request = httptest.NewRequest("GET", "/", nil)
|
||||
c.Request.Host = "localhost:8080"
|
||||
// TLS nil → http
|
||||
c.Request.TLS = nil
|
||||
|
||||
got := externalPlatformURL(c)
|
||||
if got != "http://localhost:8080" {
|
||||
t.Errorf("got %q; want http://localhost:8080", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -327,7 +327,33 @@ func (h *RegistryHandler) Register(c *gin.Context) {
|
||||
}
|
||||
}
|
||||
|
||||
agentCardStr := string(payload.AgentCard)
|
||||
// Reconcile the runtime-supplied card's identity fields against the
|
||||
// trusted workspaces row before storing. The runtime builds its card
|
||||
// from config.name, which the CP-regenerated /configs/config.yaml
|
||||
// sets to the workspace UUID — so without this the stored card
|
||||
// served at /.well-known/agent-card.json and returned to peers via
|
||||
// agent_card_url has name = UUID, description = "", role = null even
|
||||
// though the operator-controlled workspaces.name holds the friendly
|
||||
// name the canvas shows. We only FILL gaps from the DB (never
|
||||
// downgrade a card that already carries a real name); identity stays
|
||||
// platform-controlled — the agent cannot self-set these. Best-effort:
|
||||
// a lookup failure leaves the card exactly as the runtime sent it
|
||||
// (no-worse-than-before). See agent_card_reconcile.go.
|
||||
reconciledCard := payload.AgentCard
|
||||
{
|
||||
var dbName, dbRole sql.NullString
|
||||
if qErr := db.DB.QueryRowContext(ctx,
|
||||
`SELECT name, role FROM workspaces WHERE id = $1`, payload.ID,
|
||||
).Scan(&dbName, &dbRole); qErr == nil {
|
||||
if rc, did := reconcileAgentCardIdentity(
|
||||
payload.AgentCard, payload.ID, dbName.String, dbRole.String,
|
||||
); did {
|
||||
reconciledCard = rc
|
||||
log.Printf("Registry register: reconciled agent_card identity for %s from workspaces row", payload.ID)
|
||||
}
|
||||
}
|
||||
}
|
||||
agentCardStr := string(reconciledCard)
|
||||
|
||||
// urlForUpsert: poll-mode workspaces don't need a URL. Empty input
|
||||
// becomes NULL via sql.NullString so the row's URL stays clean (the
|
||||
@@ -413,10 +439,12 @@ func (h *RegistryHandler) Register(c *gin.Context) {
|
||||
}
|
||||
}
|
||||
|
||||
// Broadcast WORKSPACE_ONLINE
|
||||
// Broadcast WORKSPACE_ONLINE — use the reconciled card so the canvas
|
||||
// Agent Card view live-updates with the friendly name, matching what
|
||||
// was just persisted (not the runtime's raw UUID-name card).
|
||||
if err := h.broadcaster.RecordAndBroadcast(ctx, string(events.EventWorkspaceOnline), payload.ID, map[string]interface{}{
|
||||
"url": cachedURL,
|
||||
"agent_card": payload.AgentCard,
|
||||
"agent_card": reconciledCard,
|
||||
"delivery_mode": effectiveMode,
|
||||
}); err != nil {
|
||||
log.Printf("Registry broadcast error: %v", err)
|
||||
|
||||
@@ -19,6 +19,7 @@ package handlers
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log"
|
||||
"os"
|
||||
@@ -357,6 +358,28 @@ func writeFileViaEIC(ctx context.Context, instanceID, runtime, root, relPath str
|
||||
var stderr bytes.Buffer
|
||||
sshCmd.Stderr = &stderr
|
||||
if err := sshCmd.Run(); err != nil {
|
||||
// When the per-op context deadline (eicFileOpTimeout) fires,
|
||||
// exec.CommandContext SIGKILLs the ssh subprocess and Run()
|
||||
// returns the bare "signal: killed" with empty stderr. That
|
||||
// surfaced to the canvas as an opaque
|
||||
// `500 {"error":"ssh install: signal: killed ()"}` which gave
|
||||
// the operator no idea the workspace was simply mid-provision
|
||||
// with a slow/unready EIC tunnel (internal#423). Detect the
|
||||
// deadline explicitly and return an actionable message instead
|
||||
// — the EIC mechanism, timeout value, and success path are all
|
||||
// unchanged; this only improves the error a stuck write emits.
|
||||
if cerr := ctx.Err(); cerr != nil {
|
||||
reason := "timed out after " + eicFileOpTimeout.String()
|
||||
if errors.Is(cerr, context.Canceled) && !errors.Is(cerr, context.DeadlineExceeded) {
|
||||
reason = "was cancelled"
|
||||
}
|
||||
return fmt.Errorf(
|
||||
"ssh install: EIC tunnel to workspace %s — "+
|
||||
"the workspace may still be provisioning (slow/unready SSH); "+
|
||||
"retry once it is online, or apply provider credentials via "+
|
||||
"Settings → Secrets (encrypted, does not use this file-write path)",
|
||||
reason)
|
||||
}
|
||||
return fmt.Errorf("ssh install: %w (%s)", err, strings.TrimSpace(stderr.String()))
|
||||
}
|
||||
log.Printf("writeFileViaEIC: ws instance=%s runtime=%s root=%s wrote %d bytes → %s",
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
package handlers
|
||||
|
||||
// template_files_eic_write_timeout_test.go — pins the actionable-error
|
||||
// behavior added for internal#423.
|
||||
//
|
||||
// When the per-op context deadline (eicFileOpTimeout) fires,
|
||||
// exec.CommandContext SIGKILLs the ssh subprocess and Run() returns the
|
||||
// bare "signal: killed" with empty stderr. Before the fix that surfaced
|
||||
// to the canvas as an opaque `500 {"error":"ssh install: signal:
|
||||
// killed ()"}` — useless to an operator whose workspace was simply
|
||||
// mid-provision with a slow/unready EIC tunnel. The fix detects the
|
||||
// deadline explicitly (errors.Is(ctx.Err(), context.DeadlineExceeded))
|
||||
// and returns a message that names the cause and the
|
||||
// Settings → Secrets workaround.
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// TestWriteFileViaEIC_DeadlineExceeded_ActionableError stubs
|
||||
// withEICTunnel so the *real* inner closure runs against a context that
|
||||
// has already exceeded its deadline. The ssh subprocess fails (no real
|
||||
// sshd on the fake port) and ctx.Err() == DeadlineExceeded, so the new
|
||||
// branch must fire and produce an actionable message — NOT the opaque
|
||||
// "signal: killed ()" string the canvas used to show.
|
||||
func TestWriteFileViaEIC_DeadlineExceeded_ActionableError(t *testing.T) {
|
||||
prev := withEICTunnel
|
||||
withEICTunnel = func(_ context.Context, instanceID string, fn func(s eicSSHSession) error) error {
|
||||
// Run the real inner closure. It closes over the ctx that
|
||||
// writeFileViaEIC derived from our already-cancelled parent, so
|
||||
// the ssh subprocess is killed immediately and ctx.Err()
|
||||
// resolves — exactly the eicFileOpTimeout-expiry shape.
|
||||
return fn(eicSSHSession{
|
||||
instanceID: instanceID,
|
||||
osUser: "ubuntu",
|
||||
localPort: 1, // nothing listening → ssh fails fast
|
||||
keyPath: "/nonexistent/key",
|
||||
})
|
||||
}
|
||||
t.Cleanup(func() { withEICTunnel = prev })
|
||||
|
||||
// Drive the real writeFileViaEIC. Pass a parent whose deadline has
|
||||
// already passed: the context.WithTimeout(ctx, eicFileOpTimeout)
|
||||
// derived inside writeFileViaEIC inherits the expired parent
|
||||
// deadline, so ctx.Err() == context.DeadlineExceeded by the time
|
||||
// the killed ssh subprocess returns — the exact production shape
|
||||
// (eicFileOpTimeout expiry), exercised deterministically.
|
||||
parent, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second))
|
||||
defer cancel()
|
||||
|
||||
err := writeFileViaEIC(parent, "i-test", "claude-code", "/configs", "config.yaml", []byte("model: sonnet\n"))
|
||||
if err == nil {
|
||||
t.Fatalf("expected an error from a killed ssh subprocess, got nil")
|
||||
}
|
||||
msg := err.Error()
|
||||
|
||||
// Must NOT leak the opaque bare-signal string to the operator.
|
||||
if strings.Contains(msg, "signal: killed ()") {
|
||||
t.Fatalf("error still surfaces the opaque %q form: %q", "signal: killed ()", msg)
|
||||
}
|
||||
// Must name the cause and the Secrets workaround so the canvas
|
||||
// shows something actionable.
|
||||
for _, want := range []string{"timed out", "provisioning", "Settings", "Secrets"} {
|
||||
if !strings.Contains(msg, want) {
|
||||
t.Errorf("actionable error missing %q; got: %q", want, msg)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user