fix(skills): harden managed update safety

This commit is contained in:
Brennan Benson
2026-07-12 23:44:14 -07:00
parent f4a12bcd6b
commit a0148e7442
22 changed files with 581 additions and 127 deletions
+4 -3
View File
@@ -3702,7 +3702,8 @@
{
"file": "src/main/skills/skill-management-inventory.test.ts",
"assertions": [
"oversized and access-denied candidates fail closed without hiding healthy siblings"
"oversized and access-denied candidates fail closed without hiding healthy siblings",
"candidate filesystem classification never exceeds four concurrent tasks"
]
}
],
@@ -3714,7 +3715,7 @@
"command": "pnpm exec vitest run --config config/vitest.config.ts src/main/skills/skill-package-observation-limits.test.ts src/main/skills/skill-management-inventory.test.ts",
"result": "passed",
"durationSeconds": 0.183,
"summary": "15 deterministic bounded-observation and sibling-isolation tests passed."
"summary": "16 deterministic bounded-observation, concurrency, and sibling-isolation tests passed."
}
],
"runtimeBudget": { "p95Seconds": 10, "scope": "local main-process unit tests" },
@@ -3728,7 +3729,7 @@
},
"performanceBudget": {
"required": true,
"evidence": "Production observation is capped at depth 16, 2,048 entries, 512 files, 4 MiB per file, and 32 MiB aggregate per candidate with no polling or subprocesses."
"evidence": "Production observation is capped at four concurrent candidates, depth 16, 2,048 entries, 512 files, 4 MiB per file, and 32 MiB aggregate per candidate with no polling or subprocesses."
},
"promotionCriteria": [
"Accumulate the required soak runs across macOS, Linux, and Windows.",
@@ -287,6 +287,49 @@ function readJsonAtRef(ref, relativePath) {
}
}
function gitRefExists(ref) {
try {
execFileSync('git', ['rev-parse', '--verify', '--quiet', `${ref}^{commit}`], {
stdio: 'ignore'
})
return true
} catch {
return false
}
}
function assertReleaseMappingProvenance(
artifacts,
{
allowUnreleasedAppVersion = null,
refExists = gitRefExists,
digestAtRef = (ref, name) => packageDigest(collectGitPackageFiles(ref, name))
} = {}
) {
for (const release of artifacts.releaseMapping.releases) {
const releaseRef = `v${release.appVersion}`
if (!refExists(releaseRef)) {
if (release.appVersion === allowUnreleasedAppVersion) {
continue
}
throw new Error(`Skill release mapping references missing tag ${releaseRef}`)
}
for (const [name, revision] of Object.entries(release.skills)) {
const snapshot = (artifacts.snapshotRegistry.skills[name] ?? []).find(
(entry) => entry.releaseRevision === revision
)
if (!snapshot) {
throw new Error(`Skill release mapping references unknown ${name} revision ${revision}`)
}
if (digestAtRef(releaseRef, name) !== snapshot.packageDigest) {
throw new Error(
`Skill release mapping for ${releaseRef} does not match shipped ${name} revision ${revision}`
)
}
}
}
}
function assertBaseHistoryPreserved(baseRef, artifacts) {
if (!baseRef) {
return
@@ -442,6 +485,11 @@ async function main() {
recordRelease,
bootstrapHistory
})
// Why: a package version can remain unchanged while main advances; only
// tag bytes prove that a historical revision actually shipped.
assertReleaseMappingProvenance(artifacts, {
allowUnreleasedAppVersion: recordRelease ? packageJson.version : null
})
assertBaseHistoryPreserved(baseRef, artifacts)
await (write ? writeArtifacts : verifyArtifacts)(artifacts)
}
@@ -454,6 +502,7 @@ if (process.argv[1] && path.resolve(process.argv[1]) === import.meta.filename) {
}
export {
assertReleaseMappingProvenance,
buildArtifacts,
classifyFile,
collectPackageFiles,
@@ -4,6 +4,7 @@ import { tmpdir } from 'node:os'
import path from 'node:path'
import { afterEach, describe, expect, it } from 'vitest'
import {
assertReleaseMappingProvenance,
classifyFile,
collectPackageFiles,
gitTreeSha,
@@ -95,4 +96,43 @@ describe('skill bundle manifest generator', () => {
const files = await collectPackageFiles(packageRoot)
expect(gitTreeSha(files)).toMatch(/^[a-f0-9]{40}$/)
})
it('rejects a released revision that differs from the tagged package', () => {
const digest = 'a'.repeat(64)
const artifacts = {
snapshotRegistry: {
skills: {
orchestration: [{ releaseRevision: 2, packageDigest: digest }]
}
},
releaseMapping: {
releases: [{ appVersion: '1.2.3', skills: { orchestration: 2 } }]
}
}
expect(() =>
assertReleaseMappingProvenance(artifacts, {
refExists: () => true,
digestAtRef: () => 'b'.repeat(64)
})
).toThrow('does not match shipped orchestration revision 2')
})
it('allows only the in-progress release cut to precede its tag', () => {
const artifacts = {
snapshotRegistry: { skills: { 'orca-cli': [{ releaseRevision: 1, packageDigest: 'a' }] } },
releaseMapping: { releases: [{ appVersion: '1.2.4', skills: { 'orca-cli': 1 } }] }
}
const options = { refExists: () => false, digestAtRef: () => 'a' }
expect(() => assertReleaseMappingProvenance(artifacts, options)).toThrow(
'references missing tag v1.2.4'
)
expect(() =>
assertReleaseMappingProvenance(artifacts, {
...options,
allowUnreleasedAppVersion: '1.2.4'
})
).not.toThrow()
})
})
+1 -1
View File
@@ -22,7 +22,7 @@
"check:max-lines-ratchet": "node config/scripts/check-max-lines-ratchet.mjs",
"check:feature-wall-assets": "node config/scripts/check-feature-wall-assets.mjs",
"generate:skill-bundle-manifest": "node config/scripts/generate-skill-bundle-manifest.mjs --write",
"bootstrap:skill-release-history": "node config/scripts/generate-skill-bundle-manifest.mjs --write --record-release --bootstrap-release-history",
"bootstrap:skill-release-history": "node config/scripts/generate-skill-bundle-manifest.mjs --write --bootstrap-release-history",
"record-release:skill-bundle-manifest": "node config/scripts/generate-skill-bundle-manifest.mjs --write --record-release",
"verify:skill-bundle-manifest": "node config/scripts/generate-skill-bundle-manifest.mjs",
"verify:macos-entitlements": "node config/scripts/verify-macos-entitlements.mjs",
+1 -1
View File
@@ -139,7 +139,7 @@
"name": "orchestration",
"sourcePath": "skills/orchestration",
"appVersion": "1.4.138-rc.2",
"releaseRevision": 23,
"releaseRevision": 25,
"packageDigest": "dab5cc1bf87b12838a53dda90f1f4f35d1f77da66bcc63840574531eff600d2b",
"gitTreeSha": "f7a017f3e1f14dafed0c59717bec37c022075be2",
"files": [
+14 -1
View File
@@ -459,7 +459,7 @@
}
},
{
"appVersion": "1.4.138-rc.2",
"appVersion": "1.4.138-rc.3",
"skills": {
"computer-use": 4,
"linear-tickets": 4,
@@ -470,6 +470,19 @@
"orca-per-workspace-env": 2,
"orchestration": 23
}
},
{
"appVersion": "1.4.138-rc.4",
"skills": {
"computer-use": 4,
"linear-tickets": 4,
"orca-cli": 31,
"orca-emulator": 3,
"orca-emulator-android": 1,
"orca-linear": 2,
"orca-per-workspace-env": 2,
"orchestration": 24
}
}
]
}
+32
View File
@@ -867,6 +867,38 @@
"identitySha256": "b73c7c665aa233a782ba81982ebc954e220fdf1c1c6ca2b6bf38133c1cd76fc9"
}
]
},
{
"releaseRevision": 24,
"packageDigest": "9fbfa2ae3f3f99441563a4b8b1c6302107944480db8718ddb326a862a51f7ab9",
"gitTreeSha": "086c41e0b353b4908d2963694b4a7c791d4b3982",
"files": [
{
"path": "SKILL.md",
"size": 22861,
"executable": false,
"classification": "text",
"exactSha256": "0cb898d37560ab7ac4cb477a2d7e8ba3e593d45affd6d7e9f99151879aaeb1a5",
"textNormalizedSha256": "0cb898d37560ab7ac4cb477a2d7e8ba3e593d45affd6d7e9f99151879aaeb1a5",
"identitySha256": "0cb898d37560ab7ac4cb477a2d7e8ba3e593d45affd6d7e9f99151879aaeb1a5"
}
]
},
{
"releaseRevision": 25,
"packageDigest": "dab5cc1bf87b12838a53dda90f1f4f35d1f77da66bcc63840574531eff600d2b",
"gitTreeSha": "f7a017f3e1f14dafed0c59717bec37c022075be2",
"files": [
{
"path": "SKILL.md",
"size": 22850,
"executable": false,
"classification": "text",
"exactSha256": "b73c7c665aa233a782ba81982ebc954e220fdf1c1c6ca2b6bf38133c1cd76fc9",
"textNormalizedSha256": "b73c7c665aa233a782ba81982ebc954e220fdf1c1c6ca2b6bf38133c1cd76fc9",
"identitySha256": "b73c7c665aa233a782ba81982ebc954e220fdf1c1c6ca2b6bf38133c1cd76fc9"
}
]
}
],
"mobile-fit-debug": [
@@ -0,0 +1,25 @@
import type { ClassifiedSkillCandidate } from './skill-management-inaccessible-candidate'
export const SKILL_CANDIDATE_SCAN_CONCURRENCY = 4
export async function runSkillCandidateTasks(
tasks: readonly (() => Promise<ClassifiedSkillCandidate | null>)[]
): Promise<(ClassifiedSkillCandidate | null)[]> {
const results: (ClassifiedSkillCandidate | null)[] = tasks.map(() => null)
let nextIndex = 0
const workers = Array.from(
{ length: Math.min(SKILL_CANDIDATE_SCAN_CONCURRENCY, tasks.length) },
async () => {
for (;;) {
const index = nextIndex
nextIndex += 1
if (index >= tasks.length) {
return
}
results[index] = await tasks[index]!()
}
}
)
await Promise.all(workers)
return results
}
@@ -68,7 +68,7 @@ describe('managed skill inventory', () => {
it('suppresses only the explicitly dismissed destination snapshot tuple', async () => {
const home = await createHome()
await installSkill(home, '.agents', 'orca-cli')
await installSkill(home, '.agents', 'orchestration')
await installSkill(home, '.agents', 'computer-use')
const initial = await inventoryManagedSkills({
ledger: emptySkillManagementLedger(),
hostId: 'local',
@@ -99,7 +99,7 @@ describe('managed skill inventory', () => {
expect(next.installations.find((entry) => entry.name === 'orca-cli')).toMatchObject({
adoptionPromptEligible: false
})
expect(next.installations.find((entry) => entry.name === 'orchestration')).toMatchObject({
expect(next.installations.find((entry) => entry.name === 'computer-use')).toMatchObject({
adoptionPromptEligible: true
})
})
@@ -240,7 +240,7 @@ describe('managed skill inventory', () => {
it('bounds one oversized candidate without hiding healthy siblings', async () => {
const home = await createHome()
const oversized = await installSkill(home, '.agents', 'orca-cli')
await installSkill(home, '.agents', 'orchestration')
await installSkill(home, '.agents', 'computer-use')
const oversizedFile = join(oversized, 'oversized.bin')
await writeFile(oversizedFile, '')
await truncate(oversizedFile, 4 * 1024 * 1024 + 1)
@@ -256,7 +256,7 @@ describe('managed skill inventory', () => {
status: 'unknown',
errorCategory: 'skill-package-file-size-limit'
})
expect(inventory.installations.find((entry) => entry.name === 'orchestration')).toMatchObject({
expect(inventory.installations.find((entry) => entry.name === 'computer-use')).toMatchObject({
status: 'known-current',
eligible: true
})
@@ -321,6 +321,29 @@ describe('managed skill inventory', () => {
).toBeTruthy()
})
it('caps concurrent candidate filesystem observations', async () => {
const home = await createHome()
let active = 0
let peak = 0
await inventoryManagedSkills({
ledger: emptySkillManagementLedger(),
hostId: 'local',
homeDir: home,
resourceRoot,
candidateLstat: async () => {
active += 1
peak = Math.max(peak, active)
await new Promise<void>((resolve) => queueMicrotask(resolve))
active -= 1
throw Object.assign(new Error('missing'), { code: 'ENOENT' })
}
})
expect(peak).toBeGreaterThan(1)
expect(peak).toBeLessThanOrEqual(4)
})
it.runIf(process.platform !== 'win32')(
'deduplicates a provider alias into its canonical physical copy',
async () => {
+14 -21
View File
@@ -7,8 +7,7 @@ import type {
SkillCurrentBundleEntry,
SkillManagementInstallation,
SkillManagementInventory,
SkillManagementLedger,
SkillManagementStatus
SkillManagementLedger
} from '../../shared/skill-management'
import { buildSkillDiscoverySources, type SkillScanRoot } from './skill-discovery-sources'
import { loadSkillBundleArtifacts, type SkillBundleArtifacts } from './skill-bundle-artifacts'
@@ -26,6 +25,8 @@ import {
skillTopologyPriority
} from './skill-installation-topology'
import { readOfficialSkillsCliLockEntries } from './skills-cli-lock-provenance'
import { runSkillCandidateTasks } from './skill-candidate-concurrency'
import { knownSkillManagementStatus } from './skill-management-status'
type CandidateLstat = (path: string) => Promise<Stats>
type CandidateTopology = (
@@ -34,20 +35,6 @@ type CandidateTopology = (
canonicalRootPath: string
) => Promise<ClassifiedSkillTopology>
function knownStatus(
managed: boolean,
installedRevision: number,
currentRevision: number
): SkillManagementStatus {
if (installedRevision > currentRevision) {
return 'newer-known'
}
if (installedRevision < currentRevision) {
return managed ? 'managed-update-available' : 'known-update-available'
}
return managed ? 'managed-current' : 'known-current'
}
async function classifyCandidate(args: {
root: SkillScanRoot
name: string
@@ -161,7 +148,11 @@ async function classifyCandidate(args: {
status: match
? managed && managedRecord?.lastOutcome === 'failed'
? 'update-failed'
: knownStatus(managed, match.releaseRevision, args.current.releaseRevision)
: knownSkillManagementStatus(
managed,
match.releaseRevision,
args.current.releaseRevision
)
: managedRecord
? 'modified'
: 'unknown',
@@ -222,9 +213,9 @@ export async function inventoryManagedSkills(args: {
: null
: args.xdgStateHome
})
const candidates = await Promise.all(
artifacts.manifest.skills.flatMap((current) =>
roots.map((root) =>
const candidateTasks = artifacts.manifest.skills.flatMap((current) =>
roots.map(
(root) => () =>
classifyCandidate({
root,
name: current.name,
@@ -239,9 +230,11 @@ export async function inventoryManagedSkills(args: {
candidateLstat: args.candidateLstat ?? ((path) => lstat(path)),
classifyTopology: args.classifyTopology ?? classifySkillInstallationTopology
})
)
)
)
// Why: each candidate may retain up to the package byte ceiling while it is
// hashed, so mounted/focus inventory must not fan out across every root.
const candidates = await runSkillCandidateTasks(candidateTasks)
const deduped = new Map<string, SkillManagementInstallation>()
for (const candidate of candidates) {
@@ -0,0 +1,15 @@
import type { SkillManagementStatus } from '../../shared/skill-management'
export function knownSkillManagementStatus(
managed: boolean,
installedRevision: number,
currentRevision: number
): SkillManagementStatus {
if (installedRevision > currentRevision) {
return 'newer-known'
}
if (installedRevision < currentRevision) {
return managed ? 'managed-update-available' : 'known-update-available'
}
return managed ? 'managed-current' : 'known-current'
}
@@ -1,18 +1,19 @@
import { lstat, mkdir, readFile, readdir, realpath, rm } from 'node:fs/promises'
import { dirname, join, resolve } from 'node:path'
import type { SkillManagementLedger } from '../../shared/skill-management'
import { normalizedSkillIdentityPath } from './skill-installation-topology'
import { normalizedSkillIdentityPath, skillPhysicalIdentity } from './skill-installation-topology'
import {
parseSkillTransactionMarker,
type SkillTransactionMarker
} from './skill-transaction-marker'
import { RESERVED_SKILL_TRANSACTION_DIRECTORY } from './skill-transaction-paths'
import { acquireSkillDestinationLock } from './skill-transaction-recovery'
import { recoverTransactionToPrior } from './skill-transaction-restore'
import { skillTransactionReservedRoot } from './skill-transaction-workspace'
async function assertApprovedDestinationParent(
marker: SkillTransactionMarker,
rootIdentity: string
rootIdentity: string,
rootPhysicalIdentity: string
): Promise<void> {
if (normalizedSkillIdentityPath(dirname(resolve(marker.destinationPath))) !== rootIdentity) {
throw new Error('skill-transaction-destination-outside-root')
@@ -21,7 +22,11 @@ async function assertApprovedDestinationParent(
lstat(dirname(marker.destinationPath)),
realpath(dirname(marker.destinationPath)).then(normalizedSkillIdentityPath)
])
if (parentStat.isSymbolicLink() || parentIdentity !== rootIdentity) {
if (
parentStat.isSymbolicLink() ||
parentIdentity !== rootIdentity ||
skillPhysicalIdentity(dirname(marker.destinationPath), parentStat) !== rootPhysicalIdentity
) {
throw new Error('skill-transaction-destination-outside-root')
}
}
@@ -30,19 +35,32 @@ export async function sweepOrphanedSkillTransactions(
approvedSkillsRoot: string,
ledger: SkillManagementLedger
): Promise<void> {
const linkedRoot = await Promise.all([
const roots = await Promise.all([
lstat(approvedSkillsRoot),
lstat(dirname(approvedSkillsRoot))
])
.then((entries) => entries.some((entry) => entry.isSymbolicLink()))
.catch(() => true)
if (linkedRoot) {
lstat(dirname(approvedSkillsRoot)),
realpath(approvedSkillsRoot),
realpath(dirname(approvedSkillsRoot))
]).catch(() => null)
if (
!roots ||
roots[0].isSymbolicLink() ||
roots[1].isSymbolicLink() ||
normalizedSkillIdentityPath(dirname(roots[2])) !== normalizedSkillIdentityPath(roots[3])
) {
return
}
const rootIdentity = normalizedSkillIdentityPath(await realpath(approvedSkillsRoot))
const [skillsRootStat, parentRootStat, resolvedSkillsRoot] = roots
const rootIdentity = normalizedSkillIdentityPath(resolvedSkillsRoot)
const rootPhysicalIdentity = skillPhysicalIdentity(approvedSkillsRoot, skillsRootStat)
// Why: mount changes invalidate the old workspace authority; sweeping both
// possible locations could mutate a directory Orca did not reserve now.
const reservedRoots = [
join(dirname(approvedSkillsRoot), RESERVED_SKILL_TRANSACTION_DIRECTORY),
join(approvedSkillsRoot, RESERVED_SKILL_TRANSACTION_DIRECTORY)
skillTransactionReservedRoot({
skillsRoot: approvedSkillsRoot,
skillsRootDevice: skillsRootStat.dev,
parentRoot: dirname(approvedSkillsRoot),
parentRootDevice: parentRootStat.dev
})
]
for (const reservedRoot of reservedRoots) {
const reservedRootSafe = await lstat(reservedRoot)
@@ -64,7 +82,7 @@ export async function sweepOrphanedSkillTransactions(
continue
}
try {
await assertApprovedDestinationParent(marker, rootIdentity)
await assertApprovedDestinationParent(marker, rootIdentity, rootPhysicalIdentity)
} catch {
continue
}
@@ -90,7 +108,8 @@ export async function sweepOrphanedSkillTransactions(
await recoverTransactionToPrior({
transactionRoot,
marker,
assertParentAuthority: () => assertApprovedDestinationParent(marker, rootIdentity),
assertParentAuthority: () =>
assertApprovedDestinationParent(marker, rootIdentity, rootPhysicalIdentity),
currentDisposition: committed ? 'committed' : 'restore'
})
await rm(transactionRoot, { recursive: true, force: true })
+24 -3
View File
@@ -1,4 +1,4 @@
import { mkdir, rm, stat } from 'node:fs/promises'
import { mkdir, readFile, rm, stat } from 'node:fs/promises'
import { dirname, join } from 'node:path'
import type {
ManagedSkillDestination,
@@ -17,6 +17,7 @@ import {
import { destinationUsesCrlf, replaceSkillPackage } from './skill-package-publication'
import type { SkillRenameRuntime } from './skill-transaction-rename'
import { createSkillTransactionContext } from './skill-package-transaction-context'
import { parseSkillTransactionMarker } from './skill-transaction-marker'
import {
assertManagedDestinationUnchanged,
assertPublishedDestinationOwned
@@ -58,6 +59,7 @@ export async function publishBundledSkill(args: {
)
let retainTransaction = false
let publicationStarted = false
let durableCommitCompleted = false
try {
await transaction.assertAuthority(true)
await recoverMarkedSkillTransactions({
@@ -141,6 +143,7 @@ export async function publishBundledSkill(args: {
marker: verifiedMarker
})
await args.commit(destination)
durableCommitCompleted = true
return destination
} catch (error) {
try {
@@ -160,9 +163,27 @@ export async function publishBundledSkill(args: {
.then(() => true)
.catch(() => false)
if (workspaceSafe && !retainTransaction) {
await rm(transaction.paths.transactionRoot, { recursive: true, force: true }).catch(
() => undefined
const cleanupMarker = await readFile(
join(transaction.paths.transactionRoot, 'transaction.json'),
'utf8'
)
.then(parseSkillTransactionMarker)
.catch(() => null)
if (cleanupMarker?.transactionId === transaction.marker('staging').transactionId) {
try {
// Why: cleanup repeats recovery from the durable marker immediately
// before recursive removal, including after an ambiguous rollback.
await recoverTransactionToPrior({
transactionRoot: transaction.paths.transactionRoot,
marker: cleanupMarker,
assertParentAuthority: transaction.assertParentAuthority,
currentDisposition: durableCommitCompleted ? 'committed' : 'restore'
})
await rm(transaction.paths.transactionRoot, { recursive: true, force: true })
} catch {
retainTransaction = true
}
}
}
if (workspaceSafe) {
await releaseLock().catch(() => undefined)
@@ -8,10 +8,8 @@ import type {
import { assertManagedSkillParentTopology } from './skill-installation-topology'
import type { PublicationCallbacks } from './skill-package-publication'
import { assertManagedDestinationUnchanged } from './skill-published-destination-authority'
import {
writeSkillTransactionMarker,
type SkillTransactionMarker
} from './skill-transaction-marker'
import type { SkillTransactionMarker } from './skill-transaction-marker'
import { writeSkillTransactionMarker } from './skill-transaction-marker-write'
import { assertTransactionOwnedState } from './skill-transaction-owned-state'
import type { SkillPublicationState } from './skill-transaction-publication-state'
import {
@@ -0,0 +1,76 @@
import { randomUUID } from 'node:crypto'
import { lstat, readFile, rename, unlink, writeFile } from 'node:fs/promises'
import { join } from 'node:path'
import { skillPhysicalIdentity } from './skill-installation-topology'
import {
parseSkillTransactionMarker,
type SkillTransactionMarker
} from './skill-transaction-marker'
export async function writeSkillTransactionMarker(
transactionRoot: string,
marker: SkillTransactionMarker,
beforeMutation: () => Promise<void> = async () => undefined
): Promise<void> {
const markerPath = join(transactionRoot, 'transaction.json')
const temporaryPath = join(transactionRoot, `.transaction-${randomUUID()}.tmp`)
const serialized = JSON.stringify(marker)
if (!parseSkillTransactionMarker(serialized)) {
throw new Error('skill-transaction-marker-invalid')
}
let temporaryPhysicalIdentity: string | null = null
const assertTemporaryOwned = async (): Promise<void> => {
const entry = await lstat(temporaryPath)
if (
!entry.isFile() ||
entry.isSymbolicLink() ||
skillPhysicalIdentity(temporaryPath, entry) !== temporaryPhysicalIdentity
) {
throw new Error('skill-transaction-marker-changed')
}
}
const assertMarkerSlotOwned = async (): Promise<void> => {
const entry = await lstat(markerPath).catch((error) => {
if (error && typeof error === 'object' && 'code' in error && error.code === 'ENOENT') {
return null
}
throw error
})
if (!entry) {
return
}
const current =
entry.isFile() && !entry.isSymbolicLink()
? await readFile(markerPath, 'utf8')
.then(parseSkillTransactionMarker)
.catch(() => null)
: null
if (current?.transactionId !== marker.transactionId) {
throw new Error('skill-transaction-marker-changed')
}
}
try {
await beforeMutation()
await writeFile(temporaryPath, serialized, { flag: 'wx' })
temporaryPhysicalIdentity = skillPhysicalIdentity(temporaryPath, await lstat(temporaryPath))
// Why: recovery must observe either the old or new phase, never a torn
// marker that could discard the only verified rollback package.
await beforeMutation()
await assertMarkerSlotOwned()
await assertTemporaryOwned()
await rename(temporaryPath, markerPath)
} finally {
if (
temporaryPhysicalIdentity &&
(await beforeMutation()
.then(() => true)
.catch(() => false))
) {
// Why: a redirected or independently replaced temporary is not
// transaction-owned cleanup, even though its randomized name matches.
await assertTemporaryOwned()
.then(() => unlink(temporaryPath))
.catch(() => undefined)
}
}
}
+1 -33
View File
@@ -1,6 +1,4 @@
import { randomUUID } from 'node:crypto'
import { rename, rm, writeFile } from 'node:fs/promises'
import { basename, isAbsolute, join, posix } from 'node:path'
import { basename, isAbsolute, posix } from 'node:path'
import { normalizeExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
import type { SkillReleasedSnapshot } from '../../shared/skill-management'
import { normalizedSkillIdentityPath } from './skill-installation-topology'
@@ -259,33 +257,3 @@ export function parseSkillTransactionMarker(value: string): SkillTransactionMark
return null
}
}
export async function writeSkillTransactionMarker(
transactionRoot: string,
marker: SkillTransactionMarker,
beforeMutation: () => Promise<void> = async () => undefined
): Promise<void> {
const markerPath = join(transactionRoot, 'transaction.json')
const temporaryPath = join(transactionRoot, `.transaction-${randomUUID()}.tmp`)
const serialized = JSON.stringify(marker)
if (!parseSkillTransactionMarker(serialized)) {
throw new Error('skill-transaction-marker-invalid')
}
try {
await beforeMutation()
await writeFile(temporaryPath, serialized)
// Why: recovery must observe either the old or new phase, never a torn
// marker that could discard the only verified rollback package.
await beforeMutation()
await rename(temporaryPath, markerPath)
} finally {
// Why: a redirected transaction path is no longer Orca-owned cleanup.
if (
await beforeMutation()
.then(() => true)
.catch(() => false)
) {
await rm(temporaryPath, { force: true }).catch(() => undefined)
}
}
}
@@ -24,9 +24,9 @@ import {
import { sweepOrphanedSkillTransactions } from './skill-orphan-transaction-sweep'
import {
parseSkillTransactionMarker,
writeSkillTransactionMarker,
type SkillTransactionMarker
} from './skill-transaction-marker'
import { writeSkillTransactionMarker } from './skill-transaction-marker-write'
import { emptySkillManagementLedger } from './skill-management-ledger'
const temporaryRoots: string[] = []
@@ -547,7 +547,13 @@ describe('skill transaction recovery', () => {
await mkdir(lockRoot)
await writeFile(
join(lockRoot, 'owner.json'),
JSON.stringify({ schemaVersion: 1, pid: 2_147_483_647 })
JSON.stringify({
schemaVersion: 1,
pid: 2_147_483_647,
lockId: '00000000-0000-4000-8000-000000000000',
createdAt: 1,
physicalIdentity: skillPhysicalIdentity(lockRoot, await stat(lockRoot))
})
)
const release = await acquireSkillDestinationLock(lockRoot)
@@ -555,6 +561,37 @@ describe('skill transaction recovery', () => {
await release()
})
it('does not release a lock whose exact ownership marker changed', async () => {
const { root } = await fixture()
const lockRoot = join(root, 'lock')
const release = await acquireSkillDestinationLock(lockRoot)
const owner = JSON.parse(await readFile(join(lockRoot, 'owner.json'), 'utf8'))
await writeFile(
join(lockRoot, 'owner.json'),
JSON.stringify({ ...owner, lockId: '11111111-1111-4111-8111-111111111111' })
)
await expect(release()).rejects.toThrow('skill-update-lock-changed')
expect(await lstat(lockRoot)).toBeTruthy()
})
it('removes its exact unmarked lock when authority fails during acquisition', async () => {
const { root } = await fixture()
const lockRoot = join(root, 'lock')
let authorityChecks = 0
await expect(
acquireSkillDestinationLock(lockRoot, async () => {
authorityChecks += 1
if (authorityChecks === 2) {
throw new Error('skill-topology-changed')
}
})
).rejects.toThrow('skill-topology-changed')
expect(await lstat(lockRoot).catch(() => null)).toBeNull()
})
it('restores a self-contained orphan before inventory has loaded a ledger', async () => {
const { root, live, record, snapshot } = await fixture()
const reservedRoot = join(root, '.orca-skill-transactions')
@@ -596,6 +633,17 @@ describe('skill transaction recovery', () => {
await release()
})
it('does not sweep the reserved location for a different mount topology', async () => {
const { root, record, snapshot } = await fixture()
const transaction = join(root, 'skills', '.orca-skill-transactions', 'wrong-workspace')
await mkdir(transaction, { recursive: true })
await writeSkillTransactionMarker(transaction, marker(record, snapshot, 'staging'))
await sweepOrphanedSkillTransactions(join(root, 'skills'), emptySkillManagementLedger())
expect(await readFile(join(transaction, 'transaction.json'), 'utf8')).toBeTruthy()
})
it('ignores a valid marker whose destination is outside the approved root', async () => {
const { root, snapshot } = await fixture()
const outside = join(root, 'outside', 'orca-cli')
+125 -24
View File
@@ -1,9 +1,11 @@
import { mkdir, readFile, readdir, rm, writeFile } from 'node:fs/promises'
import { randomUUID } from 'node:crypto'
import { lstat, mkdir, readFile, readdir, rm, writeFile } from 'node:fs/promises'
import { join } from 'node:path'
import type { ManagedSkillDestination } from '../../shared/skill-management'
import {
assertManagedSkillParentTopology,
normalizedSkillIdentityPath
normalizedSkillIdentityPath,
skillPhysicalIdentity
} from './skill-installation-topology'
import { parseSkillTransactionMarker } from './skill-transaction-marker'
import { recoverTransactionToPrior } from './skill-transaction-restore'
@@ -59,43 +61,142 @@ function processIsAlive(pid: number): boolean {
}
}
type SkillDestinationLockOwner = {
schemaVersion: 1
pid: number
lockId: string
createdAt: number
physicalIdentity: string
}
function parseSkillDestinationLockOwner(value: unknown): SkillDestinationLockOwner | null {
if (
!value ||
typeof value !== 'object' ||
!('schemaVersion' in value) ||
value.schemaVersion !== 1 ||
!('pid' in value) ||
typeof value.pid !== 'number' ||
!Number.isSafeInteger(value.pid) ||
value.pid <= 0 ||
!('lockId' in value) ||
typeof value.lockId !== 'string' ||
!/^[a-f0-9-]{36}$/.test(value.lockId) ||
!('createdAt' in value) ||
typeof value.createdAt !== 'number' ||
!Number.isFinite(value.createdAt) ||
!('physicalIdentity' in value) ||
typeof value.physicalIdentity !== 'string' ||
value.physicalIdentity.length === 0
) {
return null
}
return value as SkillDestinationLockOwner
}
async function inspectSkillDestinationLock(lockRoot: string): Promise<SkillDestinationLockOwner> {
const entry = await lstat(lockRoot).catch(() => null)
if (!entry?.isDirectory() || entry.isSymbolicLink()) {
throw new Error('skill-update-busy')
}
const owner = await readFile(join(lockRoot, 'owner.json'), 'utf8')
.then((value) => parseSkillDestinationLockOwner(JSON.parse(value)))
.catch(() => null)
if (!owner || skillPhysicalIdentity(lockRoot, entry) !== owner.physicalIdentity) {
throw new Error('skill-update-busy')
}
return owner
}
async function assertUnpublishedSkillDestinationLock(
lockRoot: string,
physicalIdentity: string
): Promise<void> {
const [entry, children] = await Promise.all([
lstat(lockRoot),
readdir(lockRoot, { withFileTypes: true })
])
if (
!entry.isDirectory() ||
entry.isSymbolicLink() ||
skillPhysicalIdentity(lockRoot, entry) !== physicalIdentity ||
children.some(
(child) => child.name !== 'owner.json' || !child.isFile() || child.isSymbolicLink()
)
) {
throw new Error('skill-update-lock-changed')
}
}
function sameSkillDestinationLockOwner(
left: SkillDestinationLockOwner,
right: SkillDestinationLockOwner
): boolean {
return (
left.pid === right.pid &&
left.lockId === right.lockId &&
left.createdAt === right.createdAt &&
left.physicalIdentity === right.physicalIdentity
)
}
export async function acquireSkillDestinationLock(
lockRoot: string,
beforeMutation: () => Promise<void> = async () => undefined
): Promise<() => Promise<void>> {
const create = async (): Promise<void> => {
const create = async (): Promise<SkillDestinationLockOwner> => {
await beforeMutation()
await mkdir(lockRoot)
await beforeMutation()
await writeFile(
join(lockRoot, 'owner.json'),
JSON.stringify({ schemaVersion: 1, pid: process.pid, createdAt: Date.now() })
)
const entry = await lstat(lockRoot)
const owner: SkillDestinationLockOwner = {
schemaVersion: 1,
pid: process.pid,
lockId: randomUUID(),
createdAt: Date.now(),
physicalIdentity: skillPhysicalIdentity(lockRoot, entry)
}
try {
await beforeMutation()
if (skillPhysicalIdentity(lockRoot, await lstat(lockRoot)) !== owner.physicalIdentity) {
throw new Error('skill-update-busy')
}
await writeFile(join(lockRoot, 'owner.json'), JSON.stringify(owner), { flag: 'wx' })
return owner
} catch (error) {
// Why: a failed authority check or partial owner write must not leave an
// unmarked lock that permanently wedges an otherwise retryable update.
await assertUnpublishedSkillDestinationLock(lockRoot, owner.physicalIdentity)
.then(() => rm(lockRoot, { recursive: true }))
.catch(() => undefined)
throw error
}
}
let acquired: SkillDestinationLockOwner
try {
await create()
acquired = await create()
} catch (error) {
if (!error || typeof error !== 'object' || !('code' in error) || error.code !== 'EEXIST') {
throw error
}
const owner = await readFile(join(lockRoot, 'owner.json'), 'utf8')
.then((value) => JSON.parse(value) as unknown)
.catch(() => null)
if (
typeof owner !== 'object' ||
owner === null ||
!('schemaVersion' in owner) ||
owner.schemaVersion !== 1 ||
!('pid' in owner) ||
typeof owner.pid !== 'number' ||
processIsAlive(owner.pid)
) {
const owner = await inspectSkillDestinationLock(lockRoot)
if (processIsAlive(owner.pid)) {
throw new Error('skill-update-busy')
}
// Why: only an Orca-marked lock whose owner is gone is safe to reclaim.
await beforeMutation()
const currentOwner = await inspectSkillDestinationLock(lockRoot)
// Why: a second Orca may reclaim and replace the stale lock while this
// process proves destination authority; never remove its new lock.
if (!sameSkillDestinationLockOwner(owner, currentOwner)) {
throw new Error('skill-update-busy')
}
await rm(lockRoot, { recursive: true })
acquired = await create()
}
return async () => {
const currentOwner = await inspectSkillDestinationLock(lockRoot)
if (!sameSkillDestinationLockOwner(acquired, currentOwner)) {
throw new Error('skill-update-lock-changed')
}
await rm(lockRoot, { recursive: true })
await create()
}
return () => rm(lockRoot, { recursive: true, force: true })
}
+2 -4
View File
@@ -8,10 +8,8 @@ import {
clearSkillPublicationTemporary,
rollbackSkillPackageInPlace
} from './skill-package-rollback'
import {
writeSkillTransactionMarker,
type SkillTransactionMarker
} from './skill-transaction-marker'
import type { SkillTransactionMarker } from './skill-transaction-marker'
import { writeSkillTransactionMarker } from './skill-transaction-marker-write'
import { assertTransactionOwnedState } from './skill-transaction-owned-state'
import { assertSkillTransactionPackageBuild } from './skill-transaction-package-build'
import {
@@ -19,7 +19,7 @@ import {
DialogTitle
} from '../ui/dialog'
import { SettingsBadge, SettingsSubsectionHeader } from './SettingsFormControls'
import { managedSkillStatusCopy } from './managed-skill-status-copy'
import { managedSkillDiagnosticCopy, managedSkillStatusCopy } from './managed-skill-status-copy'
function statusTone(
status: SkillManagementStatus
@@ -41,14 +41,6 @@ function statusTone(
}
}
function diagnosticCopy(installation: SkillManagementInstallation): string {
const revision = installation.installedReleaseRevision
? `r${installation.installedReleaseRevision}`
: translate('auto.components.settings.ManagedOrcaSkills.unverified', 'unverified')
const digest = installation.installedPackageDigest?.slice(0, 8) ?? '—'
return `${revision} · Orca ${installation.installedAppVersion ?? installation.currentAppVersion} · ${digest}`
}
function missingVisibilityCopy(installation: SkillManagementInstallation): string | null {
if (!installation.managed) {
return null
@@ -210,7 +202,7 @@ export function ManagedOrcaSkillsSection(): React.JSX.Element {
</SettingsBadge>
</div>
<p className="font-mono text-[11px] text-muted-foreground">
{diagnosticCopy(installation)}
{managedSkillDiagnosticCopy(installation)}
</p>
{installation.description ? (
<p className="text-xs text-muted-foreground">{installation.description}</p>
@@ -0,0 +1,24 @@
import { describe, expect, it } from 'vitest'
import { managedSkillDiagnosticCopy } from './managed-skill-status-copy'
describe('managed skill diagnostic copy', () => {
it('does not attribute unknown bytes to the current Orca release', () => {
expect(
managedSkillDiagnosticCopy({
installedReleaseRevision: null,
installedAppVersion: null,
installedPackageDigest: 'abcdef0123456789'
})
).toBe('unverified · abcdef01')
})
it('shows release provenance only for a mapped snapshot', () => {
expect(
managedSkillDiagnosticCopy({
installedReleaseRevision: 7,
installedAppVersion: '1.2.3',
installedPackageDigest: 'abcdef0123456789'
})
).toBe('r7 · Orca 1.2.3 · abcdef01')
})
})
@@ -1,4 +1,7 @@
import type { SkillManagementStatus } from '../../../../shared/skill-management'
import type {
SkillManagementInstallation,
SkillManagementStatus
} from '../../../../shared/skill-management'
import { translate } from '@/i18n/i18n'
export function managedSkillStatusCopy(status: SkillManagementStatus): string {
@@ -34,3 +37,18 @@ export function managedSkillStatusCopy(status: SkillManagementStatus): string {
return translate('auto.components.settings.ManagedOrcaSkills.failed', 'Update failed')
}
}
export function managedSkillDiagnosticCopy(
installation: Pick<
SkillManagementInstallation,
'installedReleaseRevision' | 'installedAppVersion' | 'installedPackageDigest'
>
): string {
const digest = installation.installedPackageDigest?.slice(0, 8) ?? '—'
if (installation.installedReleaseRevision === null || installation.installedAppVersion === null) {
// Why: the current app version says nothing about which release produced
// unknown bytes, so diagnostic copy must not invent provenance.
return `${translate('auto.components.settings.ManagedOrcaSkills.unverified', 'unverified')} · ${digest}`
}
return `r${installation.installedReleaseRevision} · Orca ${installation.installedAppVersion} · ${digest}`
}