Skip to content

Commit 2f2327b

Browse files
test(workspace): await the last detached link syncs, bound the TUI flush
skill-sync's negative-cache test and the new isApprovedRow tests seeded links without `awaitBackfill`, so the guard could fail them at random. The TUI flush is bounded at 5s; comments state the guard's serial-test assumption and that it reads skill-sync's store key. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0174HUYiaceApqHP7EKMNgnb
1 parent 71eed27 commit 2f2327b

5 files changed

Lines changed: 17 additions & 9 deletions

File tree

‎packages/opencode/src/altimate/workspace/skill-sync.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,8 @@ const REALM_ID = Math.random().toString(36).slice(2, 8)
210210
* (review) */
211211
const STAGING_LEASE_MS = 30 * 60 * 1000
212212

213+
// test/preload.ts reads this store through this key (its skill-sync leak guard), without
214+
// importing this module. Renaming the key or `inFlight` silently disables that guard.
213215
const STORE_KEY = Symbol.for("altimate.workspace.skill-sync.store")
214216

215217
export interface SkippedSkill {

‎packages/opencode/test/altimate/workspace/create-then-rebind.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,9 @@ beforeEach(() => {
120120
afterEach(async () => {
121121
// The TUI create leaves its skill sync detached so the dialog can close at once. Settled here,
122122
// against this file's stub, so it cannot run on into the next test.
123-
await flushPendingSyncs()
123+
// Bounded well inside the test timeout: a sync that never settles is the guard's to name, not a
124+
// 30s stall here.
125+
await flushPendingSyncs(5_000)
124126
globalThis.fetch = ORIGINAL_FETCH
125127
process.exitCode = 0 // Bun ignores `= undefined`; a leaked 1 fails later files
126128
})

‎packages/opencode/test/altimate/workspace/skill-sync.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2383,14 +2383,15 @@ describe("workspace skill sync", () => {
23832383
}) as unknown as typeof fetch
23842384
await syncSkills(project)
23852385

2386-
// The link. Deliberately still inside the miss window.
2386+
// The link. Deliberately still inside the miss window. Awaited: the sync it starts registers
2387+
// only once its dynamic import settles, so the `syncSkills` below may not join it.
23872388
await recordApprovedBinding(project, {
23882389
datamateId: 7,
23892390
datamateName: "ws-7",
23902391
repoRemote: null,
23912392
projectPath: project,
23922393
linkedAt: Date.now(),
2393-
})
2394+
}, { awaitBackfill: true })
23942395
serve({ "pub-1": { "SKILL.md": "one" } })
23952396
await syncSkills(project)
23962397

‎packages/opencode/test/altimate/workspace/state-account-scope.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -191,40 +191,40 @@ describe("isApprovedRow: whether this call's approval was written", () => {
191191
test("the row this call wrote is approved", async () => {
192192
asAccount("key-a")
193193
const row = binding(35, "Growth")
194-
await recordApprovedBinding(ROOT, row, { account: A, seed: false })
194+
await recordApprovedBinding(ROOT, row, { awaitBackfill: true, account: A, seed: false })
195195
expect(isApprovedRow(ROOT, A, 35, row.linkedAt)).toBe(true)
196196
expect(isApprovedRow(ROOT, A, 36, row.linkedAt)).toBe(false)
197197
})
198198

199199
test("a write refused because the account changed first leaves nothing approved", async () => {
200200
asAccount("key-b")
201201
const row = binding(35, "Growth")
202-
expect((await recordApprovedBinding(ROOT, row, { account: A }))?.status).toBe("account-changed")
202+
expect((await recordApprovedBinding(ROOT, row, { awaitBackfill: true, account: A }))?.status).toBe("account-changed")
203203
expect(isApprovedRow(ROOT, A, 35, row.linkedAt)).toBe(false)
204204
})
205205

206206
test("an approval recorded earlier does not vouch for a later write that was refused", async () => {
207207
asAccount("key-a")
208-
await recordApprovedBinding(ROOT, { ...binding(35, "Growth"), linkedAt: Date.now() - 60_000 }, { account: A, seed: false })
208+
await recordApprovedBinding(ROOT, { ...binding(35, "Growth"), linkedAt: Date.now() - 60_000 }, { awaitBackfill: true, account: A, seed: false })
209209
asAccount("key-b")
210210
const attempt = binding(35, "Growth")
211-
expect((await recordApprovedBinding(ROOT, attempt, { account: A }))?.status).toBe("account-changed")
211+
expect((await recordApprovedBinding(ROOT, attempt, { awaitBackfill: true, account: A }))?.status).toBe("account-changed")
212212
expect(isApprovedRow(ROOT, A, 35, attempt.linkedAt)).toBe(false)
213213
})
214214

215215
test("a row another account wrote is not this account's approval", async () => {
216216
asAccount("key-b")
217217
const B = credentialDigest(API_URL, TENANT, "key-b")
218218
const row = binding(35, "Growth")
219-
await recordApprovedBinding(ROOT, row, { account: B, seed: false })
219+
await recordApprovedBinding(ROOT, row, { awaitBackfill: true, account: B, seed: false })
220220
expect(isApprovedRow(ROOT, B, 35, row.linkedAt)).toBe(true)
221221
expect(isApprovedRow(ROOT, A, 35, row.linkedAt)).toBe(false)
222222
})
223223

224224
test("an adopted row for the same workspace is not an approval", async () => {
225225
asAccount("key-a")
226226
const row = { ...binding(35, "Growth"), adopted: true }
227-
await recordApprovedBinding(ROOT, row, { account: A, seed: false })
227+
await recordApprovedBinding(ROOT, row, { awaitBackfill: true, account: A, seed: false })
228228
expect(isApprovedRow(ROOT, A, 35, row.linkedAt)).toBe(false)
229229
})
230230
})

‎packages/opencode/test/preload.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,9 @@ initProjectors()
140140
// Syncs already running when a test starts are reported as inherited, not as that test's. One
141141
// whose start was deferred past its own test's end still lands on the next test; the message
142142
// says so, since telling them apart would mean tagging each sync with its owner in skill-sync.
143+
// Both rely on tests running one at a time in this process: under same-process `test.concurrent`
144+
// one test could be blamed for another's sync. Today's concurrent tests either spawn CLI
145+
// subprocesses (their own globals) or never reach skill-sync.
143146
type SkillSyncStore = { inFlight?: Map<string, Promise<unknown>> }
144147
const runningSyncs = () => [
145148
...((globalThis as unknown as Record<symbol, SkillSyncStore | undefined>)[

0 commit comments

Comments
 (0)