mirror of
https://github.com/stablyai/orca.git
synced 2026-11-03 10:37:35 +00:00
* fix(skills): judge official skill updates by ownership, not snapshot matching Orca decided whether an installed official skill was pristine by matching its bytes against a shipped list of past snapshots, falling back to the skills CLI's lock hash. Copies installed between releases, CRLF copies, and installs whose lock recorded a sha256 hash all read "doesn't match the official version" and could not be updated (#26841), and older content could be labelled newer and never offered. Now an official skill is Orca's when the skills.sh global lock says it came from stablyai/orca, and it is current only when its listed files equal the bundled manifest (line endings and sidecars ignored); anything else is outdated. Install and update are one pinned `skills add .../tree/v<appVersion>` command, used by the in-app runner, `orca skills update`, and the Settings rails. The update dialog preselects Orca-owned rows and lists same-named skills from elsewhere unchecked. Removes the snapshot registry, release mapping, git-tree identity, lock trust and convergence logic, and the release-cut provenance step. * ci(skills): run the pinned-add round trip on a skills CLI that can fetch a commit * fix(skills): let orca skills update target the shared folder on a bare host Update became the same pinned add as install, so it inherited install's refusal when no agent is detected. That broke the bare-host update the old command guaranteed, and the built-CLI check (update --dry-run, no --agent) on CI runners. The in-app runner already fell back to universal; both now share one detectSkillsCliUpdateAgentKeys rule. Also brings the CLI skills docs off `npx skills update`. * refactor(skills): derive the update selection from the grouped rows The selection hook took the live and the retained inventory, re-derived Orca ownership from installations beside the grouping's own orcaOwned, and kept a separate display count. Groups already carry both eligibility and ownership, so the hook reads them once; the button's existing live-scan gate is the only authorization check. The unselected line is inlined into the headline. * refactor(skills): drop helpers the ownership design made redundant - isOwnerManagedSkillScope only negated into the participation check once plugin caches stopped participating; fold it in. - Eligibility re-checked resolvedPath/physicalIdentity, which observation already turns into 'inaccessible'. - The manifest verifier probed access() before a readFile that throws anyway. - orchestration-install-command.ts only forwarded one constant from the lib module its three importers already import. * i18n(skills): stop naming npx skills update in four skipped-copy strings The es/ja/ko/zh cantUpdateReason and tipDuplicate strings still named `npx skills update`, which Orca no longer runs for its skills. Say 'the update command' instead. * refactor(skills): inline the single-use pinned source URL helper orcaSkillsSourceUrl was exported but only called by the builder beside it. * fix(skills): let an update choice lapse when the install source changes The selection froze every checked name, Orca's preselection included, the moment the user ticked anything. A re-check that then attributed a name to another source kept it checked, replacing it without consent. Store only the user's explicit choices, keyed by name and lock source, and default to ownership otherwise. * fix(skills): label the empty update action 'Update skills', not 'Update 0 skills' The button stays whenever a row is updatable, so unticking every row (or a dialog with only non-Orca copies outdated) doesn't reflow the footer. At zero selected it stays disabled and reads 'Update skills' instead of a zero count. The label lives beside the dialog's other count copy. * fix(skills): let keyboard focus open the Not from Orca tooltip The source of a non-Orca copy is only in the tooltip, and its trigger was a plain span. Make it focusable like Orca's other tooltip-on-text triggers (tabIndex 0, cursor-help, the row primitive's focus ring), as in the agreed mock. * test(skills): hold the round-trip add equal to the shared install builder The CI round trip hardcoded its skills add argv, so it could drift from what Orca runs. A .mjs script can't import the TS builder (extensionless imports), so the argv moves into an exported roundTripAddArgs and a unit test asserts it is the builder's unattended add with the tag swapped for the PR commit. The script runs under the same main guard as the manifest generator. * i18n(skills): translate the skill row's location count and duplicate reason es, ko and zh still carried English for SkillUpdateRow.oneLocation and manyLocations, and es and ko for SkillFreshnessRow.skippedReasonDuplicate. * fix(skills): pin official skill installs to the build commit and skills CLI `skills add .../tree/v<version>` only works for stable and RC builds: hourly, daily and adhoc builds publish elsewhere, so stablyai/orca has no tag for their version and both install and update failed there. Every build is made from a commit that exists in stablyai/orca, so the build now stamps `git rev-parse HEAD` and the one shared builder pins to `tree/<commit>`, which equals the bundled skills by construction. Main, renderer, web and vitest get it as ORCA_BUILD_COMMIT (replacing ORCA_APP_VERSION); the tsc-built CLI reads it from the compiled out/package.json boundary. A missing or malformed commit fails the build or the command with a clear error instead of building a bad URL. The builder also pins the skills CLI (`npx skills@1.7.2`): 1.5.x cannot fetch a commit, and a bare `npx skills` can reuse a stale cache or resolve to another binary (#18687). The CI round-trip script reuses the same constant. * ci(skills): run the round trip on the shared pinned skills CLI version * docs(skills): document the commit-pinned skills add * refactor(skills): keep the pinned skills CLI version in the install builder skills-cli-version.ts existed only so the plain-Node round-trip script could strip-load a .ts file, which needed --disable-warning in CI. The version now lives beside the builder that uses it; the script keeps a literal, and the existing round-trip test already asserts its argv, skills@<version> included, equals the builder's. * refactor(skills): validate the build commit once, where it is resolved resolveBuildSourceCommit already refuses anything but a full hash, and both the define and the CLI's stamped package.json come from it. The builder's second check could only fire as a module-load throw in the renderer's command constants. Also drops the resolver's unused cwd parameter. * test: stamp a fixed build commit instead of running git in the vitest config Tests only interpolate ORCA_BUILD_COMMIT, so they need not read HEAD (or fail outside a git checkout) on every run. * refactor(settings): match only skills add in the Windows npx preflight Every official skill command is now the builder's skills@<version> add; no rail emits skills update any more. * fix(skills): fail an update that leaves a broken skill link behind Eligibility already skips names with a broken link before the run, so one afterwards is the run's own damage; report it instead of claiming success. * ci(release): refuse to tag when the skill manifest is stale The removed --release step implicitly checked the committed manifest against skills/. Keep that guarantee with a verify-only run; it writes nothing. --------- Co-authored-by: Orca Worker <orca-worker@localhost>
273 lines
11 KiB
JavaScript
273 lines
11 KiB
JavaScript
import { execFileSync } from 'node:child_process'
|
|
import {
|
|
chmod,
|
|
copyFile,
|
|
mkdir,
|
|
mkdtemp,
|
|
readFile,
|
|
realpath,
|
|
rm,
|
|
symlink,
|
|
writeFile
|
|
} from 'node:fs/promises'
|
|
import { tmpdir } from 'node:os'
|
|
import path from 'node:path'
|
|
import { afterEach, describe, expect, it } from 'vitest'
|
|
import { parse } from 'yaml'
|
|
import { observeSkillPackage } from '../../src/main/skills/skill-package-identity'
|
|
import {
|
|
classifyFile,
|
|
collectPackageFiles,
|
|
describeFile,
|
|
normalizeText,
|
|
packageDigest,
|
|
sortManifestFiles
|
|
} from './generate-skill-bundle-manifest.mjs'
|
|
|
|
const temporaryDirectories = []
|
|
const REPO_ROOT = path.resolve(import.meta.dirname, '..', '..')
|
|
|
|
async function createPackage() {
|
|
const directory = await mkdtemp(path.join(tmpdir(), 'orca-skill-manifest-'))
|
|
temporaryDirectories.push(directory)
|
|
return directory
|
|
}
|
|
|
|
// Why: the generator resolves its repo root from its own location, so a copy of
|
|
// the script inside a throwaway tree exercises the real CLI without touching
|
|
// resources/skills.
|
|
async function createGeneratorSandbox() {
|
|
// Node resolves the entry point through symlinks, so the script's own
|
|
// repo-root check only matches when the sandbox path is already resolved.
|
|
const root = await realpath(await createPackage())
|
|
const skillRoot = path.join(root, 'skills', 'demo')
|
|
const script = path.join(root, 'config', 'scripts', 'generate-skill-bundle-manifest.mjs')
|
|
await mkdir(path.dirname(script), { recursive: true })
|
|
await mkdir(skillRoot, { recursive: true })
|
|
await copyFile(path.join(import.meta.dirname, 'generate-skill-bundle-manifest.mjs'), script)
|
|
await writeFile(path.join(skillRoot, 'SKILL.md'), 'demo skill\n')
|
|
return {
|
|
generate: (...args) => execFileSync(process.execPath, [script, ...args], { stdio: 'pipe' }),
|
|
read: (name) => readFile(path.join(root, 'resources', 'skills', name), 'utf8'),
|
|
editSkill: (body) => writeFile(path.join(skillRoot, 'SKILL.md'), body)
|
|
}
|
|
}
|
|
|
|
afterEach(async () => {
|
|
await Promise.all(
|
|
temporaryDirectories.splice(0).map((directory) => rm(directory, { recursive: true }))
|
|
)
|
|
})
|
|
|
|
describe('skill bundle manifest generator', () => {
|
|
it('folds platform line endings for text identity', () => {
|
|
const lf = Buffer.from('first\nsecond\n')
|
|
const crlf = Buffer.from('first\r\nsecond\r\n')
|
|
|
|
expect(classifyFile(lf)).toBe('text')
|
|
expect(normalizeText(crlf)).toEqual(lf)
|
|
})
|
|
|
|
it('classifies null-containing and invalid UTF-8 content as binary', () => {
|
|
expect(classifyFile(Buffer.from([0, 1, 2]))).toBe('binary')
|
|
expect(classifyFile(Buffer.from([0xc3, 0x28]))).toBe('binary')
|
|
})
|
|
|
|
it('uses normalized text identity but exact executable identity', () => {
|
|
const skillFile = describeFile('SKILL.md', Buffer.from('line one\r\nline two\r\n'), false)
|
|
const executable = describeFile('run.sh', Buffer.from('#!/bin/sh\r\necho ok\r\n'), true)
|
|
|
|
expect(skillFile.identitySha256).toBe(skillFile.textNormalizedSha256)
|
|
expect(skillFile.identitySha256).not.toBe(skillFile.exactSha256)
|
|
expect(executable.exactSha256).not.toBe(executable.textNormalizedSha256)
|
|
expect(executable.identitySha256).toBe(executable.exactSha256)
|
|
expect(packageDigest([skillFile, executable])).toMatch(/^[a-f0-9]{64}$/)
|
|
})
|
|
|
|
it('orders files identically however they were listed', async () => {
|
|
const packageRoot = await createPackage()
|
|
await mkdir(path.join(packageRoot, 'sub'))
|
|
for (const name of ['apple.md', 'sub.md', 'Zebra.md', path.join('sub', 'inner.txt')]) {
|
|
await writeFile(path.join(packageRoot, name), `${name}\n`)
|
|
}
|
|
const walked = await collectPackageFiles(packageRoot)
|
|
|
|
// Why: git ls-tree emits [Zebra.md, apple.md, sub.md, sub/inner.txt]; the digest
|
|
// hashes files in array order, so every producer must share one order.
|
|
const gitOrdered = ['Zebra.md', 'apple.md', 'sub.md', 'sub/inner.txt'].map((manifestPath) =>
|
|
walked.find((file) => file.path === manifestPath)
|
|
)
|
|
|
|
expect(sortManifestFiles(gitOrdered)).toEqual(walked)
|
|
expect(packageDigest(sortManifestFiles(gitOrdered))).toBe(packageDigest(walked))
|
|
expect(walked.map((file) => file.path)).toEqual([
|
|
'Zebra.md',
|
|
'apple.md',
|
|
'sub/inner.txt',
|
|
'sub.md'
|
|
])
|
|
})
|
|
|
|
it('writes a schema-3 manifest and fails verification once a skill changes', async () => {
|
|
const sandbox = await createGeneratorSandbox()
|
|
sandbox.generate('--write')
|
|
const manifest = JSON.parse(await sandbox.read('current-manifest.json'))
|
|
expect(manifest.schemaVersion).toBe(3)
|
|
expect(manifest.skills.map((skill) => Object.keys(skill))).toEqual([
|
|
['name', 'sourcePath', 'packageDigest', 'files']
|
|
])
|
|
expect(() => sandbox.generate()).not.toThrow()
|
|
|
|
await sandbox.editSkill('changed\n')
|
|
expect(() => sandbox.generate()).toThrow()
|
|
})
|
|
|
|
it.runIf(process.platform !== 'win32')(
|
|
'rejects executable files in shipped skill packages',
|
|
async () => {
|
|
const packageRoot = await createPackage()
|
|
await writeFile(path.join(packageRoot, 'SKILL.md'), 'skill\n')
|
|
await writeFile(path.join(packageRoot, 'run.sh'), '#!/bin/sh\necho ok\n')
|
|
await chmod(path.join(packageRoot, 'run.sh'), 0o755)
|
|
|
|
await expect(collectPackageFiles(packageRoot)).rejects.toThrow(
|
|
'Executable file is not allowed in a shipped skill: run.sh'
|
|
)
|
|
}
|
|
)
|
|
|
|
it.runIf(process.platform === 'linux')('rejects case-colliding paths', async () => {
|
|
const packageRoot = await createPackage()
|
|
await writeFile(path.join(packageRoot, 'SKILL.md'), 'skill')
|
|
await writeFile(path.join(packageRoot, 'Readme.md'), 'one')
|
|
await writeFile(path.join(packageRoot, 'README.md'), 'two')
|
|
|
|
await expect(collectPackageFiles(packageRoot)).rejects.toThrow('Case-colliding skill paths')
|
|
})
|
|
|
|
it.runIf(process.platform !== 'win32')('rejects symlinks inside shipped packages', async () => {
|
|
const packageRoot = await createPackage()
|
|
await writeFile(path.join(packageRoot, 'SKILL.md'), 'skill')
|
|
await symlink('SKILL.md', path.join(packageRoot, 'linked.md'))
|
|
|
|
await expect(collectPackageFiles(packageRoot)).rejects.toThrow(
|
|
'Symlink is not allowed in a shipped skill'
|
|
)
|
|
})
|
|
|
|
it('ignores OS-authored sidecars a working tree may carry', async () => {
|
|
const packageRoot = await createPackage()
|
|
await writeFile(path.join(packageRoot, 'SKILL.md'), 'demo skill\n')
|
|
await mkdir(path.join(packageRoot, 'references'))
|
|
await writeFile(path.join(packageRoot, 'references', 'guide.md'), 'nested\n')
|
|
const pristine = await collectPackageFiles(packageRoot)
|
|
expect(pristine.map((file) => file.path)).toEqual(['SKILL.md', 'references/guide.md'])
|
|
// Finder writes .DS_Store into any browsed folder, and it is gitignored — so without
|
|
// this the committed artifacts read as stale and lint fails for that developer, while
|
|
// the scanner would have no snapshot a real install could match.
|
|
await writeFile(path.join(packageRoot, '.DS_Store'), Buffer.from([0, 1, 2, 3]))
|
|
await writeFile(path.join(packageRoot, '._SKILL.md'), Buffer.from([0, 5]))
|
|
await writeFile(path.join(packageRoot, 'Thumbs.db'), Buffer.from([9]))
|
|
// Nested folders get browsed too, and a sidecar there shifts the same index-aligned list.
|
|
await writeFile(path.join(packageRoot, 'references', '.DS_Store'), Buffer.from([7]))
|
|
|
|
expect(await collectPackageFiles(packageRoot)).toEqual(pristine)
|
|
})
|
|
|
|
it('still records an unexpected file that is not OS metadata', async () => {
|
|
const packageRoot = await createPackage()
|
|
await writeFile(path.join(packageRoot, 'SKILL.md'), 'demo skill\n')
|
|
await writeFile(path.join(packageRoot, 'payload.sh'), 'echo hi\n')
|
|
|
|
expect((await collectPackageFiles(packageRoot)).map((file) => file.path)).toEqual([
|
|
'SKILL.md',
|
|
'payload.sh'
|
|
])
|
|
})
|
|
|
|
it('keeps guarding a directory or link that only wears an OS metadata name', async () => {
|
|
const packageRoot = await createPackage()
|
|
await writeFile(path.join(packageRoot, 'SKILL.md'), 'demo skill\n')
|
|
// Only plain files are OS-authored, so a subtree behind one of these names is real
|
|
// content that must stay in the manifest instead of shipping unrecorded.
|
|
await mkdir(path.join(packageRoot, '.DS_Store'))
|
|
await writeFile(path.join(packageRoot, '.DS_Store', 'payload.sh'), 'echo hi\n')
|
|
|
|
expect((await collectPackageFiles(packageRoot)).map((file) => file.path)).toEqual([
|
|
'.DS_Store/payload.sh',
|
|
'SKILL.md'
|
|
])
|
|
|
|
if (process.platform !== 'win32') {
|
|
await rm(path.join(packageRoot, '.DS_Store'), { recursive: true })
|
|
await symlink('SKILL.md', path.join(packageRoot, '._SKILL.md'))
|
|
await expect(collectPackageFiles(packageRoot)).rejects.toThrow(
|
|
'Symlink is not allowed in a shipped skill'
|
|
)
|
|
}
|
|
})
|
|
|
|
// Why: the predicate is hand-copied from the scanner, and an asymmetric skip is worse than
|
|
// no skip — one side would bake in content the other can never observe, leaving every
|
|
// install permanently unrecognized. Compared through both walkers so ordering and the
|
|
// case-fold map are covered too, not just the name test.
|
|
it('skips exactly the names the scanner skips', async () => {
|
|
const packageRoot = await createPackage()
|
|
for (const name of [
|
|
'SKILL.md',
|
|
'.DS_Store',
|
|
'.ds_store',
|
|
'.DS_STORE',
|
|
'Thumbs.db',
|
|
'THUMBS.DB',
|
|
'ehthumbs.db',
|
|
'desktop.ini',
|
|
'Desktop.INI',
|
|
'._SKILL.md',
|
|
'._',
|
|
// Near misses that both sides must keep.
|
|
'.dsstore',
|
|
'ds_store.md',
|
|
'_SKILL.md',
|
|
'.DS_Store.md'
|
|
]) {
|
|
await writeFile(path.join(packageRoot, name), `${name}\n`)
|
|
}
|
|
|
|
const generated = (await collectPackageFiles(packageRoot)).map((file) => file.path)
|
|
expect(generated).toEqual((await observeSkillPackage(packageRoot)).files.map((f) => f.path))
|
|
expect(generated).toEqual(['.DS_Store.md', '.dsstore', 'SKILL.md', '_SKILL.md', 'ds_store.md'])
|
|
})
|
|
|
|
// Why: every step in the cut job shares one workspace and one index, so any of
|
|
// them can stage a regenerated manifest and the bump step's own commit then
|
|
// carries it into the tag. The cut asserts what its commit carries; this test
|
|
// pins that guard and adds a tripwire for the literal spellings.
|
|
it('keeps the whole release-cut job off skill regeneration', async () => {
|
|
const workflow = parse(
|
|
await readFile(path.join(REPO_ROOT, '.github/workflows/release-cut.yml'), 'utf8')
|
|
)
|
|
const runSteps = workflow.jobs.cut.steps
|
|
.filter((step) => typeof step.run === 'string')
|
|
.map((step) => ({ name: step.name ?? '(unnamed)', run: step.run.replace(/^\s*#.*$/gm, '') }))
|
|
const bumpStep = runSteps.find((step) => step.name === 'Bump package.json and tag')
|
|
|
|
// Asserted on the commit rather than the index because `git commit -a/-i/--only/
|
|
// <pathspec>` bypasses the index. -F is part of the contract; without it `.` admits a
|
|
// path like packageXjson. Dropping -m makes a merge commit report nothing.
|
|
expect(bumpStep.run).toMatch(
|
|
/git diff-tree --no-commit-id --name-only -r -m --first-parent HEAD\s*\|\s*grep -vxF -e 'package\.json'/
|
|
)
|
|
expect(bumpStep.run.indexOf('grep -vxF')).toBeLessThan(bumpStep.run.indexOf('git tag'))
|
|
// ...and that it aborts. A guard degraded to a warning still reads as covered.
|
|
expect(bumpStep.run).toMatch(
|
|
/if \[\[ -n "\$committed" \]\]; then(?:(?!\bfi\b)[\s\S])*exit 1[\s\S]*?fi/
|
|
)
|
|
for (const step of runSteps) {
|
|
expect(step.run, step.name).not.toMatch(
|
|
/resources[/\\]skills|--write|generate:skill-bundle-manifest/
|
|
)
|
|
}
|
|
})
|
|
})
|