diff --git a/.github/workflows/release-cut.yml b/.github/workflows/release-cut.yml index 1409ed1c371..605645186fb 100644 --- a/.github/workflows/release-cut.yml +++ b/.github/workflows/release-cut.yml @@ -472,10 +472,7 @@ jobs: # message and tag name explicitly (avoids npm's `v1.2.3` prefix # assumptions and any lifecycle scripts that would run on bump). npm version "$VERSION" --no-git-tag-version --allow-same-version - # Why: package version is embedded in generated skill provenance; full tag - # history preserves released snapshots while the current version is updated. - node config/scripts/generate-skill-bundle-manifest.mjs --write - git add package.json resources/skills + git add package.json commit_message="release: v$VERSION" if [[ "$EVENT_NAME" == "schedule" ]]; then commit_message="$commit_message [rc-slot:$SLOT]" diff --git a/config/scripts/generate-skill-bundle-manifest.mjs b/config/scripts/generate-skill-bundle-manifest.mjs index c3542d6f0e2..d4ba49c9b52 100644 --- a/config/scripts/generate-skill-bundle-manifest.mjs +++ b/config/scripts/generate-skill-bundle-manifest.mjs @@ -6,7 +6,11 @@ import path from 'node:path' import process from 'node:process' import { isDeepStrictEqual } from 'node:util' -const SCHEMA_VERSION = 1 +// Why: the three artifacts version independently — bumping one shape must not +// rewrite the others or bypass the registry's schema-gated append-only guard. +const CURRENT_MANIFEST_SCHEMA_VERSION = 2 +const SNAPSHOT_REGISTRY_SCHEMA_VERSION = 1 +const RELEASE_MAPPING_SCHEMA_VERSION = 1 const SCRIPT_DIR = import.meta.dirname const REPO_ROOT = path.resolve(SCRIPT_DIR, '..', '..') const SKILLS_ROOT = path.join(REPO_ROOT, 'skills') @@ -313,8 +317,8 @@ function skillsTreeShasAtRefs(refs) { } function buildReleasedHistory() { - const registry = { schemaVersion: SCHEMA_VERSION, skills: {} } - const mapping = { schemaVersion: SCHEMA_VERSION, releases: [] } + const registry = { schemaVersion: SNAPSHOT_REGISTRY_SCHEMA_VERSION, skills: {} } + const mapping = { schemaVersion: RELEASE_MAPPING_SCHEMA_VERSION, releases: [] } const tags = releaseTags() const treeShas = skillsTreeShasAtRefs(tags) const distinctTreeShas = [...new Set(treeShas.filter(Boolean))] @@ -366,7 +370,10 @@ function buildReleasedHistory() { return { registry, mapping } } -async function buildArtifacts(appVersion) { +// Why: the artifacts must be pure functions of skills/ bytes and release-tag +// history. Stamping the app version made every release cut invalidate the +// committed output on all open branches and drag skill CI onto unrelated PRs. +async function buildArtifacts() { const { registry, mapping } = buildReleasedHistory() const releasedSnapshotCounts = Object.fromEntries( Object.entries(registry.skills).map(([name, snapshots]) => [name, snapshots.length]) @@ -399,14 +406,12 @@ async function buildArtifacts(appVersion) { currentSkills.push({ name, sourcePath: `skills/${name}`, - appVersion, ...snapshot }) } return { currentManifest: { - schemaVersion: SCHEMA_VERSION, - appVersion, + schemaVersion: CURRENT_MANIFEST_SCHEMA_VERSION, skills: currentSkills }, snapshotRegistry: registry, @@ -419,7 +424,7 @@ async function buildArtifacts(appVersion) { // so a generation-logic change must not rewrite them silently. Only the one // unreleased working-tree append per skill may change between runs. function assertReleasedHistoryPreserved(committedRegistry, artifacts) { - if (!committedRegistry || committedRegistry.schemaVersion !== SCHEMA_VERSION) { + if (!committedRegistry || committedRegistry.schemaVersion !== SNAPSHOT_REGISTRY_SCHEMA_VERSION) { return } for (const [name, committedSnapshots] of Object.entries(committedRegistry.skills ?? {})) { @@ -466,17 +471,50 @@ async function writeArtifacts(artifacts) { ]) } +// Why: cutting a release tag adds a trailing mapping row on every checkout at +// once, before any branch can regenerate. A trailing row whose revisions all +// equal the current manifest is provably redundant until the next real +// regeneration (installs of those bytes classify as current and are labeled by +// the running build), so verify must not fail branches over it. +function isToleratedReleaseMappingPrefix(committedText, artifacts) { + let committed + try { + committed = JSON.parse(committedText) + } catch { + return false + } + const derived = artifacts.releaseMapping + const committedCount = Array.isArray(committed?.releases) ? committed.releases.length : -1 + if (committedCount < 0 || committedCount >= derived.releases.length) { + return false + } + const prefix = { + schemaVersion: derived.schemaVersion, + releases: derived.releases.slice(0, committedCount) + } + if (committedText !== serialized(prefix)) { + return false + } + const currentRevisions = Object.fromEntries( + artifacts.currentManifest.skills.map((skill) => [skill.name, skill.releaseRevision]) + ) + return derived.releases + .slice(committedCount) + .every((release) => isDeepStrictEqual(release.skills, currentRevisions)) +} + async function verifyArtifacts(artifacts) { const expected = [ - [CURRENT_MANIFEST_PATH, artifacts.currentManifest], - [SNAPSHOT_REGISTRY_PATH, artifacts.snapshotRegistry], - [RELEASE_MAPPING_PATH, artifacts.releaseMapping] + [CURRENT_MANIFEST_PATH, artifacts.currentManifest, null], + [SNAPSHOT_REGISTRY_PATH, artifacts.snapshotRegistry, null], + [RELEASE_MAPPING_PATH, artifacts.releaseMapping, isToleratedReleaseMappingPrefix] ] const stale = [] - for (const [filePath, value] of expected) { + for (const [filePath, value, tolerated] of expected) { try { await access(filePath, constants.R_OK) - if ((await readFile(filePath, 'utf8')) !== serialized(value)) { + const committedText = await readFile(filePath, 'utf8') + if (committedText !== serialized(value) && !tolerated?.(committedText, artifacts)) { stale.push(filePath) } } catch { @@ -493,8 +531,7 @@ async function verifyArtifacts(artifacts) { } async function main() { - const packageJson = JSON.parse(await readFile(path.join(REPO_ROOT, 'package.json'), 'utf8')) - const artifacts = await buildArtifacts(packageJson.version) + const artifacts = await buildArtifacts() assertReleasedHistoryPreserved(await readCommittedRegistry(), artifacts) await (process.argv.includes('--write') ? writeArtifacts : verifyArtifacts)(artifacts) } @@ -514,6 +551,7 @@ export { collectPackageFiles, describeFile, gitTreeSha, + isToleratedReleaseMappingPrefix, normalizeText, packageDigest, sortManifestFiles, diff --git a/config/scripts/generate-skill-bundle-manifest.test.mjs b/config/scripts/generate-skill-bundle-manifest.test.mjs index cf4a8a100e5..a6539f800a0 100644 --- a/config/scripts/generate-skill-bundle-manifest.test.mjs +++ b/config/scripts/generate-skill-bundle-manifest.test.mjs @@ -9,6 +9,7 @@ import { collectPackageFiles, describeFile, gitTreeSha, + isToleratedReleaseMappingPrefix, normalizeText, packageDigest, sortManifestFiles @@ -136,6 +137,56 @@ describe('skill bundle manifest generator', () => { expect(() => assertReleasedHistoryPreserved(null, artifacts)).not.toThrow() }) + it('tolerates only redundant trailing release-mapping rows', () => { + const serialized = (value) => `${JSON.stringify(value, null, 2)}\n` + const rows = [ + { appVersion: '1.0.0', skills: { 'orca-cli': 1 } }, + { appVersion: '1.1.0', skills: { 'orca-cli': 2 } } + ] + const artifacts = { + currentManifest: { skills: [{ name: 'orca-cli', releaseRevision: 2 }] }, + releaseMapping: { schemaVersion: 1, releases: rows } + } + const committedPrefix = serialized({ schemaVersion: 1, releases: [rows[0]] }) + + // A just-cut tag whose bytes equal the working tree may lag in the mapping. + expect(isToleratedReleaseMappingPrefix(committedPrefix, artifacts)).toBe(true) + // The committed file matching the derived mapping is byte-equality's job, not tolerance. + expect(isToleratedReleaseMappingPrefix(serialized(artifacts.releaseMapping), artifacts)).toBe( + false + ) + // A trailing row for bytes the committed artifacts do not describe is a real gap. + expect( + isToleratedReleaseMappingPrefix(committedPrefix, { + ...artifacts, + currentManifest: { skills: [{ name: 'orca-cli', releaseRevision: 3 }] } + }) + ).toBe(false) + expect( + isToleratedReleaseMappingPrefix(committedPrefix, { + ...artifacts, + currentManifest: { + skills: [ + { name: 'orca-cli', releaseRevision: 2 }, + { name: 'orca-linear', releaseRevision: 1 } + ] + } + }) + ).toBe(false) + // Rewritten earlier rows never pass, with or without trailing rows. + expect( + isToleratedReleaseMappingPrefix( + serialized({ + schemaVersion: 1, + releases: [{ appVersion: '0.9.0', skills: { 'orca-cli': 1 } }] + }), + artifacts + ) + ).toBe(false) + expect(isToleratedReleaseMappingPrefix('not json', artifacts)).toBe(false) + expect(isToleratedReleaseMappingPrefix(serialized({ schemaVersion: 1 }), artifacts)).toBe(false) + }) + it.runIf(process.platform !== 'win32')( 'rejects executable files in shipped skill packages', async () => { diff --git a/config/scripts/package-electron-runtime-contract.test.mjs b/config/scripts/package-electron-runtime-contract.test.mjs index e6f6018e6b0..c18272d56a4 100644 --- a/config/scripts/package-electron-runtime-contract.test.mjs +++ b/config/scripts/package-electron-runtime-contract.test.mjs @@ -374,7 +374,7 @@ describe('Electron runtime package contract', () => { expect(afterInstallScript).not.toContain('chmod 0755 "$sandbox"') }) - it('keeps release-cut version commits self-healing and taggable on retries', () => { + it('keeps release-cut version commits skill-independent and taggable on retries', () => { const releaseWorkflow = readFileSync( join(projectDir, '.github/workflows/release-cut.yml'), 'utf8' @@ -388,14 +388,13 @@ describe('Electron runtime package contract', () => { const bumpIndex = bumpStep.run.indexOf( 'npm version "$VERSION" --no-git-tag-version --allow-same-version' ) - const generateIndex = bumpStep.run.indexOf( - 'node config/scripts/generate-skill-bundle-manifest.mjs --write' - ) - const stageIndex = bumpStep.run.indexOf('git add package.json resources/skills') + const stageIndex = bumpStep.run.indexOf('git add package.json') expect(checkoutStep.with['fetch-depth']).toBe(0) expect(bumpIndex).toBeGreaterThanOrEqual(0) - expect(generateIndex).toBeGreaterThan(bumpIndex) - expect(stageIndex).toBeGreaterThan(generateIndex) + expect(stageIndex).toBeGreaterThan(bumpIndex) + // Why: version-only cuts must not mutate content-addressed skill artifacts. + expect(bumpStep.run).not.toContain('generate-skill-bundle-manifest') + expect(bumpStep.run).not.toContain('resources/skills') expect(bumpStep.run).toContain('git diff --cached --quiet') expect(bumpStep.run).toContain('git commit --allow-empty -m "$commit_message"') }) diff --git a/resources/skills/current-manifest.json b/resources/skills/current-manifest.json index cea063aa334..b0053b7b118 100644 --- a/resources/skills/current-manifest.json +++ b/resources/skills/current-manifest.json @@ -1,11 +1,9 @@ { - "schemaVersion": 1, - "appVersion": "1.4.144-rc.4", + "schemaVersion": 2, "skills": [ { "name": "computer-use", "sourcePath": "skills/computer-use", - "appVersion": "1.4.144-rc.4", "releaseRevision": 5, "packageDigest": "cd2809474d57fd7277adb277448e6fa446810d3cbad71ac0b473b9e8ff1bad68", "gitTreeSha": "306c0f8cb63bcac265a5b7975dc2f855be4f1344", @@ -24,7 +22,6 @@ { "name": "linear-tickets", "sourcePath": "skills/linear-tickets", - "appVersion": "1.4.144-rc.4", "releaseRevision": 4, "packageDigest": "f198d7b22e5ee1673dac403f9cca0553b124e0a90e4fdd05d2c23b7344e32d2b", "gitTreeSha": "de9fc106bbb4e313a90ff9a9513a720909bbd176", @@ -43,7 +40,6 @@ { "name": "orca-cli", "sourcePath": "skills/orca-cli", - "appVersion": "1.4.144-rc.4", "releaseRevision": 32, "packageDigest": "51740ff13f379ac5743d3fd28a14b17168dcef40f7048c20182dce166098c45f", "gitTreeSha": "ded93000a5f654e2b4f324501282459bd56afe19", @@ -62,7 +58,6 @@ { "name": "orca-emulator", "sourcePath": "skills/orca-emulator", - "appVersion": "1.4.144-rc.4", "releaseRevision": 4, "packageDigest": "453b1d9aa20b51b8a4d32c7b6def6a93f7ef9c730de32abbcbc1788ad1b1820b", "gitTreeSha": "66be6abe99f1807da85934aee0e22daefc8f7656", @@ -81,7 +76,6 @@ { "name": "orca-emulator-android", "sourcePath": "skills/orca-emulator-android", - "appVersion": "1.4.144-rc.4", "releaseRevision": 2, "packageDigest": "12272cf82e0731f11e424822b961882457034e730358cc65ea28e4eb9c8ff7f5", "gitTreeSha": "f7b0fc8cbf5cd78ca5156f6bbe3a20f1462d8f83", @@ -100,7 +94,6 @@ { "name": "orca-linear", "sourcePath": "skills/orca-linear", - "appVersion": "1.4.144-rc.4", "releaseRevision": 2, "packageDigest": "d44d09e6ecb6a64da177083aad26a95f031cd1cf26ba059fdc888c2628aef64f", "gitTreeSha": "c34f42030f43e5a85737996fa375bbd79cb5bea8", @@ -119,7 +112,6 @@ { "name": "orca-per-workspace-env", "sourcePath": "skills/orca-per-workspace-env", - "appVersion": "1.4.144-rc.4", "releaseRevision": 2, "packageDigest": "fa3b65a1a107fca3f0375c696852477b62f58c154b9eb5c0663c41edc4bcd30d", "gitTreeSha": "354e775b79ea6952ec63acac4d3ee8a9ae07a650", @@ -138,7 +130,6 @@ { "name": "orchestration", "sourcePath": "skills/orchestration", - "appVersion": "1.4.144-rc.4", "releaseRevision": 24, "packageDigest": "9fbfa2ae3f3f99441563a4b8b1c6302107944480db8718ddb326a862a51f7ab9", "gitTreeSha": "086c41e0b353b4908d2963694b4a7c791d4b3982", diff --git a/src/main/ipc/skills.test.ts b/src/main/ipc/skills.test.ts index bc390b2beb7..91c449f751f 100644 --- a/src/main/ipc/skills.test.ts +++ b/src/main/ipc/skills.test.ts @@ -15,6 +15,9 @@ const { })) vi.mock('electron', () => ({ + app: { + getVersion: () => '9.9.9-test' + }, ipcMain: { handle: handleMock } @@ -168,7 +171,10 @@ describe('registerSkillsHandlers', () => { await handler(null) - expect(inventorySkillFreshnessMock).toHaveBeenCalledWith({ repos }) + expect(inventorySkillFreshnessMock).toHaveBeenCalledWith({ + currentAppVersion: '9.9.9-test', + repos + }) expect(getWslHomeMock).not.toHaveBeenCalled() }) }) diff --git a/src/main/ipc/skills.ts b/src/main/ipc/skills.ts index d210123a744..711162927fe 100644 --- a/src/main/ipc/skills.ts +++ b/src/main/ipc/skills.ts @@ -1,4 +1,4 @@ -import { ipcMain } from 'electron' +import { app, ipcMain } from 'electron' import type { Store } from '../persistence' import { discoverSkills } from '../skills/discovery' import type { SkillDiscoveryResult, SkillDiscoveryTarget } from '../../shared/skills' @@ -61,6 +61,9 @@ export function registerSkillsHandlers(store: Store): void { ipcMain.handle('skills:freshnessInventory', async (): Promise => { // Why: the update command targets this machine's global homes. WSL and SSH // inventories stay out until their installer rail has an equivalent proof. - return inventorySkillFreshness({ repos: store.getRepos() }) + return inventorySkillFreshness({ + currentAppVersion: app.getVersion(), + repos: store.getRepos() + }) }) } diff --git a/src/main/skills/skill-bundle-artifacts.test.ts b/src/main/skills/skill-bundle-artifacts.test.ts index f86d5c382d7..cf58caebe09 100644 --- a/src/main/skills/skill-bundle-artifacts.test.ts +++ b/src/main/skills/skill-bundle-artifacts.test.ts @@ -11,6 +11,48 @@ afterEach(async () => { }) describe('skill bundle artifacts', () => { + it('loads the committed schema-2 artifacts and rejects a stamped schema-1 manifest', async () => { + const resourceRoot = await mkdtemp(join(tmpdir(), 'orca-skill-artifacts-')) + temporaryDirectories.push(resourceRoot) + const target = join(resourceRoot, 'skills') + const source = resolve('resources', 'skills') + await mkdir(target, { recursive: true }) + const [manifest, registry, releaseMapping] = await Promise.all( + ['current-manifest.json', 'snapshot-registry.json', 'release-mapping.json'].map((name) => + readFile(join(source, name), 'utf8') + ) + ) + await Promise.all([ + writeFile(join(target, 'current-manifest.json'), manifest), + writeFile(join(target, 'snapshot-registry.json'), registry), + writeFile(join(target, 'release-mapping.json'), releaseMapping) + ]) + + const artifacts = await loadSkillBundleArtifacts(resourceRoot) + expect(artifacts.manifest.schemaVersion).toBe(2) + expect(artifacts.manifest.skills.length).toBeGreaterThan(0) + + const legacyRoot = await mkdtemp(join(tmpdir(), 'orca-skill-artifacts-')) + temporaryDirectories.push(legacyRoot) + const legacyTarget = join(legacyRoot, 'skills') + await mkdir(legacyTarget, { recursive: true }) + const legacyManifest = JSON.parse(manifest) + legacyManifest.schemaVersion = 1 + legacyManifest.appVersion = '1.0.0' + for (const skill of legacyManifest.skills) { + skill.appVersion = '1.0.0' + } + await Promise.all([ + writeFile(join(legacyTarget, 'current-manifest.json'), JSON.stringify(legacyManifest)), + writeFile(join(legacyTarget, 'snapshot-registry.json'), registry), + writeFile(join(legacyTarget, 'release-mapping.json'), releaseMapping) + ]) + + await expect(loadSkillBundleArtifacts(legacyRoot)).rejects.toThrow( + 'Invalid skill bundle manifest' + ) + }) + it('rejects malformed nested release entries before building provenance', async () => { const resourceRoot = await mkdtemp(join(tmpdir(), 'orca-skill-artifacts-')) temporaryDirectories.push(resourceRoot) diff --git a/src/main/skills/skill-bundle-artifacts.ts b/src/main/skills/skill-bundle-artifacts.ts index e7d403442b5..08e33a0eeba 100644 --- a/src/main/skills/skill-bundle-artifacts.ts +++ b/src/main/skills/skill-bundle-artifacts.ts @@ -41,14 +41,12 @@ const snapshotShape = { const knownSnapshotSchema = z.object(snapshotShape).strict() const manifestSchema = z .object({ - schemaVersion: z.literal(1), - appVersion: z.string().min(1), + schemaVersion: z.literal(2), skills: z.array( z .object({ name: z.string().regex(/^[a-z0-9][a-z0-9._-]*$/), sourcePath: z.string().min(1), - appVersion: z.string().min(1), ...snapshotShape }) .strict() @@ -126,7 +124,6 @@ async function readSkillBundleArtifacts(resourceRoot: string): Promise snapshot.releaseRevision === current.releaseRevision && @@ -137,6 +134,8 @@ async function readSkillBundleArtifacts(resourceRoot: string): Promise> = {} for (const release of releaseMapping.releases) { for (const [name, revision] of Object.entries(release.skills)) { @@ -147,10 +146,6 @@ async function readSkillBundleArtifacts(resourceRoot: string): Promise { await test.writeSkill(join(test.homeDir, '.agents', 'skills'), test.oldMarkdown) const inventory = await inventorySkillFreshness({ + currentAppVersion: '2.0.0', homeDir: test.homeDir, repos: [], resourceRoot: test.resourceRoot @@ -130,6 +130,7 @@ describe('read-only skill freshness inventory', () => { ) const inventory = await inventorySkillFreshness({ + currentAppVersion: '2.0.0', homeDir: test.homeDir, repos: [], resourceRoot: test.resourceRoot @@ -151,6 +152,7 @@ describe('read-only skill freshness inventory', () => { ) const inventory = await inventorySkillFreshness({ + currentAppVersion: '2.0.0', homeDir: test.homeDir, repos: [], resourceRoot: test.resourceRoot @@ -176,6 +178,7 @@ describe('read-only skill freshness inventory', () => { await symlink(canonical, join(claudeRoot, 'orca-cli')) const inventory = await inventorySkillFreshness({ + currentAppVersion: '2.0.0', homeDir: test.homeDir, repos: [], resourceRoot: test.resourceRoot @@ -205,6 +208,7 @@ describe('read-only skill freshness inventory', () => { ) const inventory = await inventorySkillFreshness({ + currentAppVersion: '2.0.0', homeDir: test.homeDir, repos, resourceRoot: test.resourceRoot @@ -223,6 +227,7 @@ describe('read-only skill freshness inventory', () => { const inaccessiblePath = join(test.homeDir, '.codex', 'skills', 'orca-cli') const inventory = await inventorySkillFreshness({ + currentAppVersion: '2.0.0', homeDir: test.homeDir, repos: [], resourceRoot: test.resourceRoot, @@ -248,6 +253,7 @@ describe('read-only skill freshness inventory', () => { const inaccessiblePath = join(repoPath, '.agents', 'skills', 'orca-cli') const inventory = await inventorySkillFreshness({ + currentAppVersion: '2.0.0', homeDir: test.homeDir, repos: [{ id: 'repo', path: repoPath }] as unknown as Repo[], resourceRoot: test.resourceRoot, @@ -292,6 +298,7 @@ describe('read-only skill freshness inventory', () => { } const inventory = await inventorySkillFreshness({ + currentAppVersion: '2.0.0', homeDir: test.homeDir, repos, resourceRoot: test.resourceRoot @@ -310,6 +317,7 @@ describe('read-only skill freshness inventory', () => { ) const inventory = await inventorySkillFreshness({ + currentAppVersion: '2.0.0', homeDir: test.homeDir, repos: [], resourceRoot: test.resourceRoot @@ -326,7 +334,10 @@ describe('read-only skill freshness inventory', () => { await writeFile(registryPath, `${JSON.stringify(registry, null, 2)}\n`) await test.writeSkill(join(test.homeDir, '.agents', 'skills'), test.currentMarkdown) + // Why: the injected version deliberately differs from every mapping entry + // to prove current placements are labeled by the running build, not history. const inventory = await inventorySkillFreshness({ + currentAppVersion: '2.1.0-unreleased', homeDir: test.homeDir, repos: [], resourceRoot: test.resourceRoot @@ -335,7 +346,8 @@ describe('read-only skill freshness inventory', () => { expect(inventory.installations[0]).toMatchObject({ status: 'current', installedReleaseRevision: 2, - installedAppVersion: '2.0.0' + installedAppVersion: '2.1.0-unreleased', + currentAppVersion: '2.1.0-unreleased' }) }) @@ -348,6 +360,7 @@ describe('read-only skill freshness inventory', () => { ) const inventory = await inventorySkillFreshness({ + currentAppVersion: '2.0.0', homeDir: test.homeDir, repos, resourceRoot: test.resourceRoot diff --git a/src/main/skills/skill-freshness-inventory.ts b/src/main/skills/skill-freshness-inventory.ts index e5bbae31da9..8b1c5c19094 100644 --- a/src/main/skills/skill-freshness-inventory.ts +++ b/src/main/skills/skill-freshness-inventory.ts @@ -30,15 +30,16 @@ export function boundRepositorySkillRoots(roots: readonly SkillScanRoot[]): { } } -export async function inventorySkillFreshness( - args: { - homeDir?: string - cwd?: string - repos?: Repo[] - resourceRoot?: string - candidateLstat?: CandidateLstat - } = {} -): Promise { +export async function inventorySkillFreshness(args: { + // Why: the bundled artifacts are content-only; the running build supplies + // its own version here so current placements can be labeled honestly. + currentAppVersion: string + homeDir?: string + cwd?: string + repos?: Repo[] + resourceRoot?: string + candidateLstat?: CandidateLstat +}): Promise { const artifacts = await loadSkillBundleArtifacts(args.resourceRoot) const currentByName = new Map(artifacts.manifest.skills.map((skill) => [skill.name, skill])) const discoveryArgs = { @@ -65,6 +66,7 @@ export async function inventorySkillFreshness( classifyHomeSkillCandidate({ root, current, + currentAppVersion: args.currentAppVersion, artifacts, canonicalRootPath, candidateLstat: args.candidateLstat ?? ((path) => lstat(path)) @@ -84,6 +86,7 @@ export async function inventorySkillFreshness( classifyUnsupportedSkillCandidate({ root, current, + currentAppVersion: args.currentAppVersion, artifacts, unresolvedPath: join(root.path, current.name), candidateLstat @@ -99,6 +102,7 @@ export async function inventorySkillFreshness( (current) => () => observeSkillFreshnessInstallation({ current, + currentAppVersion: args.currentAppVersion, artifacts, rootId: 'repo-scan-limit', providers: [...new Set(omittedRepoRoots.flatMap((root) => root.providers))], @@ -128,6 +132,7 @@ export async function inventorySkillFreshness( classifyUnsupportedSkillCandidate({ root, current, + currentAppVersion: args.currentAppVersion, artifacts, unresolvedPath: candidate.path, candidateLstat @@ -142,6 +147,7 @@ export async function inventorySkillFreshness( (current) => () => observeSkillFreshnessInstallation({ current, + currentAppVersion: args.currentAppVersion, artifacts, rootId: root.id, providers: root.providers, diff --git a/src/main/skills/skill-freshness-placement-observation.ts b/src/main/skills/skill-freshness-placement-observation.ts index 2f8d32ca663..4218a4cc316 100644 --- a/src/main/skills/skill-freshness-placement-observation.ts +++ b/src/main/skills/skill-freshness-placement-observation.ts @@ -57,6 +57,7 @@ function knownSnapshots( export async function observeSkillFreshnessInstallation(args: { current: SkillCurrentBundleEntry + currentAppVersion: string artifacts: SkillBundleArtifacts rootId: string providers: SkillFreshnessInstallation['providers'] @@ -78,7 +79,7 @@ export async function observeSkillFreshnessInstallation(args: { topology: args.topology.topology, currentReleaseRevision: args.current.releaseRevision, currentPackageDigest: args.current.packageDigest, - currentAppVersion: args.current.appVersion, + currentAppVersion: args.currentAppVersion, errorCategory: args.topology.errorCategory } if (!args.topology.resolvedPath || !args.topology.identity) { @@ -105,9 +106,14 @@ export async function observeSkillFreshnessInstallation(args: { ...base, status: freshnessStatus(snapshot, args.current), installedReleaseRevision: snapshot?.releaseRevision ?? null, + // Why: the current revision may be unreleased (or trail the newest tag), + // so its label is the running build's version; only historical revisions + // resolve through the release mapping. installedAppVersion: snapshot - ? (args.artifacts.releasedAppVersions[args.current.name]?.[snapshot.releaseRevision] ?? - null) + ? snapshot.releaseRevision === args.current.releaseRevision + ? args.currentAppVersion + : (args.artifacts.releasedAppVersions[args.current.name]?.[snapshot.releaseRevision] ?? + null) : null, observedPackageDigest: observed.observedDigest } @@ -126,6 +132,7 @@ export async function observeSkillFreshnessInstallation(args: { export async function classifyHomeSkillCandidate(args: { root: SkillScanRoot current: SkillCurrentBundleEntry + currentAppVersion: string artifacts: SkillBundleArtifacts canonicalRootPath: string candidateLstat: CandidateLstat @@ -139,6 +146,7 @@ export async function classifyHomeSkillCandidate(args: { } return observeSkillFreshnessInstallation({ current: args.current, + currentAppVersion: args.currentAppVersion, artifacts: args.artifacts, rootId: args.root.id, providers: args.root.providers, @@ -167,6 +175,7 @@ export async function classifyHomeSkillCandidate(args: { } return observeSkillFreshnessInstallation({ current: args.current, + currentAppVersion: args.currentAppVersion, artifacts: args.artifacts, rootId: args.root.id, providers: args.root.providers, @@ -180,6 +189,7 @@ export async function classifyHomeSkillCandidate(args: { export async function classifyUnsupportedSkillCandidate(args: { root: SkillScanRoot current: SkillCurrentBundleEntry + currentAppVersion: string artifacts: SkillBundleArtifacts unresolvedPath: string candidateLstat: CandidateLstat @@ -192,6 +202,7 @@ export async function classifyUnsupportedSkillCandidate(args: { } return observeSkillFreshnessInstallation({ current: args.current, + currentAppVersion: args.currentAppVersion, artifacts: args.artifacts, rootId: args.root.id, providers: args.root.providers, @@ -208,6 +219,7 @@ export async function classifyUnsupportedSkillCandidate(args: { } return observeSkillFreshnessInstallation({ current: args.current, + currentAppVersion: args.currentAppVersion, artifacts: args.artifacts, rootId: args.root.id, providers: args.root.providers, diff --git a/src/renderer/src/lib/use-tab-agent.test.ts b/src/renderer/src/lib/use-tab-agent.test.ts index 9152159f14b..d2283db7dea 100644 --- a/src/renderer/src/lib/use-tab-agent.test.ts +++ b/src/renderer/src/lib/use-tab-agent.test.ts @@ -258,6 +258,20 @@ describe('resolveTabAgentFromSignals', () => { ).toBe('claude') }) + // Why: #8478 — OpenCode native `OC | …` titles must reclaim a stale Claude + // launch identity so the tab icon is OpenCode, not Claude. + it('uses OpenCode native session titles to replace stale Claude launch identity', () => { + expect( + resolveTabAgentFromSignals({ + hasObservedAgentSignal: true, + isRemote: false, + title: 'OC | Understand about the plugin', + hookAgent: null, + launchAgent: 'claude' + }) + ).toBe('opencode') + }) + it('does not let an explicit title override launch identity before any activity is observed', () => { expect( resolveTabAgentFromSignals({ diff --git a/src/renderer/src/runtime/sync-runtime-graph-terminal-layout.test.ts b/src/renderer/src/runtime/sync-runtime-graph-terminal-layout.test.ts index 6095f4e1840..385a7764fd1 100644 --- a/src/renderer/src/runtime/sync-runtime-graph-terminal-layout.test.ts +++ b/src/renderer/src/runtime/sync-runtime-graph-terminal-layout.test.ts @@ -16,6 +16,7 @@ function makeState(overrides: Partial = {}): AppState { activeBrowserTabIdByWorktree: {}, browserTabsByWorktree: {}, browserPagesByWorkspace: {}, + browserCertificateFailuresByPageId: {}, openFiles: [], editorDrafts: {}, activeTabId: null, diff --git a/src/renderer/src/runtime/sync-runtime-graph.test.ts b/src/renderer/src/runtime/sync-runtime-graph.test.ts index 2de176df7b9..d1082d89cab 100644 --- a/src/renderer/src/runtime/sync-runtime-graph.test.ts +++ b/src/renderer/src/runtime/sync-runtime-graph.test.ts @@ -26,6 +26,7 @@ function makeState(overrides: Partial = {}): AppState { activeBrowserTabIdByWorktree: {}, browserTabsByWorktree: {}, browserPagesByWorkspace: {}, + browserCertificateFailuresByPageId: {}, openFiles: [], editorDrafts: {}, activeTabId: null, @@ -60,6 +61,7 @@ function makeSharedOverrides(): Partial { activeBrowserTabIdByWorktree: {}, browserTabsByWorktree: {}, browserPagesByWorkspace: {}, + browserCertificateFailuresByPageId: {}, openFiles: [], editorDrafts: {}, agentStatusByPaneKey: {}, diff --git a/src/renderer/src/runtime/sync-runtime-graph.ts b/src/renderer/src/runtime/sync-runtime-graph.ts index e735e130004..fb32a847941 100644 --- a/src/renderer/src/runtime/sync-runtime-graph.ts +++ b/src/renderer/src/runtime/sync-runtime-graph.ts @@ -1496,7 +1496,7 @@ function buildMobileBrowserTab( // back through ?? would resurrect a stale workspace-level error. loadError: activePage ? activePage.loadError : workspace.loadError, certificateFailure: activePage - ? (state.browserCertificateFailuresByPageId[activePage.id] ?? null) + ? (state.browserCertificateFailuresByPageId?.[activePage.id] ?? null) : null, color: unifiedTab?.color ?? null, isPinned: unifiedTab?.isPinned === true, diff --git a/src/shared/agent-detection.ts b/src/shared/agent-detection.ts index 891f37093db..4fbfff86f24 100644 --- a/src/shared/agent-detection.ts +++ b/src/shared/agent-detection.ts @@ -18,6 +18,7 @@ export { STRONG_IDLE_KEYWORDS_RE, STRONG_WORKING_KEYWORDS_RE } from './agent-title-core' +export { isOpenCodeNativeTitle, isMeaningfulOpenCodeTerminalTitle } from './opencode-terminal-title' export { getAgentLabel, isClaudeAgent } from './agent-title-identity' export { clearWorkingIndicators, diff --git a/src/shared/agent-title-identity.ts b/src/shared/agent-title-identity.ts index 9f7746b01bc..af86de1667d 100644 --- a/src/shared/agent-title-identity.ts +++ b/src/shared/agent-title-identity.ts @@ -10,6 +10,7 @@ import { isPiAgentTitle, titleHasAgentName } from './agent-title-core' +import { isOpenCodeNativeTitle } from './opencode-terminal-title' import { getPiCompatibleSyntheticAgentLabel } from './pi-compatible-synthetic-title' /** @@ -17,7 +18,7 @@ import { getPiCompatibleSyntheticAgentLabel } from './pi-compatible-synthetic-ti * Used to scope prompt-cache-timer behavior to Claude sessions only. */ export function isClaudeAgent(title: string): boolean { - if (!title || isClaudeManagementTitle(title)) { + if (!title || isClaudeManagementTitle(title) || isOpenCodeNativeTitle(title)) { return false } const lower = title.toLowerCase() @@ -46,6 +47,11 @@ export function getAgentLabel(title: string): string | null { if (isClaudeManagementTitle(title)) { return null } + // Why: the native marker owns the whole title; its session text may name or + // include status glyphs from other agents without changing OpenCode identity. + if (isOpenCodeNativeTitle(title)) { + return 'OpenCode' + } // Why: Claude task titles can mention another CLI; the prefix is the identity // signal, not arbitrary task text. if ( diff --git a/src/shared/opencode-terminal-title.test.ts b/src/shared/opencode-terminal-title.test.ts index fc1b7806e35..f1cb825e8cd 100644 --- a/src/shared/opencode-terminal-title.test.ts +++ b/src/shared/opencode-terminal-title.test.ts @@ -1,16 +1,23 @@ import { describe, expect, it } from 'vitest' -import { isMeaningfulOpenCodeTerminalTitle } from './opencode-terminal-title' +import { isMeaningfulOpenCodeTerminalTitle, isOpenCodeNativeTitle } from './opencode-terminal-title' describe('OpenCode terminal titles', () => { it('recognizes native session titles', () => { expect(isMeaningfulOpenCodeTerminalTitle('OC | Native Stable Session')).toBe(true) expect(isMeaningfulOpenCodeTerminalTitle(' OC|Session ')).toBe(true) + expect(isOpenCodeNativeTitle('OC | Understand about the plugin')).toBe(true) + expect(isOpenCodeNativeTitle('tmux | OC | ses_123')).toBe(true) }) - it('rejects generic or incomplete OpenCode titles', () => { + it('rejects generic, incomplete, embedded, and lookalike titles', () => { expect(isMeaningfulOpenCodeTerminalTitle('OpenCode')).toBe(false) expect(isMeaningfulOpenCodeTerminalTitle('OpenCode ready')).toBe(false) expect(isMeaningfulOpenCodeTerminalTitle('OC |')).toBe(false) expect(isMeaningfulOpenCodeTerminalTitle(undefined)).toBe(false) + // Why: lowercase is not OpenCode's native marker; avoid "oc |" cwd/task noise. + expect(isOpenCodeNativeTitle('oc | Understand about the plugin')).toBe(false) + // Why: mid-title OC must not steal another agent's braille/task frame. + expect(isOpenCodeNativeTitle('⠋ Fix foo | OC | bar')).toBe(false) + expect(isOpenCodeNativeTitle('my session | OC | task')).toBe(false) }) }) diff --git a/src/shared/opencode-terminal-title.ts b/src/shared/opencode-terminal-title.ts index 64312b94be2..01d3d3819ae 100644 --- a/src/shared/opencode-terminal-title.ts +++ b/src/shared/opencode-terminal-title.ts @@ -1,4 +1,14 @@ +// Why: OpenCode abbreviates native OSC session titles as `OC | ` (no +// agent-name token). Optional single-token multiplexer prefix covers SSH/tmux +// frames like `tmux | OC | …`. Case-sensitive `OC` avoids ordinary lowercase +// "oc" lookalikes; require non-whitespace after the marker so bare `OC |` is not +// identity. Used for both display-title preservation and tab-agent identity. +const OPENCODE_NATIVE_TITLE_RE = /^(?:[^|\s]+ \| )?OC\s*\|\s*\S/u + +export function isOpenCodeNativeTitle(title: string | null | undefined): boolean { + return OPENCODE_NATIVE_TITLE_RE.test(title?.trim() ?? '') +} + export function isMeaningfulOpenCodeTerminalTitle(title: string | null | undefined): boolean { - // Why: bare OpenCode labels are status/identity; `OC | …` carries native session identity. - return /^OC\s*\|\s*\S/u.test(title?.trim() ?? '') + return isOpenCodeNativeTitle(title) } diff --git a/src/shared/repro-8478-opencode-native-title-icon.test.ts b/src/shared/repro-8478-opencode-native-title-icon.test.ts new file mode 100644 index 00000000000..554ac0c5ddb --- /dev/null +++ b/src/shared/repro-8478-opencode-native-title-icon.test.ts @@ -0,0 +1,49 @@ +/** + * Issue #8478 — OpenCode logo / icon not coming up well (Claude glyph on + * OpenCode tabs). + * + * Root cause (pre-fix): OpenCode's native OSC tab title format is `OC | `. + * Title classifiers only recognized OpenCode when the token "opencode" appeared. + * Native `OC | …` titles fell through, so AgentIcon rendered Claude/"?". + * + * OpenCode source (packages/tui/src/app.tsx) sets titles as: + * - "OpenCode" on home / default session titles + * - `OC | ${title}` for named sessions (title truncated at 40 chars) + * - `OC | ${pluginId}` on plugin routes + * + * Related: #8940 (OpenCode activity frames mislabeled Claude Code — separate + * braille-without-token path; not fully solved here). + * + * Re-run: + * pnpm exec vitest run --config config/vitest.config.ts \ + * src/shared/repro-8478-opencode-native-title-icon.test.ts + */ +import { describe, expect, it } from 'vitest' +import { getAgentLabel, isClaudeAgent } from './agent-detection' +import { + resolveExplicitTerminalTitleAgentType, + resolveTerminalTitleAgentType +} from './terminal-title-agent-type' + +describe('#8478 OpenCode native OC | titles map to OpenCode icon', () => { + it('recognizes OpenCode native "OC | …" title format as opencode identity', () => { + const native = 'OC | Understand about the plugin' + expect(getAgentLabel(native)).toBe('OpenCode') + expect(resolveTerminalTitleAgentType(native)).toBe('opencode') + expect(resolveExplicitTerminalTitleAgentType(native)).toBe('opencode') + expect(isClaudeAgent(native)).toBe(false) + }) + + // Why: bare "OpenCode" is what the TUI sets on home/default sessions; keep it + // classified so the icon path stays OpenCode without the OC | abbreviation. + it('keeps bare OpenCode home titles and Claude-style frames classified correctly', () => { + expect(getAgentLabel('OpenCode')).toBe('OpenCode') + expect(resolveTerminalTitleAgentType('OpenCode')).toBe('opencode') + expect(getAgentLabel('OpenCode ready')).toBe('OpenCode') + expect(resolveTerminalTitleAgentType('OpenCode ready')).toBe('opencode') + // Same family as #8940: braille/task frames without "opencode" still become Claude + // when they lack the native OC marker — documented, out of scope for this fix. + expect(isClaudeAgent('⠋ implementing the feature')).toBe(true) + expect(getAgentLabel('⠋ implementing the feature')).toBe('Claude Code') + }) +}) diff --git a/src/shared/skill-freshness.ts b/src/shared/skill-freshness.ts index 08383d5e5f5..0a18eca1a81 100644 --- a/src/shared/skill-freshness.ts +++ b/src/shared/skill-freshness.ts @@ -20,12 +20,12 @@ export type SkillKnownSnapshot = { export type SkillCurrentBundleEntry = SkillKnownSnapshot & { name: string sourcePath: string - appVersion: string } +// Why: schema 2 removed the stamped app version so the committed artifact is a +// pure function of skills/ content; the running build supplies its own version. export type SkillBundleManifest = { - schemaVersion: 1 - appVersion: string + schemaVersion: 2 skills: SkillCurrentBundleEntry[] } diff --git a/src/shared/terminal-title-agent-type.test.ts b/src/shared/terminal-title-agent-type.test.ts index d28dd43f503..d0fda7dcc45 100644 --- a/src/shared/terminal-title-agent-type.test.ts +++ b/src/shared/terminal-title-agent-type.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it } from 'vitest' +import { getAgentLabel as getSharedAgentLabel } from './agent-title-identity' +import { isOpenCodeNativeTitle } from './opencode-terminal-title' import { isClaudeAgent, isGrokRotatingWorkingTitle, @@ -51,6 +53,39 @@ describe('resolveExplicitTerminalTitleAgentType', () => { expect(resolveExplicitTerminalTitleAgentType('* Review Codex behavior')).toBeNull() }) + it('resolves OpenCode native abbreviated session titles before task-text identities', () => { + expect(resolveExplicitTerminalTitleAgentType('OC | Understand about the plugin')).toBe( + 'opencode' + ) + expect(resolveExplicitTerminalTitleAgentType('OC | Compare Codex and Claude')).toBe('opencode') + // Why: Gemini glyphs inside OpenCode session text must not rebrand the tab. + expect(resolveExplicitTerminalTitleAgentType('OC | ✦ Gemini CLI')).toBe('opencode') + expect(getSharedAgentLabel('OC | Compare Codex and Claude')).toBe('OpenCode') + expect(getSharedAgentLabel('OC | ✦ Gemini CLI')).toBe('OpenCode') + expect(resolveExplicitTerminalTitleAgentType('tmux | OC | ses_123')).toBe('opencode') + expect(resolveExplicitTerminalTitleAgentType('OC|compact-session')).toBe('opencode') + expect(resolveExplicitTerminalTitleAgentType('oc | Understand about the plugin')).toBeNull() + }) + + it('does not find an OpenCode marker inside another agent task title', () => { + expect(isOpenCodeNativeTitle('⠋ Fix foo | OC | bar')).toBe(false) + expect(resolveExplicitTerminalTitleAgentType('⠋ Fix foo | OC | bar')).toBeNull() + }) + + // Why: adversarial coverage — native OC must not steal Claude/Codex/Cursor/ + // Gemini/Pi identity, and those agents must keep resolving when titled normally. + it('keeps other agents classified correctly alongside OpenCode native titles', () => { + expect(resolveExplicitTerminalTitleAgentType('✳ Claude Code')).toBe('claude') + expect(resolveExplicitTerminalTitleAgentType('⠋ Codex')).toBe('codex') + expect(resolveExplicitTerminalTitleAgentType('✦ Gemini CLI')).toBe('gemini') + expect(resolveExplicitTerminalTitleAgentType('Cursor Agent')).toBe('cursor') + expect(resolveExplicitTerminalTitleAgentType('Pi ready')).toBe('pi') + expect(resolveExplicitTerminalTitleAgentType('OpenCode ready')).toBe('opencode') + expect(resolveTerminalTitleAgentType('OC | ⠋ implementing the feature')).toBe('opencode') + expect(isClaudeAgent('OC | ⠋ implementing the feature')).toBe(false) + expect(isClaudeAgent('OC | Understand about the plugin')).toBe(false) + }) + it('still resolves Claude when the title explicitly names Claude', () => { expect(resolveExplicitTerminalTitleAgentType('. Claude Code compare Opencode')).toBe('claude') }) diff --git a/src/shared/terminal-title-agent-type.ts b/src/shared/terminal-title-agent-type.ts index ae7c97c09e7..fa92bb198c0 100644 --- a/src/shared/terminal-title-agent-type.ts +++ b/src/shared/terminal-title-agent-type.ts @@ -5,6 +5,7 @@ import { titleHasAgentName } from './agent-name-token-match' import { isCursorAgentTitle } from './agent-title-core' +import { isOpenCodeNativeTitle } from './opencode-terminal-title' import { getPiCompatibleSyntheticAgentLabel, isLegacyPiCompatibleTitle @@ -80,7 +81,7 @@ export function isPiAgentTitle(title: string): boolean { * agents have different (or no) caching semantics. */ export function isClaudeAgent(title: string): boolean { - if (!title || isClaudeManagementTitle(title)) { + if (!title || isClaudeManagementTitle(title) || isOpenCodeNativeTitle(title)) { return false } const lower = title.toLowerCase() @@ -124,6 +125,11 @@ export function getAgentLabel(title: string): string | null { if (isClaudeManagementTitle(title)) { return null } + // Why: the native marker owns the whole title; its session text may name or + // include status glyphs from other agents without changing OpenCode identity. + if (isOpenCodeNativeTitle(title)) { + return 'OpenCode' + } // Why: Claude Code title text is often the task title. If that task mentions // another CLI, the Claude-specific prefix is the identity signal, not the words. if (