Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
b87577d
fix(workspace): name the bound workspace in the system prompt (#1269)
sahrizvi Sep 8, 2026
e0e8ed2
feat(workspace): status, refresh, sync and unlink operations (#1270, …
sahrizvi Sep 8, 2026
ae80f16
feat(workspace): /workspace action menu (#1270, #1272, #1273)
sahrizvi Sep 8, 2026
02be1fd
fix(workspace): count unsynced memory against the workspace's own set…
sahrizvi Sep 8, 2026
d3382e0
fix(workspace): guard the unlink purge against a symlinked project dir
sahrizvi Sep 9, 2026
116a03c
fix(workspace): three more unlink defects from the cubic review on #1278
sahrizvi Sep 9, 2026
e3f3237
fix(workspace): make the status line and the sweep agree about gating
sahrizvi Sep 9, 2026
23b4760
fix(workspace): the six finishing defects from review
sahrizvi Sep 14, 2026
586cc54
fix(workspace): the fourth review batch — unlink races, gate reasons,…
sahrizvi Sep 14, 2026
5571fe3
fix(workspace): finish the relink guard, and keep the name util realm…
sahrizvi Sep 14, 2026
3d1cab7
fix(workspace): confirm a kept relink with the server, and judge the …
sahrizvi Sep 14, 2026
66273f7
fix(workspace): the post-unlink server check asks by the relinked row…
sahrizvi Sep 14, 2026
a45b2ed
fix(workspace): a relink the second cleanup keeps is left whole, snap…
sahrizvi Sep 14, 2026
1701078
fix(workspace): status stays off the network with a cached row; unrem…
sahrizvi Sep 15, 2026
3c563e1
fix(workspace): a cache that already belonged to another account is n…
sahrizvi Sep 15, 2026
976a874
fix(workspace): take the unscoped-row snapshot before unlink's first …
sahrizvi Sep 15, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions packages/opencode/src/altimate/workspace/api-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,34 @@ export namespace WorkspaceApi {
return null
}

/** Detach this project from its workspace, server-side.
*
* Returns false when the server had no active binding to remove — the project
* was already unlinked, by someone else or on another machine. That is a
* distinct outcome from "removed", not an error, so the caller can tell the
* user which happened.
*
* A local-only unlink is not possible: ``lookupBinding`` re-asks the server
* whenever the cache misses, so a row dropped only on disk comes straight back
* on the next resolve. */
export async function unbindProject(id: ProjectIdentifier): Promise<boolean> {
const query: Record<string, string> = {}
// Send exactly one identifier. The endpoint answers 409 when both are given
// and they name different bindings, and preferring the remote matches how
// ``getBindingForProject`` resolves — so unlink removes the binding that
// lookup would have found.
if (id.repoRemote) query.repo_remote = id.repoRemote
else if (id.projectPath) query.project_path = id.projectPath
else return false
try {
await req<unknown>("DELETE", "/", { query, allowEmptyBody: true })
return true
} catch (err) {
if (err instanceof NotFoundError) return false

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When DELETE returns 404, this reports success instead of an error. manage.unlink then clears local state, so a stale or non-normalized identifier can leave the server binding intact and silently re-adopt it later; propagate 404 and retain local state.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/altimate/workspace/api-client.ts, line 378:

<comment>When DELETE returns 404, this reports success instead of an error. `manage.unlink` then clears local state, so a stale or non-normalized identifier can leave the server binding intact and silently re-adopt it later; propagate 404 and retain local state.</comment>

<file context>
@@ -352,6 +352,34 @@ export namespace WorkspaceApi {
+      await req<unknown>("DELETE", "/", { query, allowEmptyBody: true })
+      return true
+    } catch (err) {
+      if (err instanceof NotFoundError) return false
+      throw err
+    }
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By design, and documented on unbindProject and Manage.unlink: a 404 means the server has no active binding under that identifier, which is exactly when a stale local row most needs clearing. The risk you describe — a wrong identifier producing a 404 that clears a live binding — is closed on the other side: unlink deletes on the identifier the row was recorded with, and when there is no row it asks the server which arm it matched on first (116a03c8c2). As of 586cc54987 that pre-check no longer swallows transport errors either, so the only way to reach the 404 branch is with the identifier the server itself resolved. The TUI reports the two outcomes differently ("Unlinked from X" vs "already unlinked").

throw err
Comment on lines +377 to +379

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve 404 as the non-idempotent unlink result

When DELETE returns 404—most simply on the claimed second unlink—this converts the response into a normal false result, after which Manage.unlink clears local state and the TUI reports success. Consequently unlink is idempotent at this API boundary and local cleanup is not limited to a 204, contrary to C3; the 404 needs to propagate rather than be normalized here.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By design, and the PR text is the part that was wrong, not the code: C3 should read "local cleanup runs on 204 and 404, never on an error". A 404 means the server has no active binding under the identifier unlink resolved — the identifier the row was recorded with, or the arm the server itself said it matched on (116a03c8c2; the pre-check no longer swallows transport errors as of 586cc54987) — which is precisely when a stale local row most needs clearing. The API boundary reports the two outcomes distinctly (removedServerSide), and the TUI says "already unlinked" for the 404. The second-unlink 404 in the description is the server's answer, which this client surfaces rather than hides.

}
}

export async function createAndBind(input: {
name: string
identifier: ProjectIdentifier
Expand Down
75 changes: 71 additions & 4 deletions packages/opencode/src/altimate/workspace/awareness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export const MAX_SECTION_CHARS = 2_000

const HEADING = "## Workspace integrations"

const BINDING_HEADING = "## Workspace"

/** How each capability is named to the model. Keyed on the `Capability` union, so a
* new capability is a compile error here rather than an unlabelled row. */
const CAPABILITY_LABEL: Record<Capability, string> = {
Expand Down Expand Up @@ -111,6 +113,52 @@ const DISABLED_COPY: Record<NonNullable<Precedence["disabledReason"]>, string> =
"nothing-materialised": "",
}

/** Whether the workspace may be NAMED in this state. Separate from `DISABLED_COPY`
* because identity and routing are different claims: the routing directive stays
* silent unless there is something to steer, but "which workspace is this project
* linked to" is a question the model is asked directly and could not previously
* answer — nothing else puts the binding in the prompt, and no tool reports it.
*
* Keyed on the union so a new `disabledReason` is a compile error here rather than
* silently naming — or silently failing to name — a workspace. `false` for the three
* unverified states for the reason `UNVERIFIED_SECTION` gives: nothing has confirmed
* the binding those states were derived from, and under `unattributed` the engine may
* belong to a different workspace than the one the link names. `false` for the hatch
* and `pilot-off` because neither carries a name to print (see `EMPTY` in
* `precedence.ts`) — a bound project with the hatch on therefore stays unnamed, which
* is a data limitation of that call site, not a decision made here.
*
* NOTE: this deliberately breaks the "byte-identical system prompt" property that
* `DISABLED_COPY` claims for `nothing-materialised`. A project bound to a workspace
* that materialised no integrations is exactly the case users hit — a freshly created
* workspace — and it is the case where being told nothing is most confusing. */
const NAMES_BINDING: Record<NonNullable<Precedence["disabledReason"]>, boolean> = {
"pilot-off": false,
"escape-hatch": false,
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
unbound: false,
"binding-unreadable": false,
unattributed: false,
"derive-failed": false,
"nothing-materialised": true,
}

/** The identity line: what this project is linked to, independent of whether anything
* is being routed. Empty when the state may not name a binding, or when the snapshot
* carries no name to print. */
function bindingSection(precedence: Precedence): string {
const nameable = precedence.enabled || (precedence.disabledReason ? NAMES_BINDING[precedence.disabledReason] : false)
if (!nameable) return ""
// A name that sanitises to nothing must not erase the identity when the id
// is known: the line is the only place the binding is stated. Without an id
// either there is nothing left to print.
if (!inertWorkspaceName(precedence.workspaceName) && !precedence.workspaceId) return ""
return [
BINDING_HEADING,
"",
`This project is linked to Altimate workspace ${workspaceLabel(precedence.workspaceName, precedence.workspaceId)}.`,
Comment thread
sahrizvi marked this conversation as resolved.
].join("\n")
}

/**
* Render the section, or "" when there is nothing to steer.
*
Expand All @@ -130,6 +178,20 @@ const DISABLED_COPY: Record<NonNullable<Precedence["disabledReason"]>, string> =
*/
export function systemSection(precedence: Precedence | undefined): string {
if (!precedence) return ""
// Identity first, then routing. Either half can be empty; both empty renders "".
// The identity line is charged against MAX_SECTION_CHARS rather than added on top:
// the cap exists to bound what this module injects, so letting a new part sit
// outside it would raise the real ceiling silently.
const binding = bindingSection(precedence)
const routing = routingSection(precedence, binding ? binding.length + SEPARATOR.length : 0)
return [binding, routing].filter(Boolean).join(SEPARATOR)
}

const SEPARATOR = "\n\n"

/** The routing directive. Unchanged contract: silent unless the workspace is really
* routing, so the model is never steered toward tools it should not use. */
function routingSection(precedence: Precedence, reserved = 0): string {
if (!precedence.enabled) return precedence.disabledReason ? DISABLED_COPY[precedence.disabledReason] : ""

const served = servedInventory(precedence)
Expand All @@ -147,7 +209,7 @@ export function systemSection(precedence: Precedence | undefined): string {
return `- ${type} — ${servedPart}${localPart}`
})

return assemble(precedence.workspaceName, precedence.workspaceId, typeLines)
return assemble(precedence.workspaceName, precedence.workspaceId, typeLines, reserved)
}

/** The workspace name is customer-authored and lands in the system prompt — the
Expand All @@ -157,7 +219,7 @@ export function systemSection(precedence: Precedence | undefined): string {
* is named alongside as the stable identifier. Re-applying the sanitiser costs
* nothing and keeps this surface safe even for a snapshot built elsewhere. */
function workspaceLabel(name: string, id: string | undefined): string {
const bounded = inertWorkspaceName(name)
const bounded = inertWorkspaceName(name) || "(unnamed)"
return id ? `${JSON.stringify(bounded)} (id ${id})` : JSON.stringify(bounded)
}

Expand All @@ -171,7 +233,12 @@ function workspaceLabel(name: string, id: string | undefined): string {
* be partial instead, and the prohibition is kept only for types the workspace does
* not serve. The count is stated once, on the list where it belongs; the converse
* carries only what the model should DO about the omission. */
function assemble(workspaceName: string, workspaceId: string | undefined, typeLines: string[]): string {
function assemble(
workspaceName: string,
workspaceId: string | undefined,
typeLines: string[],
reserved = 0,
): string {
const label = workspaceLabel(workspaceName, workspaceId)
const render = (lines: string[]) => {
const omitted = typeLines.length - lines.length
Expand Down Expand Up @@ -200,7 +267,7 @@ function assemble(workspaceName: string, workspaceId: string | undefined, typeLi

let lines = typeLines
let out = render(lines)
while (out.length > MAX_SECTION_CHARS && lines.length > 0) {
while (out.length + reserved > MAX_SECTION_CHARS && lines.length > 0) {
lines = lines.slice(0, -1)
out = render(lines)
}
Expand Down
Loading
Loading