Repository navigation
an account knows which workspace it is pinned to - #3
Merged
Merged
Conversation
`Account` gains `workspace`, and `pin(id:to:)` sets it. The host's own pin — which account a workspace uses — moves to the plugin that owns the accounts it names. Pinning is deliberately not part of `setCurrent`. They answer different questions: current is *which account is designated right now*, a pin is *where this account belongs*, and a pin survives the current one changing. Scoping current by workspace instead would leave "which account is current" with no answer until the caller also said where — and that plain question is the one a host asks. Keeping them apart costs one method and keeps both answerable. The relation is carried on the account rather than in a table keyed by workspace, because it is one workspace per account. A host that wants the accounts of one workspace filters a listing it already has, so the reverse lookup needs no second method. `nil` is not pinned, and the framework ships no default. "origin", "main", "default" are all host vocabulary, and a framework that picked one would be taking a side in a naming decision that is not its own. `workspace` crosses as an opaque string. The plugin records it without knowing what a workspace is, the same way it already takes a directory path without knowing what the host keeps in it. On the wire it is an explicit null when absent, like `email` and `plan`: a missing key cannot be told from a reply that lost one, and "pinned nowhere" is a state a host renders differently from "unknown". Verified both wire tests catch it — dropping the field from the encoder fails them immediately.
`Account` shipped as a struct in dev.10. It should not have: the rule for this package is that a foundation type is a protocol, for the reason `AITool` is one — holding only properties today does not mean nothing will ever need behaviour, and a struct forces a breaking change the first time one does. `workspace` belongs on that protocol rather than being bolted to a struct, which is what prompted this. The pattern is the one `AITool` and `ToolDescription` already set. `Account` is the interface, with a defaults extension so the smallest conformer supplies `id` and `name` and nothing else — a test pins that, because a defaults extension nobody exercises stops being real and the protocol then buys nothing. Alongside it, `AccountRecord` is the concrete `Codable` type the protocol deliberately is not: a protocol cannot be `Decodable`, because decoding has to know what to build. `Codable` and `Equatable` move to the record, so `any Account` has neither. That is the cost, and it is the right side of the trade: a forwarding proxy can now conform without carrying values it does not have, and comparisons in tests go through the record. Verified with `swift build -Xswiftc -strict-concurrency=complete -Xswiftc -warnings-as-errors`, which is what this package's concurrency rule asks for. 126 tests pass.
`pin` moves from `AIToolAccounts` to `Account`. It is the account's own relation — one workspace, and the account is what moves — so `pin(id:to:)` on the tool made a caller name an account it was already holding. This is also the protocol earning its keep. An `Account` that only carried properties could have been a struct; behaviour arriving on it is exactly the case a struct would have turned into a breaking change, which is why the rule exists. `AccountRecord` therefore stops conforming to `Account`, matching `ToolDescription`, which has never conformed to `AITool`. An account can be pinned and a decoded value has nothing to pin *with* — no store, no connection — so a record that conformed would answer `pin(to:)` by failing. That is a conformance that lies. The types that conform are the ones that can act: a plugin's own account, and a host-side proxy holding the connection back to it. The in-suite pool now models that, holding a view onto its store rather than a detached copy, which is what a real plugin does. `pin(to:)` gets no default implementation. One that did nothing would tell a host the pin was recorded; one that threw would hide the decision from whoever wrote the conformer. The identity fields still default, so the smallest account is two properties and this method. The wire keeps `tool/pin`, since a request has to name which account. The server resolves the id against `list()` and asks that account to pin itself, rather than a second `pin(id:to:)` existing on the tool just to carry the id across. 126 tests pass; `-strict-concurrency=complete -warnings-as-errors` is clean.
`pin(to:)` was the first, and the rest follow it. `setCurrent(id:)` becomes `makeCurrent()`, `deleteAccount(id:)` becomes `delete()`, both on `Account`. The rule is now legible in the shape: anything done *to* an account is on the account, because a caller doing one of those is already holding it, and a `verb(id:)` on the tool asked it to name the thing in its hand. What stays on `AIToolAccounts` is what only the tool can answer. `list()` and `current()` are questions about the tool — which accounts exist, which one is designated — and `current()` in particular can legitimately have no answer, which an account-side method could not express. `addAccount(id:name:)` stays because there is nothing to call a method on until it has been made. A consequence worth having: "no such account" stops being an operation error. You cannot act on an account you are not holding, so failing to find one is a lookup that failed, earlier and somewhere else — in `PluginServer`, which is where the wire's id gets resolved. That resolution is now the single place `noSuchAccount` arises, and the wire tests still cover it for all three operations. The in-suite pool gained the methods an account calls back into, which is what a real plugin's storage does. Its account type is a view onto that storage; a detached value could not answer any of these. 126 tests pass; `-strict-concurrency=complete -warnings-as-errors` is clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Accountgainsworkspace, andpin(id:to:)sets it. The host's own pin — which account a workspace uses — moves to the plugin that owns the accounts it names.Why pinning is not part of
setCurrentThey answer different questions:
currentA pin survives the current one changing, and a test says so. Scoping current by workspace instead —
current(in:)— would leave "which account is current" with no answer until the caller also said where, and that plain question is exactly what a host asks. Keeping them apart costs one method and leaves both answerable.Why the account carries it, not a table
It is one workspace per account, so the account is where the relation belongs. A host that wants the accounts of one workspace filters a listing it already has:
So the reverse lookup needs no second method — this lands at six methods rather than seven.
nilmeans not pinned, and there is no default"origin", "main", "default" are all host vocabulary. A framework that shipped one would be taking a side in a naming decision that is not its own.
workspacecrosses as an opaque string: the plugin records it without knowing what a workspace is, the same way it already accepts a directory path without knowing what the host keeps in it.On the wire
An explicit
nullwhen absent, likeemailandplan. A missing key cannot be told apart from a reply that lost one, and "pinned nowhere" is a state a host renders differently from "unknown".Accountis now a protocolIt shipped as a struct in dev.10 and should not have. The rule for this package is that a foundation type is a protocol, for the reason
AIToolis one: holding only properties today does not mean nothing will ever need behaviour, and a struct forces a breaking change the first time one does. Puttingworkspaceon it is what surfaced the miss.The pattern is the one
AIToolandToolDescriptionalready set:The smallest conformer supplies
idandname. A test pins that, because a defaults extension nobody exercises stops being real and the protocol then buys nothing over a struct.AccountRecordis the concreteCodabletype the protocol deliberately is not — a protocol cannot beDecodable, because decoding has to know what to build.CodableandEquatablelive on the record, soany Accounthas neither. That is the cost, and it is the right side of the trade: a forwarding proxy can conform without carrying values it does not have.Operations live on
Accountpin(to:)came first, and the rest follow it:setCurrent(id:)becomesmakeCurrent(),deleteAccount(id:)becomesdelete().The rule is legible in the shape now — anything done to an account is on the account, because a caller doing one of those is already holding it, and a
verb(id:)on the tool asked it to name the thing in its hand.What stays on the tool is what only the tool can answer.
list()andcurrent()are questions about the tool, andcurrent()can legitimately have no answer, which an account-side method could not express.addAccountstays because there is nothing to call a method on until it has been made."No such account" stops being an operation error. You cannot act on an account you are not holding, so failing to find one is a lookup that failed, earlier and somewhere else — in
PluginServer, resolving the wire's id. That is now the single place it arises, and the wire tests still cover it for all three operations.This is the protocol earning its keep. An
Accountthat only carried properties could have been a struct; behaviour arriving on it is exactly the case a struct would have turned into a breaking change.AccountRecordtherefore stops conforming toAccount, matchingToolDescription, which has never conformed toAITool. A decoded value has nothing to pin with — no store, no connection — so a record that conformed would answerpin(to:)by failing, which is a conformance that lies. The types that conform are the ones that can act: a plugin's own account, and a host-side proxy holding the connection back to it. The in-suite pool now models that, holding a view onto its store rather than a detached copy.pin(to:)gets no default. One that did nothing would tell a host the pin was recorded; one that threw would hide the decision from whoever wrote the conformer. Identity fields still default, so the smallest conformer is two properties and this method.The wire keeps
tool/pin, since a request has to name an account. The server resolves the id againstlist()and asks that account to pin itself.Verification
126 tests pass, and
swift build -Xswiftc -strict-concurrency=complete -Xswiftc -warnings-as-errorsis clean. Both wire tests were checked for discrimination — droppingworkspacefrom the encoder fails them immediately, which is the mistake worth guarding against since the field is optional and its absence looks legitimate.Note for the sibling PR
PR #1 adds a test that pins the advertised capability set exactly. When it lands,
tool/pingoes in that list.🤖 Generated with Claude Code