Skip to content

Commit beb2709

Browse files
t3dotggclaude
andauthored
feat(server): agents can now open the images you paste into chat (pingdotgg#5757)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent c95e3c7 commit beb2709

4 files changed

Lines changed: 113 additions & 7 deletions

File tree

‎apps/server/integration/providerService.integration.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
ProviderService,
2424
type ProviderServiceShape,
2525
} from "../src/provider/Services/ProviderService.ts";
26+
import * as ServerConfig from "../src/config.ts";
2627
import { ServerSettingsService } from "../src/serverSettings.ts";
2728
import { AnalyticsService } from "../src/telemetry/Services/AnalyticsService.ts";
2829
import { SqlitePersistenceMemory } from "../src/persistence/Layers/Sqlite.ts";
@@ -93,6 +94,7 @@ const makeIntegrationFixture = (options?: { readonly analytics?: Layer.Layer<Ana
9394
const shared = Layer.mergeAll(
9495
directoryLayer,
9596
Layer.succeed(ProviderAdapterRegistry, registry),
97+
ServerConfig.layerTest(cwd, cwd).pipe(Layer.provide(NodeServices.layer)),
9698
ServerSettingsService.layerTest(DEFAULT_SERVER_SETTINGS),
9799
options?.analytics ?? AnalyticsService.layerTest,
98100
Layer.succeed(ProviderEventLoggers, NoOpProviderEventLoggers),

‎apps/server/src/provider/Layers/ClaudeAdapter.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4090,6 +4090,14 @@ export const makeClaudeAdapter = Effect.fn("makeClaudeAdapter")(function* (
40904090
...(ultracode ? { ultracode: true } : {}),
40914091
};
40924092
const mcpSession = McpProviderSession.readMcpProviderSession(input.threadId);
4093+
// The attachments dir grant lets the agent Read/copy pasted images at
4094+
// the paths ProviderService injects into the turn text, without an
4095+
// approval prompt. It is a leaf directory holding only attachment
4096+
// files; siblings like secrets/ and state.sqlite stay ungranted.
4097+
const additionalDirectories = [
4098+
...(input.cwd ? [input.cwd] : []),
4099+
serverConfig.attachmentsDir,
4100+
];
40934101
const queryOptions: ClaudeQueryOptions = {
40944102
...(input.cwd ? { cwd: input.cwd } : {}),
40954103
...(apiModelId ? { model: apiModelId } : {}),
@@ -4113,7 +4121,7 @@ export const makeClaudeAdapter = Effect.fn("makeClaudeAdapter")(function* (
41134121
includePartialMessages: true,
41144122
canUseTool,
41154123
env: claudeEnvironment,
4116-
...(input.cwd ? { additionalDirectories: [input.cwd] } : {}),
4124+
additionalDirectories,
41174125
...(Object.keys(extraArgs).length > 0 ? { extraArgs } : {}),
41184126
...(mcpSession
41194127
? {
@@ -4148,7 +4156,7 @@ export const makeClaudeAdapter = Effect.fn("makeClaudeAdapter")(function* (
41484156
"claude.query.resume": existingResumeSessionId ?? "",
41494157
"claude.query.session_id": newSessionId ?? "",
41504158
"claude.query.include_partial_messages": true,
4151-
"claude.query.additional_directories": input.cwd ? [input.cwd] : [],
4159+
"claude.query.additional_directories": additionalDirectories,
41524160
"claude.query.setting_sources": [...CLAUDE_SETTING_SOURCES],
41534161
"claude.query.settings_json": encodeJsonStringForDiagnostics(settings) ?? "",
41544162
"claude.query.extra_args_json": encodeJsonStringForDiagnostics(extraArgs) ?? "",

‎apps/server/src/provider/Layers/ProviderService.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,15 @@ import {
5555
makeSqlitePersistenceLive,
5656
SqlitePersistenceMemory,
5757
} from "../../persistence/Layers/Sqlite.ts";
58+
import * as ServerConfig from "../../config.ts";
5859
import * as ServerSettings from "../../serverSettings.ts";
5960
import * as AnalyticsService from "../../telemetry/AnalyticsService.ts";
6061
import { makeAdapterRegistryMock } from "../testUtils/providerAdapterRegistryMock.ts";
6162

6263
const defaultServerSettingsLayer = ServerSettings.ServerSettingsService.layerTest();
64+
const serverConfigTestLayer = ServerConfig.layerTest(process.cwd(), process.cwd()).pipe(
65+
Layer.provide(NodeServices.layer),
66+
);
6367

6468
const asRequestId = (value: string): ApprovalRequestId => ApprovalRequestId.make(value);
6569
const asEventId = (value: string): EventId => EventId.make(value);
@@ -292,6 +296,7 @@ function makeProviderServiceLayer() {
292296
Layer.provide(providerAdapterLayer),
293297
Layer.provide(directoryLayer),
294298
Layer.provide(defaultServerSettingsLayer),
299+
Layer.provide(serverConfigTestLayer),
295300
Layer.provideMerge(AnalyticsService.layerTest),
296301
Layer.provide(
297302
Layer.succeed(
@@ -343,6 +348,7 @@ it.effect("ProviderServiceLive catches stopAll failures during shutdown", () =>
343348
Layer.provide(providerAdapterLayer),
344349
Layer.provide(directoryLayer),
345350
Layer.provide(defaultServerSettingsLayer),
351+
Layer.provide(serverConfigTestLayer),
346352
Layer.provideMerge(AnalyticsService.layerTest),
347353
Layer.provide(
348354
Layer.succeed(
@@ -402,6 +408,7 @@ it.effect("ProviderServiceLive rejects new sessions for disabled providers", ()
402408
Layer.provide(providerAdapterLayer),
403409
Layer.provide(directoryLayer),
404410
Layer.provide(defaultServerSettingsLayer),
411+
Layer.provide(serverConfigTestLayer),
405412
Layer.provide(AnalyticsService.layerTest),
406413
Layer.provide(
407414
Layer.succeed(
@@ -486,6 +493,7 @@ it.effect(
486493
Layer.provide(providerAdapterLayer),
487494
Layer.provide(directoryLayer),
488495
Layer.provide(serverSettingsLayer),
496+
Layer.provide(serverConfigTestLayer),
489497
Layer.provide(AnalyticsService.layerTest),
490498
Layer.provide(
491499
Layer.succeed(
@@ -556,6 +564,7 @@ it.effect("ProviderServiceLive rejects new sessions for disabled custom instance
556564
Layer.provide(providerAdapterLayer),
557565
Layer.provide(directoryLayer),
558566
Layer.provide(defaultServerSettingsLayer),
567+
Layer.provide(serverConfigTestLayer),
559568
Layer.provide(AnalyticsService.layerTest),
560569
Layer.provide(
561570
Layer.succeed(
@@ -611,6 +620,7 @@ it.effect("ProviderServiceLive writes canonical events to the emitting thread se
611620
Layer.provide(Layer.succeed(ProviderAdapterRegistry.ProviderAdapterRegistry, registry)),
612621
Layer.provide(directoryLayer),
613622
Layer.provide(defaultServerSettingsLayer),
623+
Layer.provide(serverConfigTestLayer),
614624
Layer.provide(AnalyticsService.layerTest),
615625
Layer.provide(
616626
Layer.succeed(
@@ -671,6 +681,7 @@ it.effect("ProviderServiceLive keeps persisted resumable sessions on startup", (
671681
Layer.provide(Layer.succeed(ProviderAdapterRegistry.ProviderAdapterRegistry, registry)),
672682
Layer.provide(directoryLayer),
673683
Layer.provide(defaultServerSettingsLayer),
684+
Layer.provide(serverConfigTestLayer),
674685
Layer.provide(AnalyticsService.layerTest),
675686
Layer.provide(
676687
Layer.succeed(
@@ -737,6 +748,7 @@ it.effect(
737748
),
738749
Layer.provide(firstDirectoryLayer),
739750
Layer.provide(defaultServerSettingsLayer),
751+
Layer.provide(serverConfigTestLayer),
740752
Layer.provide(AnalyticsService.layerTest),
741753
Layer.provide(
742754
Layer.succeed(
@@ -796,6 +808,7 @@ it.effect(
796808
),
797809
Layer.provide(secondDirectoryLayer),
798810
Layer.provide(defaultServerSettingsLayer),
811+
Layer.provide(serverConfigTestLayer),
799812
Layer.provide(AnalyticsService.layerTest),
800813
Layer.provide(
801814
Layer.succeed(
@@ -927,6 +940,54 @@ routing.layer("ProviderServiceLive routing", (it) => {
927940
}),
928941
);
929942

943+
it.effect("appends attachment file paths to the turn input text", () =>
944+
Effect.gen(function* () {
945+
const provider = yield* ProviderService.ProviderService;
946+
947+
const session = yield* provider.startSession(asThreadId("thread-attach"), {
948+
provider: ProviderDriverKind.make("codex"),
949+
providerInstanceId: codexInstanceId,
950+
threadId: asThreadId("thread-attach"),
951+
cwd: "/tmp/project",
952+
runtimeMode: "full-access",
953+
});
954+
955+
const attachment = {
956+
type: "image" as const,
957+
id: "thread-attach-12345678-1234-1234-1234-123456789abc",
958+
name: "screenshot.png",
959+
mimeType: "image/png",
960+
sizeBytes: 123,
961+
};
962+
963+
routing.codex.sendTurn.mockClear();
964+
yield* provider.sendTurn({
965+
threadId: session.threadId,
966+
input: "use this screenshot",
967+
attachments: [attachment],
968+
});
969+
970+
const turnInput = routing.codex.sendTurn.mock.calls[0]?.[0] as ProviderSendTurnInput;
971+
assert.equal(typeof turnInput.input, "string");
972+
const turnText = turnInput.input ?? "";
973+
assert.equal(turnText.startsWith("use this screenshot"), true);
974+
assert.include(turnText, '[Attached image "screenshot.png" is saved at: ');
975+
assert.equal(turnText.endsWith(`${attachment.id}.png]`), true);
976+
977+
// An attachment-only turn stays valid and the injected line becomes the
978+
// whole input text, so the agent still learns the path.
979+
routing.codex.sendTurn.mockClear();
980+
yield* provider.sendTurn({
981+
threadId: session.threadId,
982+
attachments: [attachment],
983+
});
984+
const imageOnlyInput = routing.codex.sendTurn.mock.calls[0]?.[0] as ProviderSendTurnInput;
985+
assert.equal(imageOnlyInput.input?.startsWith('[Attached image "screenshot.png"'), true);
986+
987+
yield* provider.stopSession({ threadId: session.threadId });
988+
}),
989+
);
990+
930991
it.effect("recovers stale persisted sessions for rollback by resuming thread identity", () =>
931992
Effect.gen(function* () {
932993
const provider = yield* ProviderService.ProviderService;
@@ -1307,6 +1368,7 @@ routing.layer("ProviderServiceLive routing", (it) => {
13071368
),
13081369
Layer.provide(firstDirectoryLayer),
13091370
Layer.provide(defaultServerSettingsLayer),
1371+
Layer.provide(serverConfigTestLayer),
13101372
Layer.provide(AnalyticsService.layerTest),
13111373
Layer.provide(
13121374
Layer.succeed(
@@ -1345,6 +1407,7 @@ routing.layer("ProviderServiceLive routing", (it) => {
13451407
),
13461408
Layer.provide(secondDirectoryLayer),
13471409
Layer.provide(defaultServerSettingsLayer),
1410+
Layer.provide(serverConfigTestLayer),
13481411
Layer.provide(AnalyticsService.layerTest),
13491412
Layer.provide(
13501413
Layer.succeed(
@@ -1413,6 +1476,7 @@ routing.layer("ProviderServiceLive routing", (it) => {
14131476
),
14141477
Layer.provide(firstDirectoryLayer),
14151478
Layer.provide(defaultServerSettingsLayer),
1479+
Layer.provide(serverConfigTestLayer),
14161480
Layer.provide(AnalyticsService.layerTest),
14171481
Layer.provide(
14181482
Layer.succeed(
@@ -1446,6 +1510,7 @@ routing.layer("ProviderServiceLive routing", (it) => {
14461510
),
14471511
Layer.provide(secondDirectoryLayer),
14481512
Layer.provide(defaultServerSettingsLayer),
1513+
Layer.provide(serverConfigTestLayer),
14491514
Layer.provide(AnalyticsService.layerTest),
14501515
Layer.provide(
14511516
Layer.succeed(

‎apps/server/src/provider/Layers/ProviderService.ts‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ import * as Schema from "effect/Schema";
3535
import * as SchemaIssue from "effect/SchemaIssue";
3636
import * as Stream from "effect/Stream";
3737

38+
import { resolveAttachmentPath } from "../../attachmentStore.ts";
39+
import * as ServerConfig from "../../config.ts";
3840
import {
3941
increment,
4042
providerMetricAttributes,
@@ -203,6 +205,7 @@ const makeProviderService = Effect.fn("makeProviderService")(function* (
203205
options?: ProviderServiceLiveOptions,
204206
) {
205207
const analytics = yield* Effect.service(AnalyticsService.AnalyticsService);
208+
const serverConfig = yield* ServerConfig.ServerConfig;
206209
const eventLoggers = yield* ProviderEventLoggers.ProviderEventLoggers;
207210
// Options-provided logger wins (test overrides); otherwise we take whatever
208211
// the `ProviderEventLoggers` tag exposes — `undefined` means "no canonical
@@ -665,16 +668,44 @@ const makeProviderService = Effect.fn("makeProviderService")(function* (
665668
payload: rawInput,
666669
});
667670

668-
const input = {
669-
...parsed,
670-
attachments: parsed.attachments ?? [],
671-
};
672-
if (!input.input && input.attachments.length === 0) {
671+
const attachments = parsed.attachments ?? [];
672+
if (!parsed.input && attachments.length === 0) {
673673
return yield* toValidationError(
674674
"ProviderService.sendTurn",
675675
"Either input text or at least one attachment is required",
676676
);
677677
}
678+
679+
// Adapters inline attachment pixels into the model prompt, but the model's
680+
// tools cannot dereference pixels. Appending the on-disk path is what lets
681+
// a turn like "include this screenshot in the PR" copy the actual file.
682+
// This runs after schema decode, so the appended lines are exempt from the
683+
// PROVIDER_SEND_TURN_MAX_INPUT_CHARS check; attachment count is capped, so
684+
// the overhead is bounded. Unresolvable ids are skipped here and surface
685+
// as adapter errors when the file is read for inlining.
686+
const attachmentPathLines = attachments.flatMap((attachment) => {
687+
const attachmentPath = resolveAttachmentPath({
688+
attachmentsDir: serverConfig.attachmentsDir,
689+
attachment,
690+
});
691+
return attachmentPath === null
692+
? []
693+
: [`[Attached ${attachment.type} "${attachment.name}" is saved at: ${attachmentPath}]`];
694+
});
695+
const inputTextWithAttachmentPaths =
696+
attachmentPathLines.length === 0
697+
? parsed.input
698+
: [parsed.input, attachmentPathLines.join("\n")]
699+
.filter((part): part is string => typeof part === "string" && part.length > 0)
700+
.join("\n\n");
701+
702+
const input = {
703+
...parsed,
704+
...(inputTextWithAttachmentPaths !== undefined
705+
? { input: inputTextWithAttachmentPaths }
706+
: {}),
707+
attachments,
708+
};
678709
yield* Effect.annotateCurrentSpan({
679710
"provider.operation": "send-turn",
680711
"provider.thread_id": input.threadId,

0 commit comments

Comments
 (0)