Skip to content

Commit 81490cb

Browse files
sahrizviclaude
andcommitted
feat(workspace): tell the user which workspace skills a sync skipped, and why
A skill that failed to sync was a log line only, so "nothing arrived" looked exactly like "this workspace has no skills". Surface it instead: - `syncSkills` now returns `skipped` (each skill with a short reason) and `error` (set when the whole sync could not proceed: an unusable or foreign folder, unreadable credentials, an unconfirmed workspace, an unreachable skill list, or a failed publish). - The per-turn sync shows a warning toast naming the skipped skills. The same problem is announced once, not on every retry; a clean run clears it. - `/workspace` Refresh folds skipped skills into its "problems" line, and the `serve` refresh report carries `skillsSkipped` through to the IDE. - Reasons are fixed, user-facing strings. The raw error can carry request URLs, server text or local paths; it stays in the log. Offline is reported at the binding lookup, not the list fetch: the binding is re-resolved on the network, so an outage fails that first. Found in e2e — a unit test that bound the project locally reached the list and missed it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 7ed6e16 commit 81490cb

6 files changed

Lines changed: 297 additions & 12 deletions

File tree

‎packages/opencode/src/altimate/workspace/manage.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ export interface RefreshReport {
6262
/** True when the skill snapshot on disk changed. The caller owns the registry
6363
* invalidation this implies; see the note at the top of the file. */
6464
skillsChanged: boolean
65+
/** Skills the re-sync dropped, with the reason for each. A partial sync
66+
* otherwise reads as success with fewer skills than the workspace has. */
67+
skillsSkipped: SkillSync.SkippedSkill[]
6568
/** Absent when workspace memory is off, or when no session was supplied. */
6669
memory?: MemorySync.RefreshResult
6770
/** Set when there was no session to reload in place, so the overlay was
@@ -168,8 +171,12 @@ export async function refresh(directory: string, sessionID?: string): Promise<Re
168171
const errors: string[] = []
169172

170173
let skillsChanged = false
174+
let skillsSkipped: SkillSync.SkippedSkill[] = []
171175
try {
172-
skillsChanged = (await SkillSync.syncSkills(directory)).changed
176+
const result = await SkillSync.syncSkills(directory)
177+
skillsChanged = result.changed
178+
skillsSkipped = result.skipped
179+
if (result.error) errors.push(`skills: ${result.error}`)
173180
} catch (err) {
174181
// `syncSkills` documents that it never throws. Caught anyway: this is the
175182
// user asking for a repair, and the one thing it must not do is fail the turn.
@@ -197,7 +204,7 @@ export async function refresh(directory: string, sessionID?: string): Promise<Re
197204
}
198205
}
199206

200-
return { skillsChanged, memory, memoryInvalidated, errors }
207+
return { skillsChanged, skillsSkipped, memory, memoryInvalidated, errors }
201208
}
202209

203210
/** Push: re-send local memory the workspace never received.

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

Lines changed: 88 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,67 @@ const STAGING_LEASE_MS = 30 * 60 * 1000
139139

140140
const STORE_KEY = Symbol.for("altimate.workspace.skill-sync.store")
141141

142+
export interface SkippedSkill {
143+
skill: string
144+
reason: string
145+
}
146+
147+
export interface SyncResult {
148+
changed: boolean
149+
/** Skills this run dropped, each with a short, user-facing reason. Surfaced
150+
* on the turn so a missing skill has an explanation instead of silence. */
151+
skipped: SkippedSkill[]
152+
/** Set when the whole sync could not proceed (unusable folder, unreadable
153+
* credentials, unreachable list, or a failed publish). Per skill is
154+
* `skipped`; this is the run. */
155+
error?: string
156+
}
157+
158+
/** One toast's worth of what a sync dropped, or null when nothing was.
159+
* Pure — no UI imports — so the wording is testable; the caller shows it. */
160+
export function describeSyncProblems(result: SyncResult): { title: string; message: string } | null {
161+
const n = result.skipped.length
162+
if (n === 0) return result.error ? { title: "Workspace skills not synced", message: result.error } : null
163+
const lines = result.skipped.slice(0, 3).map((s) => `${s.skill}: ${s.reason}`)
164+
if (n > 3) lines.push(`…and ${n - 3} more`)
165+
// A skipped bundle and a failed publish are different news: the second means
166+
// the snapshot as a whole did not change.
167+
if (result.error) lines.push(result.error)
168+
return { title: `${n} workspace skill${n === 1 ? "" : "s"} skipped`, message: lines.join("\n") }
169+
}
170+
171+
/** A fixed, user-facing reason for a skill that failed to sync. The raw error
172+
* can carry request URLs, server text or local paths — diagnostics for the
173+
* log, not for a toast. */
174+
function skipReason(err: unknown): string {
175+
const msg = err instanceof Error ? err.message : ""
176+
if (msg.startsWith("size mismatch")) return "its download was incomplete"
177+
if (msg.startsWith("would exceed the client snapshot limit")) return "it is too large for this client"
178+
if (msg.startsWith("unrecognised")) return "the server sent an unexpected response for it"
179+
return "it could not be downloaded"
180+
}
181+
182+
const announced = new Map<string, string>()
183+
184+
/** Whether a sync problem is new for this directory. The per-turn sync retries
185+
* a failed run every message, so without this an offline user or a
186+
* permanently bad bundle would be told the same thing on every turn — and
187+
* twice when concurrent turns join one run. A clean result clears it, so the
188+
* problem is announced again if it comes back. */
189+
export function shouldAnnounce(directory: string, problem: { title: string; message: string } | null): boolean {
190+
const key = path.resolve(directory)
191+
if (!problem) {
192+
announced.delete(key)
193+
return false
194+
}
195+
const text = `${problem.title}\n${problem.message}`
196+
if (announced.get(key) === text) return false
197+
announced.set(key, text)
198+
return true
199+
}
200+
142201
interface SyncStore {
143-
inFlight: Map<string, Promise<{ changed: boolean }>>
202+
inFlight: Map<string, Promise<SyncResult>>
144203
lastSyncedAt: Map<string, number>
145204
registryAppliedAt: Map<string, number>
146205
syncedFor: Map<string, string>
@@ -710,7 +769,7 @@ async function removeManaged(directory: string): Promise<void> {
710769
* Never throws: skills must not be able to block a bind or a turn. Every
711770
* failure path leaves whatever is already on disk in place, except the
712771
* deliberate purge described below. */
713-
export async function syncSkills(directory: string): Promise<{ changed: boolean }> {
772+
export async function syncSkills(directory: string): Promise<SyncResult> {
714773
const canon = path.resolve(directory)
715774
// Joined BEFORE the flag is read, so the opt-out purge is serialised against
716775
// a sync too. Both paths write the same tree; with the purge outside this
@@ -721,7 +780,7 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
721780
if (existing) {
722781
// Report the joined run's real outcome. Returning a hard-coded `false` is a
723782
// false answer waiting for the next caller to trust it.
724-
return await existing.catch(() => ({ changed: false }))
783+
return await existing.catch(() => ({ changed: false, skipped: [] }))
725784
}
726785
if (!isEnabled()) {
727786
// Opting out has to actually take effect: a snapshot left behind keeps
@@ -732,7 +791,7 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
732791
const dropped = (await pathsAreReal(canon).catch(() => false))
733792
? await deactivate(canon, "the workspace feature is off").catch(() => false)
734793
: false
735-
return { changed: dropped }
794+
return { changed: dropped, skipped: [] }
736795
})()
737796
inFlight.set(canon, purge)
738797
try {
@@ -742,6 +801,9 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
742801
}
743802
}
744803
let changed = false
804+
// Named apart from the loop's `skipped` counter below, which it would shadow.
805+
const skippedSkills: SkippedSkill[] = []
806+
let syncError: string | undefined
745807
/** The manifest an unchanged run validated, so its marker describes that
746808
* snapshot rather than whatever is live when the stamp is written. */
747809
let validated: Manifest | null = null
@@ -772,6 +834,7 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
772834
path: managedRoot(canon),
773835
})
774836
failed = true
837+
syncError = "the workspace skill folder is not a real directory"
775838
return
776839
}
777840

@@ -806,6 +869,12 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
806869
if (outcome.status === "unbound") {
807870
if (await deactivate(canon, "this project is no longer bound to a workspace")) changed = true
808871
}
872+
// Offline lands here, before the list is ever fetched: the binding is
873+
// re-resolved on the network, so an outage fails this lookup first. So
874+
// does a workspace this account cannot see, hence the wording.
875+
// Unbound is a state, not a failure, and says nothing.
876+
if (outcome.status === "unknown")
877+
syncError = "could not confirm this project's workspace (offline, or no access to it)"
809878
return
810879
}
811880
const binding = outcome.binding
@@ -818,6 +887,7 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
818887
"refusing to manage the workspace skill directory: it has contents this client did not write",
819888
{ path: managedRoot(canon) },
820889
)
890+
syncError = "the workspace skill folder has files this app did not create, so it was left alone"
821891
return
822892
}
823893
await sweepStaging(canon)
@@ -829,6 +899,7 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
829899
log.warn("could not read altimate credentials; keeping the existing snapshot", {
830900
err: String(err),
831901
})
902+
syncError = "could not read your Altimate credentials"
832903
return
833904
}
834905

@@ -854,7 +925,11 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
854925
}
855926

856927
const remote = await listAll(binding)
857-
if (!remote) return // error, not empty — keep what is on disk
928+
if (!remote) {
929+
// error, not empty — keep what is on disk
930+
syncError = "could not fetch the workspace's skill list"
931+
return
932+
}
858933
sawRemote = true
859934
syncedFor.set(canon, accountKeyOf(creds.altimateInstanceName, creds.altimateUrl))
860935

@@ -920,6 +995,7 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
920995
skipped += 1
921996
failed = true
922997
log.warn("skipping a workspace skill with an unusable id", { skill: summary.publicId })
998+
skippedSkills.push({ skill: summary.publicId, reason: "its id is not usable as a folder name" })
923999
continue
9241000
}
9251001
try {
@@ -1007,6 +1083,8 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
10071083
carriedPrevious: carried,
10081084
err: String(err),
10091085
})
1086+
const why = skipReason(err)
1087+
skippedSkills.push({ skill: summary.publicId, reason: carried ? `${why} (kept the previous copy)` : why })
10101088
}
10111089
}
10121090
if (remote.length > 0 && Object.keys(next.skills).length === 0) {
@@ -1068,6 +1146,9 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
10681146
failed = true
10691147
await fs.rm(staging, { recursive: true, force: true }).catch(() => {})
10701148
log.warn("workspace skill sync failed; kept the existing snapshot", { err: String(err) })
1149+
// Per-skill reasons are already in `skippedSkills`; this says the snapshot
1150+
// as a whole did not change.
1151+
syncError ??= "could not update the workspace skills; kept the previous ones"
10711152
}
10721153
})()
10731154
// Published to `inFlight` so a joining caller awaits the SAME settled result
@@ -1079,6 +1160,7 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
10791160
} catch (err) {
10801161
ok = false
10811162
log.warn("workspace skill sync errored", { err: String(err) })
1163+
syncError ??= "workspace skill sync errored"
10821164
}
10831165
// Only a clean run earns the poll interval. `failed` is set by the inner
10841166
// catch, which swallows so that skills can never block a turn.
@@ -1098,7 +1180,7 @@ export async function syncSkills(directory: string): Promise<{ changed: boolean
10981180
if (!changed && validated)
10991181
await fs.writeFile(path.join(managedRoot(canon), SYNCED_MARKER), markerFor(validated, now)).catch(() => {})
11001182
}
1101-
return { changed }
1183+
return { changed, skipped: skippedSkills, error: syncError }
11021184
})()
11031185
inFlight.set(canon, settled)
11041186
try {

‎packages/opencode/src/plugin/tui/altimate/workspace.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1856,18 +1856,22 @@ async function runWorkspaceManage(api: TuiPluginApi, directory: string): Promise
18561856
if (option.value === "refresh") {
18571857
Manage.refresh(directory)
18581858
.then((result) => {
1859+
// Named, with reasons: a partial pull otherwise reads as success
1860+
// with fewer skills than the workspace has.
1861+
const skipped = result.skillsSkipped.map((s) => `${s.skill}: ${s.reason}`)
1862+
const problems = skipped.length > 0 ? [...result.errors, `skipped ${skipped.join("; ")}`] : result.errors
18591863
const said = [
18601864
result.skillsChanged ? "skills updated" : "skills already current",
18611865
result.memoryInvalidated ? "memory reloads on your next message" : null,
18621866
].filter(Boolean)
18631867
api.ui.toast({
1864-
variant: result.errors.length > 0 ? "warning" : "success",
1868+
variant: problems.length > 0 ? "warning" : "success",
18651869
// The problems line still names what DID land: the halves are
18661870
// independent, and a failed skill pull does not undo the memory
18671871
// invalidation that happened beside it.
18681872
message:
1869-
result.errors.length > 0
1870-
? `Refreshed with problems — ${result.errors.join("; ")}${
1873+
problems.length > 0
1874+
? `Refreshed with problems — ${problems.join("; ")}${
18711875
result.memoryInvalidated ? "; memory reloads on your next message" : ""
18721876
}`
18731877
: `Refreshed: ${said.join(", ")}.`,

‎packages/opencode/src/session/prompt.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,19 @@ export namespace SessionPrompt {
397397
await refreshRegistry()
398398

399399
if (!(await skillSync.recentlySynced(dir))) {
400-
const applied = skillSync.syncSkills(dir).then(refreshRegistry)
400+
const applied = skillSync.syncSkills(dir).then(async (result) => {
401+
await refreshRegistry()
402+
// A skill that silently fails to arrive looks exactly like a
403+
// workspace with no skills. Say which, and why. Imported only when
404+
// there is something to show, keeping the common path free of it.
405+
const problem = skillSync.describeSyncProblems(result)
406+
// Called on a clean result too: that is what clears the memo.
407+
const announce = skillSync.shouldAnnounce(dir, problem)
408+
if (problem && announce) {
409+
const { notify } = await import("../altimate/workspace/engine-probes")
410+
await notify({ ...problem, variant: "warning" })
411+
}
412+
})
401413
applied.catch((err) => log.warn("workspace skill sync failed", { err: String(err) }))
402414
// Timer cleared when the sync wins the race: an armed timer keeps the
403415
// event loop alive, so a short-lived `run` would linger for the rest

0 commit comments

Comments
 (0)