fix(codex): use app-server native skills

[swear01]

Summary

• load Codex skills from app-server skills/list
• expose only enabled native skills through HAPI $ completion
• translate a leading known $skill into Codex structured skill input
• preserve unknown/disabled $ text and existing file mentions
• reload with forceReload: true when app-server emits skills/changed
• keep the filesystem handler as a fallback when native discovery fails
• refetch the catalog when $ completion opens so handler replacement/refresh is visible immediately

Tests

• bun typecheck
• bun run test (CLI 1646, hub 692, web 1760, shared 152)

Closes #1287
Part of #1284

[github-actions]

Findings

• [Major] Refresh native skills after app-server invalidation - nativeSkills is a launch-time snapshot used by both completion and structured turn input, so skill installs/removals/config changes during a long-lived session remain stale. Evidence cli/src/codex/codexRemoteLauncher.ts:3178, cli/src/codex/codexRemoteLauncher.ts:3720
  Suggested fix:

  const refreshNativeSkills = async (forceReload: boolean) => {
      const response = await appServerClient.listSkills({
          cwds: [session.path],
          forceReload
      });
      const inventory = response.data?.find(entry => entry.cwd === session.path)
          ?? response.data?.[0];
      nativeSkills = (inventory?.skills ?? []).filter(skill => skill.enabled);
  };
  
  if (method === 'skills/changed') {
      void refreshNativeSkills(true);
  }

• [Major] Preserve the filesystem fallback for resolved discovery failures - skills/list reports per-CWD failures through errors, but the new code treats those successful JSON-RPC responses as an empty native catalog and overwrites the existing fallback handler. Evidence cli/src/codex/appServerTypes.ts:83, cli/src/codex/codexRemoteLauncher.ts:3176
  Suggested fix:

  if (!inventory || (
      inventory.skills.length === 0
      && (inventory.errors?.length ?? 0) > 0
  )) {
      throw new Error('skills/list returned no usable inventory');
  }

Summary
Review mode: initial

Two correctness issues found in native skill catalog lifecycle/fallback handling.

Testing
Not run (automation); workflow test is pending. Add coverage for a skills/changed refresh affecting both RPC completion and the next turn, plus a resolved skills/list response with errors preserving the common filesystem handler.

HAPI Bot

[github-actions]

[MAJOR] Refresh the native catalog after skills/changed

This assignment is the only population of nativeSkills, while the app-server emits skills/changed as an invalidation signal. The same snapshot is later used by both the listSkills RPC and every structured turn input, so installing, enabling, disabling, or removing a skill during a long-lived remote session leaves completion stale; removed paths can still be sent as type: 'skill'.

Suggested fix:

const refreshNativeSkills = async (forceReload: boolean) => {
    const response = await appServerClient.listSkills({
        cwds: [session.path],
        forceReload
    });
    const inventory = response.data?.find(entry => entry.cwd === session.path)
        ?? response.data?.[0];
    nativeSkills = (inventory?.skills ?? []).filter(skill => skill.enabled);
};

if (method === 'skills/changed') {
    void refreshNativeSkills(true).catch((error) => {
        logger.debug(`[Codex] skills refresh failed: ${errorMessage(error)}`);
    });
}

Declare the refresh state before the notification handler, call it once after initialization, and add a test that changes the mocked response before emitting skills/changed.

[github-actions]

[MAJOR] Do not replace the filesystem fallback with an errored empty catalog

The response type added in this PR explicitly carries per-CWD errors. Those are successful JSON-RPC responses, so this code does not enter the catch; missing inventory or skills: [] plus errors is converted to nativeSkills = [], then the common filesystem listSkills handler is overwritten with a successful empty response. That defeats the stated fallback exactly when native discovery fails.

Suggested fix:

if (!inventory || (
    inventory.skills.length === 0
    && (inventory.errors?.length ?? 0) > 0
)) {
    throw new Error('skills/list returned no usable inventory');
}

nativeSkills = inventory.skills.filter(skill => skill.enabled);

Keep partial native results when usable skills exist, but leave the pre-registered handler intact for a full per-CWD failure. Add a response-with-errors test.

[github-actions]

Findings

• [Major] Preserve the filesystem fallback on per-CWD discovery errors — skills/list can return a resolved response with errors; the current refresh converts that failure into an empty native catalog and overwrites the common filesystem handler. Evidence cli/src/codex/codexRemoteLauncher.ts:3099
  Suggested fix:

  if (!inventory || (
      inventory.skills.length === 0
      && (inventory.errors?.length ?? 0) > 0
  )) {
      throw new Error('skills/list returned no usable inventory');
  }
  
  nativeSkills = inventory.skills.filter(skill => skill.enabled);

Summary
Review mode: follow-up after new commits

The new commits handle skills/changed refreshes, but the prior discovery-error fallback issue remains.

Testing
Not run (automation); GitHub test workflow pending. Add a resolved skills/list response test with skills: [] and non-empty errors, asserting the pre-registered filesystem handler remains active.

HAPI Bot

[github-actions]

[MAJOR] Keep the filesystem handler when native discovery reports errors

skills/list reports per-CWD failures inside a resolved response's errors array. This assignment therefore turns a failed discovery into nativeSkills = []; the following registration then overwrites the common filesystem listSkills handler with a successful empty result, so completion disappears and a selected $skill cannot be translated into structured input. The outer catch is never reached for this response shape.

Suggested fix:

if (!inventory || (
    inventory.skills.length === 0
    && (inventory.errors?.length ?? 0) > 0
)) {
    throw new Error('skills/list returned no usable inventory');
}

nativeSkills = inventory.skills.filter(skill => skill.enabled);

Validate before mutating nativeSkills or registering the native handler, and add coverage for a resolved response containing skills: [] plus non-empty errors.

[github-actions]

Findings

• [Major] Refresh the web completion cache when the native catalog replaces or updates the RPC handler — app-server discovery is asynchronous, so the session page can fetch the pre-existing filesystem handler before cli/src/codex/codexRemoteLauncher.ts:3105 replaces it. The web then keeps that response with staleTime: Infinity at web/src/hooks/queries/useSkills.ts:45; neither the initial replacement nor later skills/changed refreshes invalidate it. Disabled/removed skills can therefore remain selectable indefinitely, while newly enabled skills never appear until the query is garbage-collected or manually reloaded.

  Suggested fix:

  // web/src/hooks/queries/useSkills.ts
  const getSuggestions = useCallback(async (queryText: string): Promise<Suggestion[]> => {
      const refreshed = queryText === '$' ? await query.refetch() : null;
      const currentSkills = refreshed?.data?.success
          ? (refreshed.data.skills ?? [])
          : skills;
  
      // Rank/filter currentSkills below instead of the cached skills array.
  }, [query.refetch, skills]);

Summary
Review mode: follow-up after new commits

The prior filesystem-fallback finding is fixed. One Major cache-invalidation regression remains: the native catalog can be loaded correctly in the CLI while HAPI's visible $ completion continues using the earlier filesystem response.

Testing
Not run (automation); GitHub test check passed. Add a regression covering an initial filesystem response followed by native-handler registration, plus a skills/changed refresh, and assert reopening $ shows the latest enabled native catalog.

HAPI Bot

[github-actions]

[MAJOR] Refresh the web skill cache after installing/updating this handler

This replaces the filesystem listSkills handler only after asynchronous app-server discovery. If the session page fetched skills first, useSkills has already cached that filesystem response forever (web/src/hooks/queries/useSkills.ts:45). Replacing this handler—and later mutating nativeSkills on skills/changed—does not invalidate the TanStack Query entry, so disabled/removed entries stay visible and new native skills do not appear.

A minimal fix is to refetch whenever the $ completion menu opens (or propagate a catalog-changed event that invalidates queryKeys.skills(sessionId)):

// web/src/hooks/queries/useSkills.ts
const getSuggestions = useCallback(async (queryText: string): Promise<Suggestion[]> => {
    const refreshed = queryText === '$' ? await query.refetch() : null;
    const currentSkills = refreshed?.data?.success
        ? (refreshed.data.skills ?? [])
        : skills;

    // Rank/filter currentSkills below instead of skills.
}, [query.refetch, skills]);

Add coverage for both the initial filesystem→native swap and a later skills/changed refresh.

[github-actions]

Findings

• No findings.

Summary
Review mode: follow-up after new commits

No correctness, security, regression, data-loss, performance, or maintainability issues found in the added or modified lines. The previous native-skill completion cache finding is resolved. Residual testing gap: the web refetch behavior added in web/src/hooks/queries/useSkills.ts has no automated regression coverage.

Testing
Not run (automation); GitHub test and pr-review checks were pending at review time. Suggested coverage: seed a stale filesystem skill response, replace it with the native RPC catalog, open $ completion, and assert the refreshed native list is rendered.

HAPI Bot