Skip to content

[REFACTOR] the advertised surface is the implemented surface #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gradyzhuo merged 2 commits into from
Sep 30, 2026
Merged

[REFACTOR] the advertised surface is the implemented surface #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
610d0c7
[REFACTOR] the advertised surface is the implemented surface
gradyzhuo Sep 25, 2026
eeea146
[TEST] pin the account surface too, not only the identity one
gradyzhuo Sep 29, 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
56 changes: 43 additions & 13 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,18 @@ classDiagram
+coloredTag : String
}

class AIToolIdentityReporting {
<<protocol>>
+listIdentities(in configDirs) [LoginIdentity?]
+showIdentity(in configDir) LoginIdentity?
}

class LoginIdentity {
<<struct>>
+email : String?
+plan : String?
}

class AIToolRegistry {
<<final class>>
-storage : Mutex
Expand Down Expand Up @@ -120,6 +132,8 @@ classDiagram
}

AITool --|> Sendable : refines
AIToolIdentityReporting --|> AITool : refines
AIToolIdentityReporting ..> LoginIdentity : returns
AIToolDefaults ..|> AITool : supplies defaults for
AIToolRegistry o-- AITool : holds many
AIToolRegistry ..> RegistrationError : throws
Expand All @@ -131,6 +145,12 @@ must answer. `supportsSetup` and `coloredTag` are derived and are not
requirements at all — a detail with teeth, since extension members do not
dispatch dynamically.

Everything on `AITool` is a fact the tool states about itself.
`AIToolIdentityReporting` is the one thing a host *asks* it to do, and it is a
separate protocol so that absence is something a host can see: a tool a host only
ever lists is a legal `AITool` and does not conform here. A capability whose
answer would have to be a silent no-op does not belong on `AITool`.

## Conforming a tool

`AITool` is a protocol. A conformer answers `id` and `displayName`; everything
Expand Down Expand Up @@ -254,21 +274,27 @@ classDiagram
class AITool {
<<protocol>>
+id : String
+accountInfo(configDir) AccountInfo
}

class AIToolIdentityReporting {
<<protocol>>
+listIdentities(in configDirs) [LoginIdentity?]
+showIdentity(in configDir) LoginIdentity?
}

class LocalTool {
<<struct>>
+id : String
+accountInfo(configDir) AccountInfo
+listIdentities(in configDirs) [LoginIdentity?]
+showIdentity(in configDir) LoginIdentity?
}

class RemoteAITool {
<<struct>>
-connection : JSONRPCConnection
-cached : Description
+id : String
+accountInfo(configDir) AccountInfo
+identityReporting : AIToolIdentityReporting?
}

class JSONRPCConnection {
Expand All @@ -281,15 +307,18 @@ classDiagram
<<process>>
+initialize()
+tool_describe()
+tool_accountInfo()
+tool_listIdentities()
+tool_showIdentity()
}

LocalTool ..|> AITool
AIToolIdentityReporting --|> AITool : refines
LocalTool ..|> AIToolIdentityReporting
RemoteAITool ..|> AITool
RemoteAITool o-- JSONRPCConnection
JSONRPCConnection ..> PluginProcess : JSON-RPC 2.0

note for RemoteAITool "Same protocol as the local struct — a call site holding 'any AITool' cannot tell which one it has."
note for AIToolIdentityReporting "A local tool answers by conforming; a remote one carries an optional, because conformance cannot depend on what a process said at runtime."
```

Two consequences fall out. Built-in and third-party tools take the *same*
Expand Down Expand Up @@ -385,18 +414,19 @@ sequenceDiagram
participant H as Host
participant P as Plugin

H->>P: tool/accountInfo
H->>P: tool/listIdentities
P--)H: timeout
Note over H: read — degrade<br/>list the account, leave email blank

H->>P: tool/credentialCopy
P--)H: timeout
Note over H: write — abort<br/>never mark the account seeded
```

Listing accounts is worth doing with one field missing. Recording a credential
as copied when it may not have been is the failure that looks like success,
and costs someone an account they believe is safe.
Listing accounts is worth doing with one field missing.

The other half of the rule has nothing to apply to yet: every method in the
surface today is a read. It is written down because it decides the shape of the
first write that gets added — recording a credential as copied when it may not
have been is the failure that looks like success, and costs someone an account
they believe is safe. A write must therefore be able to report *attempted and
failed* distinctly from *nothing to do*.

A socket earns its place when the peer outlives the caller or serves several
callers at once. Which caller that is should be established by measuring a
Expand Down
57 changes: 0 additions & 57 deletions Sources/AIToolKit/AIToolStateTransfer.swift
View file
Open in desktop

This file was deleted.

81 changes: 5 additions & 76 deletions Sources/AIToolKit/RPC/PluginServer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ public enum PluginServer {
/// Answers one request line. Returns nil when the line is not a request
/// worth answering — an unparseable line is skipped, never fatal, because
/// stdout carries the protocol and a stray write must not end the session.
/// `async` because the operations below are: a tool asked to copy its login
/// state does real work, and unlike `tool/describe` the answer cannot come
/// from data already in hand.
/// `async` because the identity methods are: unlike `tool/describe`, whose
/// answer is already in hand, a tool asked who a directory belongs to may
/// read files or consult a credential store to find out.
public static func handle(line: Data, tool: any AITool) async -> Data? {
guard let request = try? JSONDecoder().decode(JSONRPCRequest.self, from: line)
else { return nil }
Expand All @@ -26,10 +26,6 @@ public enum PluginServer {
// discovery to the first call — and for these methods, the first
// call is one with side effects.
var capabilities: [String: RPCValue] = ["tool/describe": .bool(true)]
if tool is any AIToolStateTransfer {
capabilities["tool/copyLoginState"] = .bool(true)
capabilities["tool/copyNonLoginSettings"] = .bool(true)
}
if tool is any AIToolIdentityReporting {
capabilities["tool/listIdentities"] = .bool(true)
capabilities["tool/showIdentity"] = .bool(true)
Expand Down Expand Up @@ -71,20 +67,6 @@ public enum PluginServer {
response = await report(request.method, on: reporter,
params: request.params, id: request.id)

case "tool/copyLoginState", "tool/copyNonLoginSettings":
// A tool that does not conform genuinely does not offer these, and
// said so at `initialize`. Method-not-found is the honest answer;
// inventing a no-op success would be the failure this package keeps
// legislating against — work reported as done that never happened.
guard let transfer = tool as? any AIToolStateTransfer else {
response = JSONRPCResponse(
id: request.id, result: nil,
error: .init(code: JSONRPCError.methodNotFoundCode,
message: "Method not found: \(request.method)"))
break
}
response = await perform(request.method, on: transfer, params: request.params, id: request.id)

case "tool/list", "tool/current", "tool/setCurrent",
"tool/addAccount", "tool/deleteAccount", "tool/pin":
guard let accounts = tool as? any AIToolAccounts else {
Expand Down Expand Up @@ -117,8 +99,8 @@ public enum PluginServer {
/// identity, just the wrong one, and nothing downstream can detect it.
///
/// "No login here" is a result, not an error. A tool that *threw* while
/// looking is the error — the same distinction `copyLoginState` draws between
/// nothing-to-copy and a copy that failed.
/// looking is the error. Any operation added here later owes the same
/// distinction: an answer of "nothing" is not a failure to answer.
private static func report(
_ method: String,
on tool: any AIToolIdentityReporting,
Expand Down Expand Up @@ -179,59 +161,6 @@ public enum PluginServer {
}
}

/// Runs one state-transfer operation and turns its outcome into a reply.
///
/// Three outcomes, three shapes, and keeping them apart is the whole job:
/// arguments that do not permit the call are `invalidParams`, a copy that
/// found nothing to do is a *successful* reply carrying `copied: false`, and
/// a copy that was attempted and threw is `operationFailed`. Collapsing the
/// last two would leave a host unable to tell "the source was never logged
/// in" from "the credential may be half-written".
private static func perform(
_ method: String,
on tool: any AIToolStateTransfer,
params: RPCParams?,
id: Int
) async -> JSONRPCResponse {
func invalid(_ message: String) -> JSONRPCResponse {
JSONRPCResponse(id: id, result: nil,
error: .init(code: JSONRPCError.invalidParamsCode, message: message))
}

guard case .string(let targetPath)? = params?["targetDir"], !targetPath.isEmpty else {
return invalid("\(method): targetDir is required")
}
let target = URL(fileURLWithPath: targetPath)

do {
switch method {
case "tool/copyLoginState":
// A `null` sourceDir is a real argument — "your own default
// location" — and a missing key is not the same thing. Both
// arrive as nil here, which is the one place this is lenient:
// the alternative is refusing a request whose intent is clear.
var source: URL?
if case .string(let sourcePath)? = params?["sourceDir"], !sourcePath.isEmpty {
source = URL(fileURLWithPath: sourcePath)
}
let copied = try await tool.copyLoginState(from: source, to: target)
return JSONRPCResponse(id: id, result: .object(["copied": .bool(copied)]), error: nil)

default:
guard case .string(let sourcePath)? = params?["sourceDir"], !sourcePath.isEmpty else {
return invalid("\(method): sourceDir is required")
}
try await tool.copyNonLoginSettings(
from: URL(fileURLWithPath: sourcePath), to: target)
return JSONRPCResponse(id: id, result: .object([:]), error: nil)
}
} catch {
return JSONRPCResponse(
id: id, result: nil,
error: .init(code: JSONRPCError.operationFailedCode,
message: "\(method) failed: \(error)"))
}
}

/// Runs one account operation and turns its outcome into a reply.
///
Expand Down
Loading
Loading