Merge pull request #1499 from justintruong29/fix/vault-read-tenant-workspace

fix(vault): resolve vault_read paths under tenant workspace root

cmd/gateway_vault_wiring.go

@@ -36,6 +36,10 @@ func wireVault(stores *store.Stores, toolsReg *tools.Registry, workspace string,
 	if stores.Episodic != nil {
 		vaultReadTool.SetEpisodicStore(stores.Episodic)
 	}
+	// Tenant slug fallback for runs whose context carries no slug (channels, cron).
+	if stores.Tenants != nil {
+		vaultReadTool.SetTenantStore(stores.Tenants)
+	}
 	toolsReg.Register(vaultReadTool)
 
 	// Build VaultSearchService: fan-out across vault + episodic + KG.

internal/tools/vault_read.go

@@ -22,6 +22,7 @@ type VaultReadTool struct {
 	vaultStore    store.VaultStore
 	kgStore       store.KnowledgeGraphStore
 	episodicStore store.EpisodicStore
+	tenantStore   store.TenantStore
 	workspace     string
 }
 
@@ -74,9 +75,15 @@ func (t *VaultReadTool) SetKGStore(kg store.KnowledgeGraphStore) { t.kgStore = k
 // lookup when vault lookup misses. nil is safe.
 func (t *VaultReadTool) SetEpisodicStore(es store.EpisodicStore) { t.episodicStore = es }
 
-// SetWorkspace injects the tenant workspace root (wired at boot).
+// SetWorkspace injects the global workspace root (wired at boot). This is the
+// base for every tenant — the tenant-scoped root is resolved per request in
+// tenantWorkspace, since one tool instance serves all tenants.
 func (t *VaultReadTool) SetWorkspace(ws string) { t.workspace = ws }
 
+// SetTenantStore injects an optional TenantStore used to resolve the tenant
+// slug when the request context does not carry one. nil is safe.
+func (t *VaultReadTool) SetTenantStore(ts store.TenantStore) { t.tenantStore = ts }
+
 func (t *VaultReadTool) Name() string { return "vault_read" }
 
 func (t *VaultReadTool) Description() string {
@@ -145,8 +152,8 @@ func (t *VaultReadTool) Execute(ctx context.Context, args map[string]any) *Resul
 		return ErrorResult(fmt.Sprintf("vault_read does not support %s files — use read_image/read_audio/read_video/read_document", ext))
 	}
 
-	// Resolve path under tenant workspace root.
-	fullPath, err := t.resolvePath(doc.Path)
+	// Resolve path under the tenant workspace root.
+	fullPath, err := t.resolvePath(t.tenantWorkspace(ctx, tenantID), doc.Path)
 	if err != nil {
 		return ErrorResult(err.Error())
 	}
@@ -344,13 +351,33 @@ func (t *VaultReadTool) allowed(ctx context.Context, doc *store.VaultDocument) b
 	}
 }
 
-// resolvePath joins the tenant workspace with the doc's relative path and
-// enforces that the fully-resolved path remains strictly under the workspace.
+// tenantWorkspace resolves the tenant-scoped workspace root for this request.
+// Vault paths are stored relative to that root (e.g. "agents/<key>/note.md"),
+// so joining them onto the global root silently misses every non-master tenant
+// whose files live under <root>/tenants/<slug>/. Master tenant is a no-op
+// inside TenantLayer and keeps using the base root.
+func (t *VaultReadTool) tenantWorkspace(ctx context.Context, tenantID uuid.UUID) string {
+	slug := store.TenantSlugFromContext(ctx)
+	// Channel- and cron-originated runs may not carry the slug in context.
+	// Without it TenantLayer falls back to a UUID-named directory, which does
+	// not match the slug-named directory the write path creates on disk.
+	if slug == "" && tenantID != store.MasterTenantID && t.tenantStore != nil {
+		if tenant, err := t.tenantStore.GetTenant(ctx, tenantID); err == nil && tenant != nil {
+			slug = tenant.Slug
+		}
+	}
+	return ResolveWorkspace(t.workspace, TenantLayer(tenantID, slug))
+}
+
+// resolvePath joins the given workspace root with the doc's relative path and
+// enforces that the fully-resolved path remains strictly under that root.
+// Passing the tenant root (not the global one) also makes the boundary check
+// double as cross-tenant isolation.
 // Symlinks are resolved via EvalSymlinks for defence-in-depth; if the file is
 // missing EvalSymlinks will fail and we fall back to the cleaned join (the
 // subsequent os.Open will surface the not-found error naturally).
-func (t *VaultReadTool) resolvePath(relPath string) (string, error) {
-	wsClean := filepath.Clean(t.workspace)
+func (t *VaultReadTool) resolvePath(root, relPath string) (string, error) {
+	wsClean := filepath.Clean(root)
 	joined := filepath.Join(wsClean, filepath.FromSlash(relPath))
 
 	// Resolve symlinks where possible; on error (e.g. file missing) keep the

internal/tools/vault_read_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/google/uuid"
 
+	"github.com/nextlevelbuilder/goclaw/internal/config"
 	"github.com/nextlevelbuilder/goclaw/internal/store"
 )
 
@@ -61,17 +62,30 @@ func (f *fakeVaultStore) GetDocumentsByIDs(ctx context.Context, tenantID string,
 
 // newVaultReadTestTool builds a VaultReadTool with a temp workspace and a
 // fake store pre-seeded with docs.
+//
+// The tool is given the GLOBAL workspace root, mirroring boot wiring. The
+// returned path is the TENANT-scoped root of the first seeded doc — the
+// directory the tool resolves per request, and therefore where test content
+// must be written. Tests set no slug on the context, so the tenant directory
+// is UUID-named (see config.TenantWorkspace).
 func newVaultReadTestTool(t *testing.T, docs ...*store.VaultDocument) (*VaultReadTool, string) {
 	t.Helper()
-	ws := t.TempDir()
+	base := t.TempDir()
 	fake := &fakeVaultStore{byID: make(map[string]*store.VaultDocument)}
 	for _, d := range docs {
 		fake.byID[d.TenantID+":"+d.ID] = d
 	}
 	tool := NewVaultReadTool()
 	tool.SetVaultStore(fake)
-	tool.SetWorkspace(ws)
-	return tool, ws
+	tool.SetWorkspace(base)
+
+	contentRoot := base
+	if len(docs) > 0 {
+		if tid, err := uuid.Parse(docs[0].TenantID); err == nil {
+			contentRoot = config.TenantWorkspace(base, tid, "")
+		}
+	}
+	return tool, contentRoot
 }
 
 // writeFile creates a file under workspace with the given relative path.
@@ -917,3 +931,118 @@ func TestVaultRead_TrulyMissingReturnsNotFound(t *testing.T) {
 		t.Fatalf("expected document not found, got: %s", res.ForLLM)
 	}
 }
+
+// --- 18. Regression: non-master tenant content lives under tenants/<slug>/.
+// The tool holds the GLOBAL root, so a global-root join misses the file and
+// every non-master tenant reads ENOENT while vault_search (DB-only) still
+// returns the doc. Mirrors the production failure on tenant "alpha". ---
+func TestVaultRead_NonMasterTenant_ResolvesUnderTenantRoot(t *testing.T) {
+	tenantID := uuid.New()
+	agentID := uuid.New()
+	docID := uuid.New()
+	doc := &store.VaultDocument{
+		ID: docID.String(), TenantID: tenantID.String(),
+		Scope: "shared", Path: "agents/ho-tro/Refund Flow.md",
+		Title: "Refund Flow", DocType: "note",
+	}
+	base := t.TempDir()
+	fake := &fakeVaultStore{byID: map[string]*store.VaultDocument{
+		tenantID.String() + ":" + docID.String(): doc,
+	}}
+	tool := NewVaultReadTool()
+	tool.SetVaultStore(fake)
+	tool.SetWorkspace(base)
+
+	// Written by the tenant-aware write path; slug comes from context.
+	tenantRoot := config.TenantWorkspace(base, tenantID, "alpha")
+	writeFile(t, tenantRoot, "agents/ho-tro/Refund Flow.md", "refund body")
+
+	ctx := store.WithTenantSlug(makeCtx(tenantID, agentID), "alpha")
+	res := tool.Execute(ctx, map[string]any{"doc_id": docID.String()})
+	if res.IsError {
+		t.Fatalf("tenant doc must resolve under tenants/<slug>/, got: %s", res.ForLLM)
+	}
+	if !strings.Contains(res.ForLLM, "refund body") {
+		t.Fatalf("content missing: %s", res.ForLLM)
+	}
+}
+
+// --- 19. Master tenant keeps using the base root (no tenants/ segment), so
+// the fix above must not regress single-tenant and master installs. ---
+func TestVaultRead_MasterTenant_UsesBaseRoot(t *testing.T) {
+	agentID := uuid.New()
+	docID := uuid.New()
+	doc := &store.VaultDocument{
+		ID: docID.String(), TenantID: store.MasterTenantID.String(),
+		Scope: "shared", Path: "agents/ho-tro/note.md",
+		Title: "Note", DocType: "note",
+	}
+	base := t.TempDir()
+	fake := &fakeVaultStore{byID: map[string]*store.VaultDocument{
+		store.MasterTenantID.String() + ":" + docID.String(): doc,
+	}}
+	tool := NewVaultReadTool()
+	tool.SetVaultStore(fake)
+	tool.SetWorkspace(base)
+
+	// No tenants/ segment for master.
+	writeFile(t, base, "agents/ho-tro/note.md", "master body")
+
+	res := tool.Execute(makeCtx(store.MasterTenantID, agentID),
+		map[string]any{"doc_id": docID.String()})
+	if res.IsError {
+		t.Fatalf("master doc must resolve at base root, got: %s", res.ForLLM)
+	}
+	if !strings.Contains(res.ForLLM, "master body") {
+		t.Fatalf("content missing: %s", res.ForLLM)
+	}
+}
+
+// --- 20. Slug absent from context (channel/cron runs) → fall back to the
+// TenantStore lookup, since a UUID-named directory would miss the slug-named
+// one the write path created. ---
+func TestVaultRead_SlugMissingFromContext_FallsBackToTenantStore(t *testing.T) {
+	tenantID := uuid.New()
+	agentID := uuid.New()
+	docID := uuid.New()
+	doc := &store.VaultDocument{
+		ID: docID.String(), TenantID: tenantID.String(),
+		Scope: "shared", Path: "agents/ho-tro/note.md",
+		Title: "Note", DocType: "note",
+	}
+	base := t.TempDir()
+	fake := &fakeVaultStore{byID: map[string]*store.VaultDocument{
+		tenantID.String() + ":" + docID.String(): doc,
+	}}
+	tool := NewVaultReadTool()
+	tool.SetVaultStore(fake)
+	tool.SetWorkspace(base)
+	tool.SetTenantStore(&fakeTenantStoreRead{slug: "alpha", id: tenantID})
+
+	writeFile(t, config.TenantWorkspace(base, tenantID, "alpha"),
+		"agents/ho-tro/note.md", "cron body")
+
+	// Context deliberately carries no slug.
+	res := tool.Execute(makeCtx(tenantID, agentID),
+		map[string]any{"doc_id": docID.String()})
+	if res.IsError {
+		t.Fatalf("slug fallback must resolve the doc, got: %s", res.ForLLM)
+	}
+	if !strings.Contains(res.ForLLM, "cron body") {
+		t.Fatalf("content missing: %s", res.ForLLM)
+	}
+}
+
+// fakeTenantStoreRead resolves a single tenant slug for the fallback path.
+type fakeTenantStoreRead struct {
+	store.TenantStore
+	id   uuid.UUID
+	slug string
+}
+
+func (f *fakeTenantStoreRead) GetTenant(_ context.Context, id uuid.UUID) (*store.TenantData, error) {
+	if id != f.id {
+		return nil, os.ErrNotExist
+	}
+	return &store.TenantData{ID: f.id, Slug: f.slug}, nil
+}

tests/integration/v3_test_helper.go

@@ -15,6 +15,7 @@ import (
 	"github.com/google/uuid"
 	_ "github.com/jackc/pgx/v5/stdlib"
 
+	"github.com/nextlevelbuilder/goclaw/internal/config"
 	"github.com/nextlevelbuilder/goclaw/internal/security"
 	"github.com/nextlevelbuilder/goclaw/internal/store"
 	"github.com/nextlevelbuilder/goclaw/internal/store/pg"
@@ -89,7 +90,7 @@ func seedTenantAgent(t *testing.T, db *sql.DB) (tenantID, agentID uuid.UUID) {
 	_, err := db.Exec(
 		`INSERT INTO tenants (id, name, slug, status) VALUES ($1, $2, $3, 'active')
 		 ON CONFLICT DO NOTHING`,
-		tenantID, "test-tenant-"+tenantID.String()[:8], "t"+tenantID.String()[:8])
+		tenantID, "test-tenant-"+tenantID.String()[:8], tenantSlug(tenantID))
 	if err != nil {
 		t.Fatalf("seed tenant: %v", err)
 	}
@@ -178,6 +179,34 @@ func tenantCtx(tenantID uuid.UUID) context.Context {
 	return store.WithTenantID(context.Background(), tenantID)
 }
 
+// tenantSlug returns the slug seedTenantAgent assigns to a seeded tenant.
+// Single source of truth so on-disk layout in a test cannot drift from the seed.
+func tenantSlug(tenantID uuid.UUID) string {
+	return "t" + tenantID.String()[:8]
+}
+
+// tenantCtxSlug is tenantCtx plus the tenant slug. Real runs carry the slug in
+// context, and tools that resolve a tenant-scoped workspace need it: without a
+// slug config.TenantScopedDir deliberately falls back to an id-named directory
+// (see its doc comment — an empty slug must never resolve to the tenants/ parent).
+func tenantCtxSlug(tenantID uuid.UUID) context.Context {
+	return store.WithTenantSlug(tenantCtx(tenantID), tenantSlug(tenantID))
+}
+
+// tenantWorkspaceDir creates and returns the tenant-scoped workspace root under
+// base — what config.TenantWorkspace resolves to for a seeded (non-master) tenant.
+// Vault document paths are stored relative to THIS directory, not to base: base is
+// the global workspace root wired once at boot, and one tool instance serves every
+// tenant. Fixtures written straight to base are only reachable by the master tenant.
+func tenantWorkspaceDir(t *testing.T, base string, tenantID uuid.UUID) string {
+	t.Helper()
+	dir := config.TenantWorkspace(base, tenantID, tenantSlug(tenantID))
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatalf("mkdir tenant workspace: %v", err)
+	}
+	return dir
+}
+
 // userCtx returns a context with both tenant ID and user ID set.
 func userCtx(tenantID uuid.UUID, userID string) context.Context {
 	ctx := store.WithTenantID(context.Background(), tenantID)

tests/integration/vault_namespace_fix_test.go

@@ -35,12 +35,12 @@ func TestVaultNamespaceFix_thuyTienScenario(t *testing.T) {
 	ws := t.TempDir()
 	relPath := "KG_03_Danh_Muc_San_Pham.md"
 	body := "Product catalog body"
-	if err := os.WriteFile(filepath.Join(ws, relPath), []byte(body), 0o644); err != nil {
+	if err := os.WriteFile(filepath.Join(tenantWorkspaceDir(t, ws, tenantID), relPath), []byte(body), 0o644); err != nil {
 		t.Fatalf("write file: %v", err)
 	}
 
 	userID := "kguser-" + agentID.String()[:8]
-	ctx := store.WithUserID(store.WithAgentID(tenantCtx(tenantID), agentID), userID)
+	ctx := store.WithUserID(store.WithAgentID(tenantCtxSlug(tenantID), agentID), userID)
 
 	// Seed vault doc.
 	vdoc := makeSharedVaultDoc(tenantID.String(), relPath, "KG_03 Danh Muc San Pham")

tests/integration/vault_read_test.go

@@ -27,13 +27,13 @@ func TestVaultRead_SharedAtWorkspaceRoot(t *testing.T) {
 	ws := t.TempDir()
 	body := strings.Repeat("KG_03 body line\n", 300) // ~4.8KB
 	relPath := "KG_03_Test.md"
-	if err := os.WriteFile(filepath.Join(ws, relPath), []byte(body), 0o644); err != nil {
+	if err := os.WriteFile(filepath.Join(tenantWorkspaceDir(t, ws, tenantID), relPath), []byte(body), 0o644); err != nil {
 		t.Fatalf("write file: %v", err)
 	}
 
 	// Register doc directly (avoids spinning up the full rescan pipeline).
 	doc := makeSharedVaultDoc(tenantID.String(), relPath, "KG_03 Test")
-	ctx := tenantCtx(tenantID)
+	ctx := tenantCtxSlug(tenantID)
 	ctx = store.WithAgentID(ctx, agentID)
 	if err := vs.UpsertDocument(ctx, doc); err != nil {
 		t.Fatalf("UpsertDocument: %v", err)
@@ -68,12 +68,12 @@ func TestVaultRead_PersonalCrossAgentDenied(t *testing.T) {
 
 	ws := t.TempDir()
 	relPath := "private.md"
-	if err := os.WriteFile(filepath.Join(ws, relPath), []byte("secret"), 0o644); err != nil {
+	if err := os.WriteFile(filepath.Join(tenantWorkspaceDir(t, ws, tenantID), relPath), []byte("secret"), 0o644); err != nil {
 		t.Fatalf("write file: %v", err)
 	}
 
 	doc := makeVaultDoc(tenantID.String(), agentA.String(), relPath, "Private")
-	ctxA := store.WithAgentID(tenantCtx(tenantID), agentA)
+	ctxA := store.WithAgentID(tenantCtxSlug(tenantID), agentA)
 	if err := vs.UpsertDocument(ctxA, doc); err != nil {
 		t.Fatalf("UpsertDocument: %v", err)
 	}
@@ -83,7 +83,7 @@ func TestVaultRead_PersonalCrossAgentDenied(t *testing.T) {
 	tool.SetWorkspace(ws)
 
 	// Agent B reads → must be denied.
-	ctxB := store.WithAgentID(tenantCtx(tenantID), agentB)
+	ctxB := store.WithAgentID(tenantCtxSlug(tenantID), agentB)
 	res := tool.Execute(ctxB, map[string]any{"doc_id": doc.ID})
 	if !res.IsError {
 		t.Fatalf("expected access denied for cross-agent personal read, got content")
@@ -102,7 +102,7 @@ func TestVaultRead_MediaRejected(t *testing.T) {
 
 	ws := t.TempDir()
 	relPath := "pic.png"
-	if err := os.WriteFile(filepath.Join(ws, relPath), []byte("pretend-png"), 0o644); err != nil {
+	if err := os.WriteFile(filepath.Join(tenantWorkspaceDir(t, ws, tenantID), relPath), []byte("pretend-png"), 0o644); err != nil {
 		t.Fatalf("write file: %v", err)
 	}
 
@@ -117,7 +117,7 @@ func TestVaultRead_MediaRejected(t *testing.T) {
 		ContentHash: "abc",
 		Metadata:    map[string]any{"mime_type": "image/png"},
 	}
-	ctx := store.WithAgentID(tenantCtx(tenantID), agentID)
+	ctx := store.WithAgentID(tenantCtxSlug(tenantID), agentID)
 	if err := vs.UpsertDocument(ctx, doc); err != nil {
 		t.Fatalf("UpsertDocument: %v", err)
 	}
@@ -144,12 +144,12 @@ func TestVaultRead_OversizeTruncation(t *testing.T) {
 	ws := t.TempDir()
 	relPath := "big.md"
 	big := bytes.Repeat([]byte("a"), 300_000)
-	if err := os.WriteFile(filepath.Join(ws, relPath), big, 0o644); err != nil {
+	if err := os.WriteFile(filepath.Join(tenantWorkspaceDir(t, ws, tenantID), relPath), big, 0o644); err != nil {
 		t.Fatalf("write file: %v", err)
 	}
 
 	doc := makeSharedVaultDoc(tenantID.String(), relPath, "Big Doc")
-	ctx := store.WithAgentID(tenantCtx(tenantID), agentID)
+	ctx := store.WithAgentID(tenantCtxSlug(tenantID), agentID)
 	if err := vs.UpsertDocument(ctx, doc); err != nil {
 		t.Fatalf("UpsertDocument: %v", err)
 	}
@@ -194,11 +194,11 @@ func TestVaultRead_OutlinksScopeMatrix(t *testing.T) {
 
 	ws := t.TempDir()
 	tid := tenantID.String()
-	ctxSelf := store.WithAgentID(tenantCtx(tenantID), agentSelf)
+	ctxSelf := store.WithAgentID(tenantCtxSlug(tenantID), agentSelf)
 
 	// Source file on disk + doc (shared so read is allowed).
 	srcRel := "src.md"
-	if err := os.WriteFile(filepath.Join(ws, srcRel), []byte("src body"), 0o644); err != nil {
+	if err := os.WriteFile(filepath.Join(tenantWorkspaceDir(t, ws, tenantID), srcRel), []byte("src body"), 0o644); err != nil {
 		t.Fatalf("write src: %v", err)
 	}
 	src := makeSharedVaultDoc(tid, srcRel, "Source")