fix(claude): separate skills from slash commands

[swear01]

Summary

• read Claude's native skills and mixed slash_commands from the SDK init catalog
• keep native skills out of / and expose only loaded names through $
• preserve plugin namespaces such as ponytail:ponytail, including --plugin-dir and --plugin-url catalogs
• pass catalog-affecting launch arguments to both the metadata probe and remote Claude process
• translate a leading advertised $skill to Claude native /skill input, keeping attachment references after the command (including whitespace-bearing paths)
• wait for session/catalog readiness before draining startup messages, while preserving cancellation and a 10-second probe timeout
• retry catalog discovery after transient probe failure and surface failures as query errors instead of cached empty data

Live check

A real Claude init probe exposed distinct native skills, mixed slash_commands, and URL plugin metadata. A temporary URL-only plugin appeared as url-probe:url-skill in the native skill list and was classified into $, not /.

Tests

• bun typecheck
• focused Claude metadata/query/launcher tests
• GitHub test check passes (full repository suite)
• HAPI Bot follow-up review: no findings

Closes #1290
Part of #1284

[github-actions]

Findings

• [Major] The Claude-specific ListSkills handler is installed only after background metadata extraction, so an earlier request can return and permanently cache the unfiltered filesystem inventory. Evidence: cli/src/claude/runClaude.ts:92.
• [Major] Messages with attachments prepend @path text before the new expansion runs, so a leading advertised $skill is not translated or invoked. Evidence: cli/src/claude/claudeRemoteLauncher.ts:452.

Summary

Review mode: initial

Two correctness issues found in the new skill advertisement/invocation path. CI checks were still pending at review time.

Testing

Not run (automation). Add coverage for a /skills request before SDK metadata resolves, and for $skill invocation with at least one attachment.

HAPI Bot

[github-actions]

[MAJOR] Wait for the classified catalog before serving ListSkills

ApiSessionClient already registers the common skills handler during construction (cli/src/api/apiSession.ts:271). This replacement happens only after the background SDK query and filesystem scan complete. If the web requests /skills first, it receives every discovered skill, including entries Claude did not load; useSkills then keeps that response with staleTime: Infinity (web/src/hooks/queries/useSkills.ts:45). The later handler replacement does not refresh that cache, so the PR can still advertise unusable $skills for the whole page lifetime.

Suggested fix:

const catalogPromise = extractSDKMetadata(workingDirectory).then(async (sdkMetadata) =>
    classifyClaudeSlashCatalog(
        sdkMetadata.slashCommands,
        await listSkills(workingDirectory, { flavor: "claude" })
    )
)

session.rpcHandlerManager.registerHandler(RPC_METHODS.ListSkills, async () => ({
    success: true,
    skills: (await catalogPromise).skills
}))

Use the same promise to update metadata/native names, so the first RPC waits for classification instead of falling through to the common handler.

[github-actions]

[MAJOR] Preserve the slash invocation when attachments are present

At this point msg.message has already passed through formatMessageWithAttachments, which prepends attachment references (@path\n\n) in cli/src/utils/attachmentFormatter.ts:29. Therefore expandSkillReference() cannot match its anchored ^\\s*\\$ pattern, and $hapi inspect plus any attachment is sent literally instead of as Claude native /hapi. Translate the raw user text first and keep the resulting slash command at the beginning of the final prompt.

Suggested fix:

const rawText = message.content.text
const expandedText = currentSessionRef.current?.expandSkillReference(rawText) ?? rawText
const attachmentText = formatAttachmentsForClaude(message.content.attachments)
const formattedText = expandedText !== rawText && attachmentText
    ? `${expandedText}\n\n${attachmentText}`
    : formatMessageWithAttachments(expandedText, message.content.attachments)

Add a test asserting the SDK receives /hapi inspect first when the message also contains an attachment.

[github-actions]

Findings

• [Major] Attachment detection still fails for paths containing whitespace, leaving an advertised $skill literal instead of invoking Claude’s native slash skill. Evidence: cli/src/claude/session.ts:109.
  Suggested fix:

  const skillNames = new Set(nativeSkills?.map((skill) => skill.name) ?? [])
  const match = /^\s*\$([^\s]+)(?=\s|$)/.exec(message.content.text)
  const expandedText = match && skillNames.has(match[1])
      ? `/${match[1]}${message.content.text.slice(match[0].length)}`
      : message.content.text
  const attachmentText = formatAttachmentsForClaude(message.content.attachments)
  const formattedText = expandedText !== message.content.text && attachmentText
      ? `${expandedText}\n\n${attachmentText}`
      : formatMessageWithAttachments(expandedText, message.content.attachments)

Summary

Review mode: follow-up after new commits

The prior early-ListSkills race is resolved. The attachment fix remains incomplete for whitespace-bearing paths because it reconstructs attachment structure from the formatted string.

Testing

Not run (automation). Add a launcher test using an attachment path such as C:\\Users\\Jane Doe\\input.txt; CI test workflow was still in progress during review.

HAPI Bot

[github-actions]

[MAJOR] Do not infer attachment structure by splitting rendered paths

formatMessageWithAttachments() emits the attachment prefix as @${path}. Splitting that prefix on whitespace means a path such as C:\\Users\\Jane Doe\\input.txt produces a Doe... token that does not start with @, so hasAttachments becomes false and the following advertised $hapi is sent literally. Attachment paths are plain strings in the protocol, and HAPI supports Windows/custom temp directories, so whitespace is valid here.

Expand the raw user text while the structured attachment list is still available, then put the attachment references after the native slash invocation. For example in runClaude.ts:

const skillNames = new Set(nativeSkills?.map((skill) => skill.name) ?? [])
const match = /^\s*\$([^\s]+)(?=\s|$)/.exec(message.content.text)
const expandedText = match && skillNames.has(match[1])
    ? `/${match[1]}${message.content.text.slice(match[0].length)}`
    : message.content.text
const attachmentText = formatAttachmentsForClaude(message.content.attachments)
const formattedText = expandedText !== message.content.text && attachmentText
    ? `${expandedText}\n\n${attachmentText}`
    : formatMessageWithAttachments(expandedText, message.content.attachments)

Then remove the formatted-string attachment heuristic and add coverage for a whitespace-bearing path.

[github-actions]

Findings

• [Major] Catalog extraction does not use the Claude launch arguments that shape the actual catalog, so sessions started with pass-through plugin/settings flags can advertise and translate a different command set than Claude loads. Evidence: cli/src/claude/runClaude.ts:83.
  Suggested fix:

  const catalogArgs = filterCatalogAffectingClaudeArgs(options.claudeArgs)
  extractSDKMetadata({
      cwd: workingDirectory,
      claudeArgs: catalogArgs
  })

• [Minor] A failed metadata probe is treated as an authoritative empty catalog and overwrites preserved slash-command metadata on resumed sessions. Evidence: cli/src/claude/runClaude.ts:99.
  Suggested fix:

  if (sdkMetadata.slashCommands === undefined) {
      if (sdkMetadata.tools !== undefined) {
          session.updateMetadata((metadata) => ({ ...metadata, tools: sdkMetadata.tools }))
      }
      return
  }

Summary

Review mode: follow-up after new commits

The prior whitespace-bearing attachment-path issue is resolved. The fresh head still has one catalog/config mismatch and one extraction-failure regression.

Testing

Not run (automation). CI checks were pending during review. Missing coverage: catalog-affecting Claude pass-through args and an extractor result with no slashCommands.

HAPI Bot

[github-actions]

[MAJOR] Mirror catalog-affecting Claude launch arguments

This probe launches a separate vanilla Claude process. The actual local session later appends options.claudeArgs in claudeLocal.ts:87-92, and the CLI intentionally forwards unknown Claude flags. Catalog-shaping flags such as plugin/settings inputs can therefore make the real session load skills or commands that this probe never sees. Those entries disappear from both suggestion lists, and their advertised $skill form cannot be translated because nativeSkillNames came from the mismatched probe.

Pass the safe catalog-affecting subset into the probe, while excluding stateful flags such as resume/continue:

const catalogArgs = filterCatalogAffectingClaudeArgs(options.claudeArgs)
extractSDKMetadata({
    cwd: workingDirectory,
    claudeArgs: catalogArgs
})

Then append that subset to the metadata-query process arguments. Add a test where a pass-through plugin argument changes slash_commands.

[github-actions]

[MINOR] Preserve the cached catalog when extraction returns no catalog

extractSDKMetadata() catches probe errors and returns {}. This continuation still classifies that as an empty catalog and writes slashCommands: [], so reactivating a session erases the slash-command metadata deliberately preserved by sessionFactory.ts. The previous async wrapper skipped the update when neither capability was present. A transient probe failure now makes command suggestions disappear and the skills RPC reports a successful empty list.

Guard the unavailable field before applying the catalog:

if (sdkMetadata.slashCommands === undefined) {
    if (sdkMetadata.tools !== undefined) {
        session.updateMetadata((metadata) => ({ ...metadata, tools: sdkMetadata.tools }))
    }
    return
}

Also return an RPC error or another explicit unavailable state instead of success: true with an empty skill list.

[github-actions]

Findings

• [Major] Pass-through --plugin-dir skills are still absent from the classifier inventory, so they remain under / and cannot be invoked through the advertised $ path. Evidence: cli/src/claude/runClaude.ts:84.
• [Minor] A failed metadata probe is still returned as a successful empty skill catalog and cached for the session. Evidence: cli/src/claude/runClaude.ts:89.

Summary

Review mode: follow-up after new commits

Two remaining correctness gaps: launch-time plugin roots are not scanned for classification, and probe failure is indistinguishable from a genuinely empty catalog.

Testing

Not run (automation). PR code was not executed. CI checks were pending at review time. Missing coverage: a skill loaded only through --plugin-dir, and a ListSkills request after metadata extraction returns no slashCommands.

HAPI Bot

[github-actions]

[MAJOR] Include pass-through plugin roots in the skill inventory

The probe now receives --plugin-dir, but this parallel listSkills() call only scans cwd/user roots plus the installed-plugin cache (cli/src/modules/common/skills.ts:282-320). A plugin supplied only through hapi --plugin-dir /path/to/plugin can therefore appear in Claude's slash_commands while its SKILL.md is absent from discoveredSkills. classifyClaudeSlashCatalog() then keeps that skill under /, omits it from $, and never adds its name to nativeSkillNames.

Extend skill discovery with each pass-through plugin's skills root:

const pluginSkillRoots = getClaudePluginSkillRoots(options.claudeArgs)
const catalogPromise = Promise.all([
    extractSDKMetadata({ cwd: workingDirectory, claudeArgs: options.claudeArgs }),
    listSkills(workingDirectory, {
        flavor: 'claude',
        additionalRoots: pluginSkillRoots
    })
])

The helper should handle both --plugin-dir value and --plugin-dir=value, mapping each plugin directory to its skills subdirectory. Add a fixture whose skill exists only in that root.

[github-actions]

[MINOR] Do not report probe failure as a successful empty catalog

extractSDKMetadata() catches probe failures/no-init cases and returns {} (cli/src/claude/sdk/metadataExtractor.ts:116-126). This handler still classifies that as [] and returns success: true. The metadata-update guard below preserves resumed slash commands, but the skills endpoint remains an authoritative empty result; the web query uses staleTime: Infinity and retry: false, so the session can keep no skill suggestions after a transient probe failure.

Return an unavailable result when the probe did not provide a catalog:

session.rpcHandlerManager.registerHandler(RPC_METHODS.ListSkills, async () => {
    const { sdkMetadata, catalog } = await catalogPromise
    if (sdkMetadata.slashCommands === undefined) {
        return { success: false, error: 'Claude skill catalog unavailable' }
    }
    return { success: true, skills: catalog.skills }
})

The web query should convert success: false into an error so it is not cached as normal data.

[github-actions]

Findings

• [Major] URL-loaded plugin skills are not added to the classifier inventory. --plugin-url is forwarded to Claude, but only --plugin-dir contributes discovery roots; URL-only skills remain under / and never gain the advertised $ invocation. Evidence: cli/src/claude/sdk/metadataExtractor.ts:25.
  Suggested fix:

  const additionalRoots = await resolveClaudePluginSkillRoots(options.claudeArgs, workingDirectory)
  const [sdkMetadata, discoveredSkills] = await Promise.all([
      extractSDKMetadata({ cwd: workingDirectory, claudeArgs: options.claudeArgs }),
      listSkills(workingDirectory, { flavor: 'claude', additionalRoots })
  ])

• [Minor] An unavailable metadata catalog still becomes a session-lifetime result. The settled catalogPromise is never rebuilt, and the web query caches { success: false } as normal data with infinite staleness, so a transient probe failure cannot recover. Evidence: cli/src/claude/runClaude.ts:94.
  Suggested fix:

  if (result.sdkMetadata.slashCommands === undefined) {
      catalogPromise = null
      return { success: false, error: 'Claude skill catalog unavailable' }
  }
  // web queryFn: throw when result.success is false so failure is not cached as data

Summary

Review mode: follow-up after new commits

The new commits address the prior --plugin-dir inventory and false-success response findings, but URL-loaded plugins and transient probe recovery remain incomplete.

Testing

Not run (automation). PR code was not executed. CI checks were pending at review time. Missing coverage: a skill supplied only through --plugin-url, and retry/recovery after the first metadata probe returns no slashCommands.

HAPI Bot

[github-actions]

[MAJOR] Inventory --plugin-url skills before classification

This flag is forwarded into both the metadata probe and the remote Claude launch, so URL-loaded plugin entries can appear in slash_commands. However, runClaude.ts:84-87 augments discoveredSkills only with roots parsed from --plugin-dir. A skill supplied only through --plugin-url therefore misses skillsByName, remains in catalog.commands, and never gets advertised or expanded through $plugin:skill.

Resolve URL plugin sources to local skill roots (or wait for the probe to materialize them, then scan the resulting cache) before classifying:

const additionalRoots = await resolveClaudePluginSkillRoots(
    options.claudeArgs,
    workingDirectory
)
const [sdkMetadata, discoveredSkills] = await Promise.all([
    extractSDKMetadata({ cwd: workingDirectory, claudeArgs: options.claudeArgs }),
    listSkills(workingDirectory, { flavor: 'claude', additionalRoots })
])

Add a fixture whose only skill source is --plugin-url.

[github-actions]

[MINOR] Make an unavailable catalog retryable

Changing this branch to success: false avoids an authoritative empty list, but catalogPromise has already settled and is reused for the entire session. In addition, web/src/hooks/queries/useSkills.ts:32-43 returns the { success: false } body as successful TanStack Query data with staleTime: Infinity and retry: false. A transient no-init/probe failure therefore leaves skill suggestions unavailable until a new session.

Reset the failed loader and make the web query treat the response as an error:

if (result.sdkMetadata.slashCommands === undefined) {
    catalogPromise = null
    return { success: false, error: 'Claude skill catalog unavailable' }
}

const result = await api.getSkills(sessionId)
if (!result.success) throw new Error(result.error ?? 'Failed to load skills')
return result

Build catalogPromise lazily through a helper so the next request can recreate it, and cover failure-then-success.

[github-actions]

Findings

• [Minor] Startup-queued attachments can bypass native skill translation — ApiSessionClient.onUserMessage immediately drains messages received before its callback is registered (cli/src/api/apiSession.ts:603), but currentSessionRef is not assigned until onSessionReady (cli/src/claude/runClaude.ts:508). For a queued $hapi inspect with an attachment, the new fallback therefore prepends @path at cli/src/claude/runClaude.ts:341; the later launcher rewrite only recognizes a $skill at the absolute start (cli/src/claude/claudeRemoteLauncher.ts:452), so Claude receives ordinary text instead of /hapi inspect.
  Suggested fix:

  let resolveSessionReady!: (session: Session) => void
  const sessionReady = new Promise<Session>((resolve) => {
      resolveSessionReady = resolve
  })
  
  // Register the existing user-message handler only after both prerequisites
  // settle, so pendingMessages drain with a Session and catalog available.
  void Promise.allSettled([sessionReady, getCatalog()]).then(() => {
      session.onUserMessage(handleUserMessage)
  })
  
  // in onSessionReady
  currentSessionRef.current = sessionInstance
  resolveSessionReady(sessionInstance)

Summary

Review mode: follow-up after new commits

One correctness issue remains in the startup/replay path for attached native-skill prompts. The previous bot findings are no longer present in the latest full diff.

Testing

Not run (automation). PR code was not executed. CI test was still in progress when checked. Add a test that delivers an attached $skill before onSessionReady and verifies the SDK receives /skill ... with attachment context; retry-after-empty-catalog coverage is also still missing.

HAPI Bot

[github-actions]

[MINOR] Delay pending attached-skill formatting until readiness

onUserMessage drains messages that arrived before callback registration, while currentSessionRef is assigned only later in onSessionReady. In that startup/replay case this line cannot expand $skill, so the fallback prepends the attachment. claudeRemoteLauncher.ts:452 then sees a prompt beginning with @, not $, and sends the skill reference as ordinary text.

Suggested fix:

let resolveSessionReady!: (session: Session) => void
const sessionReady = new Promise<Session>((resolve) => {
    resolveSessionReady = resolve
})

void Promise.allSettled([sessionReady, getCatalog()]).then(() => {
    session.onUserMessage(handleUserMessage)
})

// onSessionReady
currentSessionRef.current = sessionInstance
resolveSessionReady(sessionInstance)

Please cover a message queued before readiness with both $hapi inspect and an attachment.

[github-actions]

Findings

• [Major] Cancellation can miss messages held behind catalog readiness — the added readiness gate leaves incoming messages in ApiSessionClient.pendingMessages until both promises settle, but the cancellation callback only searches MessageQueue2. A cancel during that window returns removed: false; the hub treats that as already invoked, then onUserMessage later drains and executes the supposedly cancelled prompt. Evidence cli/src/claude/runClaude.ts:438, with the pending drain at cli/src/api/apiSession.ts:603 and false-ack handling at hub/src/sync/messageService.ts:469.
  Suggested fix:

  type DeferredMessage = Parameters<Parameters<typeof session.onUserMessage>[0]>
  const deferredMessages: DeferredMessage[] = []
  let messagePipelineReady = false
  
  session.onUserMessage((message, localId) => {
      if (!messagePipelineReady) {
          deferredMessages.push([message, localId])
          return
      }
      handleUserMessage(message, localId)
  })
  
  void Promise.allSettled([sessionReady, getCatalog()]).then(() => {
      messagePipelineReady = true
      for (const [message, localId] of deferredMessages.splice(0)) {
          handleUserMessage(message, localId)
      }
  })
  
  session.onCancelQueuedMessage((localId) => {
      const index = deferredMessages.findIndex(([, id]) => id === localId)
      if (index >= 0) {
          deferredMessages.splice(index, 1)
          return true
      }
      return messageQueue.cancelByLocalId(localId)
  })

Summary

Review mode: follow-up after new commits

The prior attached-skill startup issue is addressed, but the new readiness staging introduces one cancellation race that can still execute a prompt after the user tries to cancel it.

Testing

Not run locally (automation/security constraint). CI test job passed. Add a test that delivers a message before catalog readiness, cancels its localId, then resolves readiness and verifies the message never reaches MessageQueue2.

HAPI Bot

[github-actions]

[MAJOR] Preserve cancellation while waiting for readiness

This gate keeps incoming messages inside ApiSessionClient.pendingMessages until catalog readiness. During that interval the cancellation handler below only calls messageQueue.cancelByLocalId(), so it returns false; the hub interprets that as already invoked, but onUserMessage() later drains the pending message and executes it.

Suggested fix:

type DeferredMessage = Parameters<Parameters<typeof session.onUserMessage>[0]>
const deferredMessages: DeferredMessage[] = []
let messagePipelineReady = false

session.onUserMessage((message, localId) => {
    if (!messagePipelineReady) {
        deferredMessages.push([message, localId])
        return
    }
    handleUserMessage(message, localId)
})

void Promise.allSettled([sessionReady, getCatalog()]).then(() => {
    messagePipelineReady = true
    for (const [message, localId] of deferredMessages.splice(0)) {
        handleUserMessage(message, localId)
    }
})

session.onCancelQueuedMessage((localId) => {
    const index = deferredMessages.findIndex(([, id]) => id === localId)
    if (index >= 0) {
        deferredMessages.splice(index, 1)
        return true
    }
    return messageQueue.cancelByLocalId(localId)
})

Please cover cancel-before-catalog-ready and verify the deferred prompt is never queued.

[github-actions]

Findings

• [Major] Unbounded catalog readiness can block every incoming prompt — the message pipeline waits for getCatalog() at cli/src/claude/runClaude.ts:449, while the metadata iterator has no deadline and waits for either system/init or process exit at cli/src/claude/sdk/metadataExtractor.ts:101 and cli/src/claude/sdk/query.ts:144. If that Claude subprocess remains alive without emitting init, messagePipelineReady never becomes true, deferred messages keep accumulating, and no prompt reaches MessageQueue2.
  Suggested fix:

  const abortController = new AbortController()
  const timeout = setTimeout(() => abortController.abort(), 10_000)
  try {
      // existing query and init-message iteration
  } finally {
      clearTimeout(timeout)
  }

Summary

Review mode: follow-up after new commits

The previous cancellation race is no longer present. One startup liveness regression remains: an unbounded catalog probe can prevent the session from processing any user message.

Testing

Not run (automation/security constraint). The test and pr-review checks were pending when reviewed. Add a test with a metadata query that never emits init, then verify timeout abort releases a deferred prompt to MessageQueue2.

HAPI Bot

[github-actions]

[MAJOR] Bound catalog readiness so prompts cannot stall forever

Promise.allSettled does not settle until getCatalog() settles, but extractSDKMetadata() has no timeout: its iterator waits for an init message or child exit. A Claude subprocess that stays alive without emitting system/init therefore leaves every incoming message in deferredMessages indefinitely.

Suggested fix:

const abortController = new AbortController()
const timeout = setTimeout(() => abortController.abort(), 10_000)
try {
    // existing query and init-message iteration
} finally {
    clearTimeout(timeout)
}

Please cover a probe that never emits init and verify the deferred prompt is eventually queued.

[github-actions]

Findings

No findings.

Summary

Review mode: follow-up after new commits

No correctness, security, regression, data-loss, performance, or maintainability issue met the reporting threshold in the latest full diff. The prior startup-liveness finding is resolved by the 10-second metadata-probe abort. Residual risk: the timeout path has no regression test. GitHub checks were pending at review time.

Testing

Not run (automation/security constraint). Missing coverage: a metadata query that never emits system/init, proving timeout abort returns {} and releases deferred prompts.

HAPI Bot