fix(simulator): Reap orphaned OSLog helpers by workspace

[cameroncooke]

Fix orphaned simulator OSLog helpers after abnormal XcodeBuildMCP termination.

Detached simctl spawn ... log stream ... helpers are required so launch tools can return immediately, but they could survive if the owning MCP process died before async shutdown ran. This adds workspace-aware ownership to the durable OSLog registry and reconciles only helpers from the same workspace whose owner PID is no longer alive. Live sibling sessions and other workspaces are left alone.

This also removes the dead legacy log_capture.ts path and its lifecycle/status plumbing so simulator log cleanup is handled by the durable OSLog helper path. Console-PTY helpers remain intentionally unregistered because they are tied to the launched app lifecycle.

Validation performed locally included the full unit suite plus real simulator smoke testing with the example iOS app: creating a detached OSLog helper, starting the rebuilt MCP server from the same workspace to reap it, confirming another workspace does not reap it, and verifying no matching simctl helpers remain after app stop.

Fixes #382

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/xcodebuildmcp@386

commit: 0414d6d

[sentry]

Bug: The new isRecord validator deletes old registry records lacking a workspaceKey, preventing the orphan-reaping feature from cleaning up their associated processes.
_{Severity: MEDIUM}

Suggested Fix

Before deleting an invalid record due to a missing workspaceKey, attempt to terminate the associated process. Alternatively, the validation logic could be adjusted to handle these legacy records gracefully, perhaps by treating them as valid for the purpose of cleanup and then deleting them.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/utils/log-capture/simulator-launch-oslog-registry.ts#L56-L64

Potential issue: The new `isRecord` validator requires `record.owner.workspaceKey` to be
a non-empty string. Registry files created by previous versions lack this field. When
`listSimulatorLaunchOsLogRegistryRecords()` encounters these old records, it considers
them invalid and deletes them. However, it does so without terminating the corresponding
orphaned helper processes. This causes the new orphan-reaping feature to silently fail
for any orphans created before the upgrade, as their registry records are removed before
they can be processed for cleanup.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[cameroncooke]

@cursor investigate and report back

[cursor]

Now I have the full picture. Let me create a comprehensive investigation report:

Investigation Report: Legacy Registry Records Issue

Summary

The bug is VALID but LOW SEVERITY. Legacy registry records without workspaceKey will be deleted on first listSimulatorLaunchOsLogRegistryRecords() call, preventing orphan cleanup of their associated processes. However, this only affects the narrow upgrade window from v2.5.0-beta.1 to this branch.

Evidence

1. The registry file was introduced in PR feat(log-capture): Durable OSLog session tracking and executor settlement fixes #336 (commit dbb2d3b9), which shipped in v2.5.0-beta.1
2. In that version, RuntimeInstance had NO workspaceKey field:

export interface RuntimeInstance {
  instanceId: string;
  pid: number;
}

3. The current branch adds the workspaceKey requirement (lines 60-61):

    typeof record.owner === 'object' &&
    record.owner !== null &&
    typeof record.owner.instanceId === 'string' &&
    record.owner.instanceId.length > 0 &&
    typeof record.owner.workspaceKey === 'string' &&
    record.owner.workspaceKey.length > 0 &&
    typeof record.owner.pid === 'number' &&
    Number.isInteger(record.owner.pid) &&
    record.owner.pid > 0

4. The validation explicitly prunes these records (lines 214-216):

export async function listSimulatorLaunchOsLogRegistryRecords(): Promise<
  SimulatorLaunchOsLogRegistryRecord[]
> {
  try {
    await ensureRegistryDir();
  } catch {
    return [];
  }

  const entries: Array<{ filePath: string; record: SimulatorLaunchOsLogRegistryRecord }> = [];
  const invalidPaths: string[] = [];

  try {
    const dirEntries = await fs.readdir(getRegistryDir(), { withFileTypes: true });
    for (const dirEntry of dirEntries) {
      if (!dirEntry.isFile() || !dirEntry.name.endsWith('.json')) {
        continue;
      }

      const filePath = path.join(getRegistryDir(), dirEntry.name);
      try {
        const content = await fs.readFile(filePath, 'utf8');
        const parsed = JSON.parse(content) as unknown;
        if (!isRecord(parsed)) {
          invalidPaths.push(filePath);
          continue;
        }
        entries.push({ filePath, record: parsed });
      } catch {
        invalidPaths.push(filePath);
      }
    }
  } catch {
    return [];
  }

  if (invalidPaths.length > 0) {
    await removeRegistryPaths(invalidPaths);

5. There's even a test confirming this behavior (line 64):

  it('prunes records without owner workspace keys', async () => {
    writeFileSync(
      path.join(registryDir, 'missing-workspace.json'),
      `${JSON.stringify({
        ...createRecord(),
        owner: { instanceId: 'instance-1', pid: 1234 },
      })}\n`,
    );

    await expect(listSimulatorLaunchOsLogRegistryRecords()).resolves.toEqual([]);
  });

Why Low Severity

• Narrow affected window: Only users who:

  1. Upgraded to v2.5.0-beta.1 (released recently)
  2. Created OSLog helper records in that version
  3. Had an abnormal termination leaving orphans
  4. Then upgrade to this branch before manually cleaning up

• Self-healing: Any orphans from v2.5.0-beta.1 whose processes are already dead will be pruned naturally. The issue only affects live orphans where the simctl spawn process is still running.

• Workaround exists: Users can manually kill orphaned simctl spawn processes with ps -ef | grep "simctl spawn" | grep -v grep before upgrading.

Recommended Fix

Attempt to kill the process before deleting the registry file. Change the validation logic to gracefully handle legacy records by treating them as orphans eligible for immediate cleanup:

Option A: Best-effort kill before delete (safer, leaves no orphans)

// In listSimulatorLaunchOsLogRegistryRecords, when we find invalid records:
const legacyOrphans: Array<{ filePath: string; pid: number }> = [];

for (const dirEntry of dirEntries) {
  // ...existing parse logic...
  if (!isRecord(parsed)) {
    // Check if it's a legacy record with a PID we can clean up
    if (
      typeof parsed === 'object' &&
      parsed !== null &&
      typeof (parsed as any).helperPid === 'number' &&
      Number.isInteger((parsed as any).helperPid) &&
      (parsed as any).helperPid > 0
    ) {
      legacyOrphans.push({ filePath, pid: (parsed as any).helperPid });
    } else {
      invalidPaths.push(filePath);
    }
    continue;
  }
  // ...
}

// Before removing invalid paths, try to kill legacy orphans
for (const { pid } of legacyOrphans) {
  try {
    process.kill(pid, 'SIGTERM');
  } catch {
    // Process already dead or permission denied - that's fine
  }
}

// Now remove all invalid + legacy paths
await removeRegistryPaths([...invalidPaths, ...legacyOrphans.map(o => o.filePath)]);

Option B: Make workspaceKey optional in validation, treat missing as eligible for cleanup

This is more invasive and may complicate the reconciliation logic, so Option A is cleaner.

Recommendation

Implement Option A if the user base on v2.5.0-beta.1 is non-trivial and might have created orphans. Otherwise, document the limitation in the changelog/PR description and accept that the narrow beta window means very few users will hit this.

  

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit e1d8eec. Configure here.}