fix: address PR review findings on the settings route

Drop disabled plugin skills at discovery indexing, not only at register, so
getPluginSkill can no longer return one. Refetch subagents on every visit to
their page, since the cached list belongs to the session whose working
directory produced it. Match disabled skill names case-insensitively in the
web settings, as the core does. Keep a configured default model visible when
the catalog no longer offers it. Sum the combined desktop download counter
across both release repositories like the per-platform ones.

Author: elkaix

.github/workflows/desktop-download-badges.yml

@@ -50,6 +50,8 @@ jobs:
             gh api --paginate "repos/${repo}/releases"
           done | jq -s 'add' > releases.json
 
+          # An empty suffix matches every asset, which is what the combined
+          # counter wants; `endswith("")` is true for any string.
           write_badge() {
             local suffix="$1" label="$2" out="$3"
             local total
@@ -70,18 +72,23 @@ jobs:
 
           write_badge '.dmg' 'macOS .dmg' desktop-dmg.json
           write_badge '.exe' 'Windows .exe' desktop-exe.json
+          # The shields `github/downloads/.../total` route reads one repository,
+          # so the combined counter has to be summed here like the other two.
+          write_badge '' 'desktop' desktop-total.json
 
           rm -f releases.json
 
       - name: Publish if the counts moved
         run: |
           set -euo pipefail
-          if git diff --quiet -- desktop-dmg.json desktop-exe.json; then
+          git config user.name 'github-actions[bot]'
+          git config user.email 'github-actions[bot]@users.noreply.github.com'
+          # Stage before comparing: a document added in this run is untracked,
+          # and `git diff` on an untracked path reports no change at all.
+          git add desktop-dmg.json desktop-exe.json desktop-total.json
+          if git diff --cached --quiet; then
             echo 'Counts unchanged; nothing to publish.'
             exit 0
           fi
-          git config user.name 'github-actions[bot]'
-          git config user.email 'github-actions[bot]@users.noreply.github.com'
-          git add desktop-dmg.json desktop-exe.json
           git commit -m 'chore: refresh desktop download counts'
           git push origin badges

README.md

@@ -11,7 +11,7 @@
 
 [![npm version](https://img.shields.io/npm/v/@pymodel/pythinker-code?style=for-the-badge&logo=npm&logoColor=white&color=CB3837&label=pythinker-code)](https://www.npmjs.com/package/@pymodel/pythinker-code)
 [![Downloads](https://img.shields.io/npm/dm/@pymodel/pythinker-code?style=for-the-badge&logo=npm&logoColor=white&color=2b89ff&label=downloads)](https://www.npmjs.com/package/@pymodel/pythinker-code)
-[![Desktop downloads](https://img.shields.io/github/downloads/PyModel/pythinker-desktop-releases/total?style=for-the-badge&logo=github&logoColor=white&label=desktop&color=4D6BFE)](https://github.com/PyModel/pythinker-desktop-releases/releases)
+[![Desktop downloads](https://img.shields.io/endpoint?url=https%3A%2F%2Fraw.githubusercontent.com%2FPyModel%2Fpythinker-code%2Fbadges%2Fdesktop-total.json&style=for-the-badge&logo=github&logoColor=white)](https://github.com/PyModel/pythinker-desktop-releases/releases)
 [![macOS .dmg](https://img.shields.io/endpoint?url=https%3A%2F%2Fraw.githubusercontent.com%2FPyModel%2Fpythinker-code%2Fbadges%2Fdesktop-dmg.json&style=for-the-badge&logo=apple&logoColor=white)](https://github.com/PyModel/pythinker-desktop-releases/releases/latest)
 [![Windows .exe](https://img.shields.io/endpoint?url=https%3A%2F%2Fraw.githubusercontent.com%2FPyModel%2Fpythinker-code%2Fbadges%2Fdesktop-exe.json&style=for-the-badge&logo=windows&logoColor=white)](https://github.com/PyModel/pythinker-desktop-releases/releases/latest)
 [![Node.js](https://img.shields.io/badge/Node.js-26%2B-339933?style=for-the-badge&logo=nodedotjs&logoColor=white)](https://github.com/PyModel/pythinker-code/blob/main/package.json)

apps/pythinker-web/src/App.vue

@@ -611,7 +611,11 @@ const showAddWorkspace = ref(false);
 const showStatusPanel = ref(false);
 const showSettings = ref(false);
 
-const { activeTab: activeSettingsTab, setTab: selectSettingsTab } = useSettingsNav({
+const {
+  activeTab: activeSettingsTab,
+  setTab: selectSettingsTab,
+  refreshActiveTab: refreshSettingsTab,
+} = useSettingsNav({
   counts: {
     connectors: () => client.connectors.value.length,
     plugins: () => client.plugins.value.length,
@@ -622,8 +626,16 @@ const { activeTab: activeSettingsTab, setTab: selectSettingsTab } = useSettingsN
   onLoadSubagents: () => { void client.loadSubagents(); },
 });
 
+function openSettings(): void {
+  showSettings.value = true;
+  // The active tab persists across visits, so its data has to be refetched on
+  // open — the session it was loaded for may no longer be the active one.
+  refreshSettingsTab();
+}
+
 function toggleSettings(): void {
-  showSettings.value = !showSettings.value;
+  if (showSettings.value) showSettings.value = false;
+  else openSettings();
 }
 
 function loginFromSettings(): void {
@@ -961,7 +973,7 @@ function openPr(url: string): void {
         @rename-workspace="(id, name) => client.renameWorkspace(id, name)"
         @delete-workspace="(id) => client.deleteWorkspace(id)"
         @select-workspaces="handleSelectWorkspaces"
-        @open-settings="showSettings = true"
+        @open-settings="openSettings"
         @close-settings="showSettings = false"
         @select-settings-tab="selectSettingsTab($event)"
         @collapse="toggleSidebarCollapse"

apps/pythinker-web/src/components/settings/pages/AgentPage.vue

@@ -39,6 +39,14 @@ const modelOptions = computed<ModelOption[]>(() => {
   return Array.from(byId.values());
 });
 
+// A default the catalog no longer offers still has to appear, or the browser
+// falls back to the first option and shows a model that was never saved.
+const unlistedDefaultModel = computed<string | undefined>(() => {
+  const configured = props.config?.defaultModel;
+  if (configured === undefined || configured === '') return undefined;
+  return modelOptions.value.some((model) => model.id === configured) ? undefined : configured;
+});
+
 const modelGroups = computed<Array<{ provider: string; options: ModelOption[] }>>(() => {
   const map = new Map<string, ModelOption[]>();
   for (const option of modelOptions.value) {
@@ -122,6 +130,7 @@ function toggleConfigBoolean(key: 'defaultThinking' | 'defaultPlanMode' | 'merge
             @change="setDefaultModel"
           >
             <option v-if="!config.defaultModel" value="" disabled>{{ t('settings.noDefaultModel') }}</option>
+            <option v-if="unlistedDefaultModel" :value="unlistedDefaultModel">{{ unlistedDefaultModel }}</option>
             <optgroup v-for="group in modelGroups" :key="group.provider" :label="group.provider">
               <option v-for="model in group.options" :key="model.id" :value="model.id">{{ model.label }}</option>
             </optgroup>

apps/pythinker-web/src/components/settings/pages/SkillsPage.vue

@@ -35,18 +35,22 @@ const skillGroups = computed(() => {
     .toSorted((a, b) => a.source.localeCompare(b.source));
 });
 
-const disabledSkills = computed(() => new Set(props.config?.disabledSkills ?? []));
+// The core matches disabled names case-insensitively, so the page has to as
+// well — otherwise a config entry cased differently from the catalog reads as
+// enabled here and the switch never turns it back on.
+const disabledSkills = computed(() => props.config?.disabledSkills ?? []);
 const skillCount = computed(() => skillGroups.value.reduce((sum, group) => sum + group.skills.length, 0));
 
 function isSkillEnabled(name: string): boolean {
-  return !disabledSkills.value.has(name);
+  const key = name.toLowerCase();
+  return !disabledSkills.value.some((entry) => entry.toLowerCase() === key);
 }
 
 function toggleSkill(name: string): void {
-  const next = new Set(disabledSkills.value);
-  if (next.has(name)) next.delete(name);
-  else next.add(name);
-  emit('updateConfig', { disabledSkills: [...next].sort() });
+  const key = name.toLowerCase();
+  const next = disabledSkills.value.filter((entry) => entry.toLowerCase() !== key);
+  if (next.length === disabledSkills.value.length) next.push(name);
+  emit('updateConfig', { disabledSkills: next.toSorted() });
 }
 </script>
 

apps/pythinker-web/src/composables/useSettingsNav.ts

@@ -57,12 +57,24 @@ type UseSettingsNavOptions = {
 export function useSettingsNav(options: UseSettingsNavOptions) {
   const activeTab = shallowRef<SettingsTab>('general');
 
-  function setTab(tab: SettingsTab): void {
+  function loadFor(tab: SettingsTab): void {
     if (tab === 'connectors' && toValue(options.counts.connectors) === 0) options.onLoadConnectors();
     if (tab === 'plugins' && toValue(options.counts.plugins) === 0) options.onLoadPlugins();
-    if (tab === 'subagents' && toValue(options.counts.subagents) === 0) options.onLoadSubagents();
+    // Connectors and plugins are daemon-wide, so one load holds. Subagents are
+    // resolved from the active session's working directory, so a cached list
+    // belongs to whichever session was active when it loaded — always refetch.
+    if (tab === 'subagents') options.onLoadSubagents();
+  }
+
+  function setTab(tab: SettingsTab): void {
+    loadFor(tab);
     activeTab.value = tab;
   }
 
-  return { activeTab, setTab };
+  /** Call when the settings route opens; the active tab persists across visits. */
+  function refreshActiveTab(): void {
+    loadFor(activeTab.value);
+  }
+
+  return { activeTab, setTab, refreshActiveTab };
 }

apps/pythinker-web/test/settings-pane.test.ts

@@ -3,7 +3,9 @@ import { nextTick } from 'vue';
 import { createI18n } from 'vue-i18n';
 import { afterEach, describe, expect, it, vi } from 'vitest';
 
+import App from '../src/App.vue';
 import type { AppConfig, AppConnector, AppModel, AppSession, AppSkill } from '../src/api/types';
+import ConversationPane from '../src/components/ConversationPane.vue';
 import SettingsNav from '../src/components/settings/SettingsNav.vue';
 import SettingsPane from '../src/components/settings/SettingsPane.vue';
 import { messages } from '../src/i18n/locales';
@@ -82,9 +84,6 @@ vi.mock('../src/composables/usePythinkerWebClient', async () => {
   };
 });
 
-import App from '../src/App.vue';
-import ConversationPane from '../src/components/ConversationPane.vue';
-
 const i18n = createI18n({
   legacy: false,
   locale: 'en',
@@ -240,6 +239,35 @@ describe('settings navigation', () => {
     setTab('subagents');
     expect(onLoadSubagents).toHaveBeenCalledOnce();
   });
+
+  it('reloads subagents even when a list is already cached', () => {
+    // The cached list belongs to whichever session was active when it loaded,
+    // so a non-empty count must not suppress the refetch.
+    const onLoadSubagents = vi.fn();
+    const { setTab } = useSettingsNav({
+      counts: { connectors: 0, plugins: 0, subagents: 3 },
+      onLoadConnectors: vi.fn(),
+      onLoadPlugins: vi.fn(),
+      onLoadSubagents,
+    });
+
+    setTab('subagents');
+    expect(onLoadSubagents).toHaveBeenCalledOnce();
+  });
+
+  it('reloads the persisted tab when the settings route reopens', () => {
+    const onLoadSubagents = vi.fn();
+    const { setTab, refreshActiveTab } = useSettingsNav({
+      counts: { connectors: 0, plugins: 0, subagents: 3 },
+      onLoadConnectors: vi.fn(),
+      onLoadPlugins: vi.fn(),
+      onLoadSubagents,
+    });
+
+    setTab('subagents');
+    refreshActiveTab();
+    expect(onLoadSubagents).toHaveBeenCalledTimes(2);
+  });
 });
 
 describe('SettingsPane config controls', () => {
@@ -297,6 +325,19 @@ describe('SettingsPane desktop updates', () => {
   });
 });
 
+describe('SettingsPane agent page', () => {
+  it('keeps a configured default the catalog no longer offers', () => {
+    // Without a matching option the browser shows its first one, which reads
+    // as a saved default that was never chosen.
+    const wrapper = mountPane('agent', { config: { ...config, defaultModel: 'retired/model' } });
+    const select = wrapper.get('#settings-panel-agent select.select-field');
+
+    expect(select.findAll('option').map((option) => option.attributes('value')))
+      .toContain('retired/model');
+    expect((select.element as HTMLSelectElement).value).toBe('retired/model');
+  });
+});
+
 describe('SettingsPane skills page', () => {
   it('groups skills by source and marks the slash-only ones', () => {
     const panel = mountPane('skills', { skills }).get('#settings-panel-skills');
@@ -309,6 +350,23 @@ describe('SettingsPane skills page', () => {
   it('says so when no skill is available', () => {
     expect(mountPane('skills').get('#settings-panel-skills').text()).toContain('No skills are available');
   });
+
+  it('reads a disabled name that is cased differently as off, and clears it once', async () => {
+    // The core lowercases disabled names, so a config entry cased differently
+    // from the catalog still disables the skill and the page has to agree.
+    const wrapper = mountPane('skills', {
+      skills,
+      config: { ...config, disabledSkills: ['Gen-Changesets'] },
+    });
+    const row = wrapper.get('#settings-panel-skills').findAll('.listing-row')
+      .find((candidate) => candidate.text().includes('gen-changesets'));
+
+    expect(row?.classes()).toContain('off');
+
+    await row?.get('button.switch').trigger('click');
+
+    expect(wrapper.emitted('updateConfig')?.at(-1)).toEqual([{ disabledSkills: [] }]);
+  });
 });
 
 describe('SettingsPane connectors page', () => {

packages/agent-core/src/services/catalog/catalog.ts

@@ -1,20 +1,3 @@
-/**
- * `ICatalogService` — daemon-facing view of what the agent can reach beyond
- * skills and MCP servers: installed plugins and the subagent profiles.
- *
- * Both are read from the core process and both are global rather than
- * session-scoped, so they share one service instead of two near-identical
- * wrappers around `ICoreProcessService.rpc`.
- *
- * **CoreAPI surface used**:
- *   - `core.rpc.listPlugins({})` → `readonly PluginSummary[]`
- *   - `core.rpc.setPluginEnabled({id, enabled})`
- *   - `core.rpc.listAgentProfiles({workDir})` → `AgentProfileCatalog`
- *
- * **Anti-corruption**: imports `@pymodel/agent-core` internals only for the
- * `createDecorator` value and the rpc payload types.
- */
-
 import { createDecorator } from '../../di';
 import type { AgentProfileSummary } from '../../rpc';
 import type { PluginSummary } from '../../plugin';

packages/agent-core/src/skill/registry.ts

@@ -167,6 +167,9 @@ export class SessionSkillRegistry implements AgentSkillRegistry {
     options: { readonly replace?: boolean } = {},
   ): void {
     if (skill.plugin === undefined) return;
+    // Discovery indexes plugin skills before `register` runs, so the disabled
+    // check has to repeat here or `getPluginSkill` would still hand one back.
+    if (this.disabledNames.has(normalizeSkillName(skill.name))) return;
     const key = pluginSkillKey(skill.plugin.id, skill.name);
     if (options.replace === true || !this.byPluginAndName.has(key)) {
       this.byPluginAndName.set(key, skill);

packages/agent-core/test/skill/registry.test.ts

@@ -197,6 +197,27 @@ describe('disabled skills', () => {
 
     expect(registry.getSkill('loop')).toBeUndefined();
   });
+
+  it('never indexes a disabled plugin skill discovered from a root', async () => {
+    // Discovery indexes plugin skills before `register` runs, so a disabled one
+    // stays reachable through `getPluginSkill` unless the check repeats there.
+    const disabled = { ...makeSkill('deploy', 'extra'), plugin: { id: 'acme' } };
+    const kept = { ...makeSkill('rollback', 'extra'), plugin: { id: 'acme' } };
+    const registry = new SessionSkillRegistry({
+      disabledNames: ['Deploy'],
+      discover: async (options) => {
+        options.onDiscoveredSkill?.(disabled);
+        options.onDiscoveredSkill?.(kept);
+        return [disabled, kept];
+      },
+    });
+
+    await registry.loadRoots([{ path: '/tmp/plugins', source: 'extra' }]);
+
+    expect(registry.getPluginSkill('acme', 'deploy')).toBeUndefined();
+    expect(registry.getSkill('deploy')).toBeUndefined();
+    expect(registry.getPluginSkill('acme', 'rollback')).toBeDefined();
+  });
 });
 
 function makeRegistry(skills: readonly SkillDefinition[]): SessionSkillRegistry {