fix(sandbox): advertise only image skills and unstick installs

The host skills/preloaded tree is not what execute_skill_script finds
inside the sandbox, so stop merging it into the agent's advertised set.
Seed install files as a tar, publish transcript locators before the copy,
and wait in the UI instead of 404-polling.

Author: lyingbug

frontend/src/components/SandboxSkillsPanel.vue

@@ -125,7 +125,13 @@
         </p>
 
         <ul class="skill-list">
-          <li v-for="skill in skills" :key="skill.id" class="skill-item">
+          <li
+            v-for="skill in skills"
+            :key="skill.id"
+            :ref="(el) => bindSkillItem(skill.id, el)"
+            class="skill-item"
+            :class="{ 'skill-item--focused': focusedSkillId === skill.id }"
+          >
             <div class="skill-status-ring" :title="statusLabel(skill)">
               <t-progress
                 v-if="isBusy(skill)"
@@ -154,6 +160,7 @@
                     <span v-if="skill.version">{{ skill.version }} · </span>
                     <span>{{ statusLabel(skill) }}</span>
                     <span v-if="isBusy(skill)"> · {{ progressOf(skill) }}%</span>
+                    <span v-if="isBusy(skill) && progressLog(skill)"> · {{ progressLog(skill) }}</span>
                   </p>
                 </div>
                 <div class="skill-item__actions">
@@ -275,7 +282,7 @@
 </template>
 
 <script setup lang="ts">
-import { computed, onUnmounted, ref, watch } from 'vue'
+import { computed, nextTick, onUnmounted, ref, watch } from 'vue'
 import { MessagePlugin } from 'tdesign-vue-next'
 import { useI18n } from 'vue-i18n'
 import { fetchEventSource } from '@microsoft/fetch-event-source'
@@ -330,6 +337,9 @@ const deletingId = ref('')
 const expandedSkillId = ref('')
 const expandedCopyIds = ref<Set<string>>(new Set())
 const transcriptEpoch = ref(0)
+const focusedSkillId = ref('')
+const skillItemEls = new Map<string, HTMLElement>()
+let focusTimer: number | null = null
 const fileInputRef = ref<HTMLInputElement | null>(null)
 const progressById = ref<Record<string, ConfigSkillInstallEvent>>({})
 
@@ -408,6 +418,26 @@ function hasTranscript(skill: ConfigSkill): boolean {
   return Boolean(skill.install_session_id && skill.install_message_id)
 }
 
+function bindSkillItem(id: string, el: unknown) {
+  if (el instanceof HTMLElement) {
+    skillItemEls.set(id, el)
+    return
+  }
+  skillItemEls.delete(id)
+}
+
+function revealSkill(skillId: string) {
+  focusedSkillId.value = skillId
+  void nextTick(() => {
+    skillItemEls.get(skillId)?.scrollIntoView({ behavior: 'smooth', block: 'center' })
+  })
+  if (focusTimer != null) window.clearTimeout(focusTimer)
+  focusTimer = window.setTimeout(() => {
+    if (focusedSkillId.value === skillId) focusedSkillId.value = ''
+    focusTimer = null
+  }, 2400)
+}
+
 function onTranscriptVisible(skill: ConfigSkill, visible: boolean) {
   if (visible) {
     if (expandedSkillId.value !== skill.id) {
@@ -671,7 +701,10 @@ async function uploadFile(file: File) {
     const skillId = res?.data?.skill_id
     await loadSkills()
     await refreshImage()
-    if (skillId) followProgress(skillId)
+    if (skillId) {
+      followProgress(skillId)
+      revealSkill(skillId)
+    }
   } catch (e: any) {
     MessagePlugin.error(e?.message || t('settings.sandbox.skillUploadFailed'))
   } finally {
@@ -698,7 +731,10 @@ async function installFromSource() {
     const skillId = res?.data?.skill_id
     await loadSkills()
     await refreshImage()
-    if (skillId) followProgress(skillId)
+    if (skillId) {
+      followProgress(skillId)
+      revealSkill(skillId)
+    }
   } catch (e: any) {
     MessagePlugin.error(e?.message || t('settings.sandbox.skillSourceFailed'))
   } finally {
@@ -773,6 +809,7 @@ watch(
 onUnmounted(() => {
   stopAllFollows()
   stopPoll()
+  if (focusTimer != null) window.clearTimeout(focusTimer)
 })
 </script>
 
@@ -976,6 +1013,12 @@ onUnmounted(() => {
   border: 1px solid var(--td-component-stroke);
   border-radius: 8px;
   background: var(--td-bg-color-container);
+  transition: border-color 0.2s ease, box-shadow 0.2s ease;
+}
+
+.skill-item--focused {
+  border-color: var(--td-brand-color);
+  box-shadow: 0 0 0 2px var(--td-brand-color-focus, rgba(0, 168, 112, 0.18));
 }
 
 .skill-transcript-toggle--on {

frontend/src/components/SkillInstallTimeline.vue

@@ -38,9 +38,9 @@ const props = defineProps<{
   // The durable rows behind the run, used when the event log has aged out.
   sessionId: string
   messageId: string
-  // True while this skill is still installing. The locators are written only
-  // after the installer sandbox is up, so a 404 here means "not yet" rather
-  // than "gone" and the stream is retried.
+  // True while this skill is still installing. Locators are written after the
+  // sandbox is up; until then this component shows the waiting copy and does
+  // not hit /transcript.
   live?: boolean
   compact?: boolean
 }>()
@@ -168,22 +168,30 @@ async function open() {
   const run = ++openRun
   closed = false
   messages.splice(0, messages.length)
-  loading.value = true
   const stale = () => run !== openRun || closed
   try {
     // A finished install already has durable rows. Replaying the event log
     // through processStreamChunk would animate every tool call again, which
     // is what "view the run" must not do.
     if (!props.live) {
+      loading.value = true
       if (props.sessionId) {
         await loadPersisted(run)
       }
       return
     }
 
-    // Locators land after the installer sandbox is up. Keep asking until the
-    // stream answers; falling through to the empty state on the first 404
-    // would flash "no record" during setup.
+    // Locators land after the installer sandbox is up. Hitting /transcript
+    // before that 404s every second (WARNING in the access log) and leaves
+    // the spinner up for the entire file seed, which can take minutes.
+    // The parent already polls the skill list; this watch re-opens when
+    // sessionId arrives.
+    if (!props.sessionId || !props.messageId) {
+      loading.value = false
+      return
+    }
+
+    loading.value = true
     for (;;) {
       if (stale() || !props.live) return
       const served = await follow(run).catch(() => false)
@@ -204,7 +212,7 @@ async function open() {
 }
 
 watch(
-  () => [props.configId, props.skillId, props.sessionId, props.live] as const,
+  () => [props.configId, props.skillId, props.sessionId, props.messageId, props.live] as const,
   () => {
     stop()
     if (props.configId && props.skillId) void open()

internal/agent/skills/manager.go

@@ -77,8 +77,9 @@ type Manager struct {
 	sandboxMgr sandbox.Manager
 
 	// tenantSource holds the skills installed into this run's sandbox image.
-	// It is nil for every run whose workspace has none, which is why the
-	// preloaded path below is untouched by its existence.
+	// When set it is the only source the model is told about: the host
+	// skills/preloaded directory is not what execute_skill_script would find
+	// inside the sandbox.
 	tenantSource SkillSource
 
 	// Configuration
@@ -129,70 +130,25 @@ func (m *Manager) WithTenantSource(source SkillSource) *Manager {
 	return m
 }
 
-// resolveSource decides which source owns one skill name. An installed skill
-// shadows a preloaded one of the same name, because the sandbox boots the
-// image the install produced and that is the copy a script would run.
+// resolveSource decides which source owns one skill name. An installed image
+// is the only copy the sandbox can run: falling back to a host preloaded
+// skill would advertise files that are not in the image.
 func (m *Manager) resolveSource(skillName string) SkillSource {
 	if m.tenantSource != nil {
-		if _, err := m.tenantSource.GetSkillBasePath(skillName); err == nil {
-			return m.tenantSource
-		}
+		return m.tenantSource
 	}
 	return m.loader
 }
 
-// discoverAllSkills merges the two sources into the set the model is told
-// about.
+// discoverAllSkills returns the set the model is told about. When skills are
+// installed into the sandbox image, that image is the source of truth; the
+// deployment's skills/preloaded directory is not what execute_skill_script
+// would find inside the sandbox.
 func (m *Manager) discoverAllSkills() ([]*SkillMetadata, error) {
-	preloaded, err := m.loader.DiscoverSkills()
-	if err != nil {
-		return nil, err
-	}
-	return m.mergeWithTenantSkills(preloaded)
-}
-
-// mergeWithTenantSkills overlays the installed skills on a freshly discovered
-// preloaded set.
-func (m *Manager) mergeWithTenantSkills(preloaded []*SkillMetadata) ([]*SkillMetadata, error) {
-	if m.tenantSource == nil {
-		return preloaded, nil
-	}
-	tenant, err := m.tenantSource.DiscoverSkills()
-	if err != nil {
-		return nil, err
-	}
-	return mergeSkillMetadata(preloaded, tenant), nil
-}
-
-// mergeSkillMetadata overlays the installed skills on the preloaded ones,
-// keeping the preloaded ordering for the names both sources carry so the
-// system prompt does not reshuffle when a skill is installed.
-func mergeSkillMetadata(preloaded, tenant []*SkillMetadata) []*SkillMetadata {
-	byName := make(map[string]*SkillMetadata, len(tenant))
-	for _, meta := range tenant {
-		if meta != nil {
-			byName[meta.Name] = meta
-		}
-	}
-	merged := make([]*SkillMetadata, 0, len(preloaded)+len(tenant))
-	overridden := make(map[string]bool, len(tenant))
-	for _, meta := range preloaded {
-		if meta == nil {
-			continue
-		}
-		if installed, ok := byName[meta.Name]; ok {
-			merged = append(merged, installed)
-			overridden[meta.Name] = true
-			continue
-		}
-		merged = append(merged, meta)
-	}
-	for _, meta := range tenant {
-		if meta != nil && !overridden[meta.Name] {
-			merged = append(merged, meta)
-		}
+	if m.tenantSource != nil {
+		return m.tenantSource.DiscoverSkills()
 	}
-	return merged
+	return m.loader.Reload()
 }
 
 // Initialize discovers all skills and caches their metadata
@@ -491,11 +447,7 @@ func (m *Manager) Reload(ctx context.Context) error {
 		return nil
 	}
 
-	preloaded, err := m.loader.Reload()
-	if err != nil {
-		return err
-	}
-	metadata, err := m.mergeWithTenantSkills(preloaded)
+	metadata, err := m.discoverAllSkills()
 	if err != nil {
 		return err
 	}

internal/agent/skills/tenant_source_test.go

@@ -188,8 +188,8 @@ func TestTenantSkillSourceReportsAMissingBundleWithoutBlockingExecution(t *testi
 	require.Equal(t, "/opt/weknora/tenant/skills/pdf/scripts/extract.py", remote)
 }
 
-func TestManagerPrefersTheTenantSkillOverASameNamedPreloadedOne(t *testing.T) {
-	dir := preloadedSkillDir(t, "pdf", "preloaded description")
+func TestManagerIgnoresPreloadedSkillsWhenTenantSourceIsAttached(t *testing.T) {
+	dir := preloadedSkillDir(t, "document-analyzer", "preloaded description")
 	mgr := NewManager(&ManagerConfig{SkillDirs: []string{dir}, Enabled: true}, nil)
 	mgr.WithTenantSource(NewTenantSkillSource([]*types.TenantSkillEntity{
 		{
@@ -209,13 +209,17 @@ func TestManagerPrefersTheTenantSkillOverASameNamedPreloadedOne(t *testing.T) {
 		byName[meta.Name] = meta
 	}
 	require.Len(t, byName, 2)
-	require.Equal(t, "tenant description", byName["pdf"].Description,
-		"the tenant's own install is what the sandbox image actually carries")
+	require.NotContains(t, byName, "document-analyzer",
+		"host preloaded skills are not in the sandbox image")
+	require.Equal(t, "tenant description", byName["pdf"].Description)
 	require.Equal(t, "tenant only", byName["csv"].Description)
 
 	skill, err := mgr.LoadSkill(context.Background(), "pdf")
 	require.NoError(t, err)
 	require.Equal(t, "tenant body", skill.Instructions)
+
+	_, err = mgr.LoadSkill(context.Background(), "document-analyzer")
+	require.Error(t, err, "a host-only skill must not be readable once the image is the source")
 }
 
 func TestManagerRunsATenantSkillFromTheImageWithoutUploading(t *testing.T) {
@@ -246,13 +250,10 @@ func TestManagerRunsATenantSkillFromTheImageWithoutUploading(t *testing.T) {
 
 // Preloaded skills keep uploading from the host and keep running in their own
 // directory; the tenant source must not change that path at all.
-func TestManagerKeepsPreloadedSkillExecutionUnchanged(t *testing.T) {
+func TestManagerKeepsPreloadedSkillExecutionWhenNoTenantSource(t *testing.T) {
 	dir := preloadedSkillDir(t, "pdf", "preloaded description")
 	sandboxMgr := &recordingSandboxManager{}
 	mgr := NewManager(&ManagerConfig{SkillDirs: []string{dir}, Enabled: true}, sandboxMgr)
-	mgr.WithTenantSource(NewTenantSkillSource([]*types.TenantSkillEntity{{
-		ID: "sk-1", Name: "csv", Status: types.SkillStatusReady, Enabled: true,
-	}}, nil))
 	require.NoError(t, mgr.Initialize(context.Background()))
 
 	_, err := mgr.ExecuteScript(context.Background(), "pdf", "scripts/run.py", nil, "")

internal/application/service/agent_service.go

@@ -237,20 +237,18 @@ func (s *agentService) CreateAgentEngine(
 		}
 	}
 
-	skillsEnabledWithDirs := config.SkillsEnabled && len(config.SkillDirs) > 0
-	// Initialize the skills/sandbox manager when skills are configured, or for
-	// the skill installer, which keeps skills off yet needs the sandbox shell.
-	// The second disjunct is deliberately NOT "the config lists a sandbox
-	// tool": shell_exec is a user-selectable entry in the tool picker, so that
-	// rule would hand a live sandbox shell to every stored agent record that
-	// already lists it. Install mode is settable only through
-	// EnableSkillInstallMode, so only the built-in installer passes here and
-	// every other agent keeps exactly its previous behaviour.
-	if skillsEnabledWithDirs || config.SkillInstallMode() {
+	// TenantSkills is the sandbox image. SkillDirs is the host
+	// skills/preloaded tree and is no longer filled on the QA path; it
+	// remains so tests (and any caller that still points at a host
+	// directory) can construct a manager. Install mode initializes for
+	// the shell without hanging a skills manager on the engine.
+	offerSkills := config.SkillsEnabled &&
+		(len(config.SkillDirs) > 0 || len(config.TenantSkills) > 0)
+	if offerSkills || config.SkillInstallMode() {
 		skillsManager, err := s.initializeSkillsManager(ctx, sessionID, config, toolRegistry)
 		if err != nil {
 			logger.Warnf(ctx, "Failed to initialize skills manager: %v", err)
-		} else if skillsEnabledWithDirs && skillsManager != nil {
+		} else if offerSkills && skillsManager != nil {
 			engine.SetSkillsManager(skillsManager)
 			logger.Infof(ctx, "Skills manager initialized with %d skills",
 				len(skillsManager.GetAllMetadata()))
@@ -838,6 +836,14 @@ func (s *agentService) registerTools(
 		case tools.ToolWikiDeletePage:
 			toolToRegister = tools.NewWikiDeletePageTool(s.wikiPageService, wikiKBIDs, wikiRoutes)
 
+		case tools.ToolShellExec, tools.ToolReadSkill, tools.ToolExecuteSkillScript,
+			tools.ToolListSandboxFiles, tools.ToolReadSandboxFile:
+			// Bound to the resolved sandbox manager in initializeSkillsManager
+			// / registerSandboxShellTool. Listing them here would warn
+			// "Unknown tool: shell_exec" on every skill install, then register
+			// the real tool a few lines later.
+			continue
+
 		default:
 			logger.Warnf(ctx, "Unknown tool: %s", toolName)
 		}
@@ -1178,6 +1184,11 @@ func (s *agentService) resolvePinnedSkillInfos(config *types.AgentConfig) []*age
 			}
 		}
 	}
+	for _, row := range config.TenantSkills {
+		if row != nil && row.Name != "" {
+			descByName[row.Name] = row.Description
+		}
+	}
 
 	result := make([]*agent.PinnedSkillInfo, 0, len(config.PinnedSkillNames))
 	for _, name := range config.PinnedSkillNames {