Skip to content

Commit 3c7f313

Browse files
anthonykim1anthonykim1
committed
Dont send false query
1 parent 457542b commit 3c7f313

3 files changed

Lines changed: 288 additions & 11 deletions

File tree

‎addons/addon-image/src/kitty/KittyGraphicsHandler.ts‎

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,14 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
309309
this._sendResponse(cmd.id ?? 0, 'OK', cmd.quiet ?? 0);
310310
return true;
311311
default:
312+
// TODO: Implement remaining actions when needed:
313+
// - a=p (placement): place a previously transmitted image
314+
// - a=f (frame): animation frame operations
315+
// - a=a (animation): animation control
316+
// - a=c (compose): compose images
317+
if (cmd.id !== undefined) {
318+
this._sendResponse(cmd.id, 'EINVAL:unsupported action', cmd.quiet ?? 0);
319+
}
312320
return true;
313321
}
314322
}
@@ -319,8 +327,12 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
319327
switch (action) {
320328
case KittyAction.TRANSMIT: {
321329
const result = this._handleTransmit(cmd, bytes, decodeError);
322-
if (cmd.id !== undefined && !decodeError && bytes.length > 0) {
323-
this._sendResponse(cmd.id, 'OK', cmd.quiet ?? 0);
330+
if (cmd.id !== undefined) {
331+
if (decodeError) {
332+
this._sendResponse(cmd.id, 'EINVAL:invalid base64 data', cmd.quiet ?? 0);
333+
} else if (bytes.length > 0) {
334+
this._sendResponse(cmd.id, 'OK', cmd.quiet ?? 0);
335+
}
324336
}
325337
return result;
326338
}
@@ -329,6 +341,14 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
329341
case KittyAction.QUERY:
330342
return this._handleQuery(cmd, bytes, decodeError);
331343
default:
344+
// TODO: Implement remaining actions when needed:
345+
// - a=p (placement): place a previously transmitted image
346+
// - a=f (frame): animation frame operations
347+
// - a=a (animation): animation control
348+
// - a=c (compose): compose images
349+
if (cmd.id !== undefined) {
350+
this._sendResponse(cmd.id, 'EINVAL:unsupported action', cmd.quiet ?? 0);
351+
}
332352
return true;
333353
}
334354
}
@@ -338,11 +358,12 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
338358
// Currently only supports direct transmission (t=d, the default).
339359
// - t=f (file): Payload is base64-encoded file path. Terminal reads image from that path.
340360
// - t=t (temp file): Payload is base64-encoded path in temp directory. Terminal reads, deletes.
341-
// - t=s Payload is base64-encoded POSIX shm name. Terminal reads from shared memory.
361+
// - t=s: Payload is base64-encoded POSIX shm name. Terminal reads from shared memory.
342362
// These modes require filesystem/IPC access not available in browsers. For Node.js/Electron:
343363
// 1. Check cmd.transmission (t key) before treating bytes as image data
344364
// 2. For t=f/t/s: decode bytes as UTF-8 string (the path/name), then read file contents
345365
// 3. For t=d: treat bytes as image data (current behavior)
366+
// When implementing, also update _handleQuery to accept these transmission mediums.
346367

347368
if (decodeError || bytes.length === 0) return true;
348369

@@ -360,7 +381,12 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
360381
}
361382

362383
private _handleTransmitDisplay(cmd: IKittyCommand, bytes: Uint8Array, decodeError: boolean): boolean | Promise<boolean> {
363-
if (decodeError) return true;
384+
if (decodeError) {
385+
if (cmd.id !== undefined) {
386+
this._sendResponse(cmd.id, 'EINVAL:invalid base64 data', cmd.quiet ?? 0);
387+
}
388+
return true;
389+
}
364390

365391
const pendingKey = cmd.id ?? 0;
366392

@@ -375,12 +401,12 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
375401
if (image) {
376402
const result = this._displayImage(image, cmd);
377403
if (cmd.id !== undefined) {
378-
return (result as Promise<boolean>).then(r => {
379-
this._sendResponse(id, 'OK', cmd.quiet ?? 0);
380-
return r;
404+
return result.then(success => {
405+
this._sendResponse(id, success ? 'OK' : 'EINVAL:image rendering failed', cmd.quiet ?? 0);
406+
return true;
381407
});
382408
}
383-
return result;
409+
return result.then(() => true);
384410
}
385411
return true;
386412
}
@@ -389,6 +415,15 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
389415
const id = cmd.id ?? 0;
390416
const quiet = cmd.quiet ?? 0;
391417

418+
// Per spec: reject unsupported transmission mediums (only t=d is supported atm)
419+
// TODO: When filesystem support is added (Node.js/Electron), update this to accept
420+
// t=f (file), t=t (temp file), and t=s (shared memory) and respond OK for queries.
421+
const transmission = cmd.transmission ?? 'd';
422+
if (transmission !== 'd') {
423+
this._sendResponse(id, 'EINVAL:unsupported transmission medium', quiet);
424+
return true;
425+
}
426+
392427
// Check decode error first (invalid base64)
393428
if (decodeError) {
394429
this._sendResponse(id, 'EINVAL:invalid base64 data', quiet);
@@ -491,10 +526,10 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
491526

492527
// Image display
493528

494-
private _displayImage(image: IKittyImageData, cmd: IKittyCommand): boolean | Promise<boolean> {
529+
private _displayImage(image: IKittyImageData, cmd: IKittyCommand): Promise<boolean> {
495530
return this._decodeAndDisplay(image, cmd)
496531
.then(() => true)
497-
.catch(() => true);
532+
.catch(() => false);
498533
}
499534

500535
private async _decodeAndDisplay(image: IKittyImageData, cmd: IKittyCommand): Promise<void> {
@@ -516,7 +551,9 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
516551
h = Math.round(imgRows * ch);
517552
}
518553

519-
if (w * h > this._opts.pixelLimit) return;
554+
if (w * h > this._opts.pixelLimit) {
555+
throw new Error('image exceeds pixel limit');
556+
}
520557

521558
// Save cursor position before addImage modifies it
522559
const buffer = this._coreTerminal._core.buffer;

‎addons/addon-image/src/kitty/KittyGraphicsTypes.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ export const enum KittyKey {
7373
CURSOR_MOVEMENT = 'C',
7474
// Z-index for image layering (negative = behind text, 0+ = on top)
7575
Z_INDEX = 'z',
76+
// Transmission medium (d=direct, f=file, t=temp file, s=shared memory)
77+
TRANSMISSION = 't',
7678
// Delete selector (a/A=all, i/I=by id, c/C=at cursor, etc.) — only used when a=d
7779
DELETE_SELECTOR = 'd',
7880
// Placement ID for targeting specific placements
@@ -102,6 +104,7 @@ export interface IKittyCommand {
102104
quiet?: number;
103105
cursorMovement?: number;
104106
zIndex?: number;
107+
transmission?: string;
105108
deleteSelector?: string;
106109
placementId?: number;
107110
compression?: string;
@@ -159,6 +162,10 @@ export function parseKittyCommand(data: string): IKittyCommand {
159162
cmd.compression = value;
160163
continue;
161164
}
165+
if (key === KittyKey.TRANSMISSION) {
166+
cmd.transmission = value;
167+
continue;
168+
}
162169
if (key === KittyKey.DELETE_SELECTOR) {
163170
cmd.deleteSelector = value;
164171
continue;

‎addons/addon-image/test/KittyGraphics.test.ts‎

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,6 +544,239 @@ test.describe('Kitty Graphics Protocol', () => {
544544
});
545545
});
546546

547+
test.describe('Error responses for transmit and display', () => {
548+
test('a=t sends EINVAL on decode error when id is specified', async () => {
549+
await ctx.page.evaluate(() => {
550+
(window as any).kittyResponse = '';
551+
(window as any).term.onData((data: string) => { (window as any).kittyResponse = data; });
552+
});
553+
554+
await ctx.proxy.write('\x1b_Gi=110,a=t,f=100;!!!invalid!!!\x1b\\');
555+
await timeout(100);
556+
557+
const response = await ctx.page.evaluate('window.kittyResponse');
558+
strictEqual(response, '\x1b_Gi=110;EINVAL:invalid base64 data\x1b\\');
559+
});
560+
561+
test('a=t sends no response on decode error without id', async () => {
562+
await ctx.page.evaluate(() => {
563+
(window as any).kittyGotResponse = false;
564+
(window as any).term.onData(() => { (window as any).kittyGotResponse = true; });
565+
});
566+
567+
await ctx.proxy.write('\x1b_Ga=t,f=100;!!!invalid!!!\x1b\\');
568+
await timeout(100);
569+
570+
strictEqual(await ctx.page.evaluate('window.kittyGotResponse'), false);
571+
});
572+
573+
test('a=T sends EINVAL on decode error when id is specified', async () => {
574+
await ctx.page.evaluate(() => {
575+
(window as any).kittyResponse = '';
576+
(window as any).term.onData((data: string) => { (window as any).kittyResponse = data; });
577+
});
578+
579+
await ctx.proxy.write('\x1b_Gi=120,a=T,f=100;!!!invalid!!!\x1b\\');
580+
await timeout(100);
581+
582+
const response = await ctx.page.evaluate('window.kittyResponse');
583+
strictEqual(response, '\x1b_Gi=120;EINVAL:invalid base64 data\x1b\\');
584+
});
585+
586+
test('a=T sends no response on decode error without id', async () => {
587+
await ctx.page.evaluate(() => {
588+
(window as any).kittyGotResponse = false;
589+
(window as any).term.onData(() => { (window as any).kittyGotResponse = true; });
590+
});
591+
592+
await ctx.proxy.write('\x1b_Ga=T,f=100;!!!invalid!!!\x1b\\');
593+
await timeout(100);
594+
595+
strictEqual(await ctx.page.evaluate('window.kittyGotResponse'), false);
596+
});
597+
598+
test('a=T sends EINVAL when raw pixel render fails (missing dimensions)', async () => {
599+
await ctx.page.evaluate(() => {
600+
(window as any).kittyResponse = '';
601+
(window as any).term.onData((data: string) => { (window as any).kittyResponse = data; });
602+
});
603+
604+
await ctx.proxy.write(`\x1b_Gi=130,a=T,f=24;${RAW_RGB_1X1_BLACK}\x1b\\`);
605+
await timeout(100);
606+
607+
const response: string = await ctx.page.evaluate('window.kittyResponse');
608+
strictEqual(response.startsWith('\x1b_Gi=130;EINVAL:'), true);
609+
});
610+
611+
test('a=T sends OK on successful render with id', async () => {
612+
await ctx.page.evaluate(() => {
613+
(window as any).kittyResponse = '';
614+
(window as any).term.onData((data: string) => { (window as any).kittyResponse = data; });
615+
});
616+
617+
await ctx.proxy.write(`\x1b_Gi=140,a=T,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`);
618+
await timeout(100);
619+
620+
const response = await ctx.page.evaluate('window.kittyResponse');
621+
strictEqual(response, '\x1b_Gi=140;OK\x1b\\');
622+
});
623+
624+
test('a=t sends OK on successful transmit with id', async () => {
625+
await ctx.page.evaluate(() => {
626+
(window as any).kittyResponse = '';
627+
(window as any).term.onData((data: string) => { (window as any).kittyResponse = data; });
628+
});
629+
630+
await ctx.proxy.write(`\x1b_Gi=150,a=t,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`);
631+
await timeout(100);
632+
633+
const response = await ctx.page.evaluate('window.kittyResponse');
634+
strictEqual(response, '\x1b_Gi=150;OK\x1b\\');
635+
});
636+
637+
test('a=t EINVAL suppressed by q=2', async () => {
638+
await ctx.page.evaluate(() => {
639+
(window as any).kittyGotResponse = false;
640+
(window as any).term.onData(() => { (window as any).kittyGotResponse = true; });
641+
});
642+
643+
await ctx.proxy.write('\x1b_Gi=160,a=t,q=2,f=100;!!!invalid!!!\x1b\\');
644+
await timeout(100);
645+
646+
strictEqual(await ctx.page.evaluate('window.kittyGotResponse'), false);
647+
});
648+
649+
test('a=T EINVAL suppressed by q=2', async () => {
650+
await ctx.page.evaluate(() => {
651+
(window as any).kittyGotResponse = false;
652+
(window as any).term.onData(() => { (window as any).kittyGotResponse = true; });
653+
});
654+
655+
await ctx.proxy.write('\x1b_Gi=170,a=T,q=2,f=100;!!!invalid!!!\x1b\\');
656+
await timeout(100);
657+
658+
strictEqual(await ctx.page.evaluate('window.kittyGotResponse'), false);
659+
});
660+
661+
test('a=t OK suppressed by q=1', async () => {
662+
await ctx.page.evaluate(() => {
663+
(window as any).kittyGotResponse = false;
664+
(window as any).term.onData(() => { (window as any).kittyGotResponse = true; });
665+
});
666+
667+
await ctx.proxy.write(`\x1b_Gi=180,a=t,q=1,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`);
668+
await timeout(100);
669+
670+
strictEqual(await ctx.page.evaluate('window.kittyGotResponse'), false);
671+
});
672+
673+
test('a=T OK suppressed by q=1', async () => {
674+
await ctx.page.evaluate(() => {
675+
(window as any).kittyGotResponse = false;
676+
(window as any).term.onData(() => { (window as any).kittyGotResponse = true; });
677+
});
678+
679+
await ctx.proxy.write(`\x1b_Gi=190,a=T,q=1,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`);
680+
await timeout(100);
681+
682+
strictEqual(await ctx.page.evaluate('window.kittyGotResponse'), false);
683+
});
684+
});
685+
686+
test.describe('Transmission medium rejection', () => {
687+
test('query rejects t=f (file transmission)', async () => {
688+
await ctx.page.evaluate(() => {
689+
(window as any).kittyResponse = '';
690+
(window as any).term.onData((data: string) => { (window as any).kittyResponse = data; });
691+
});
692+
693+
await ctx.proxy.write(`\x1b_Gi=200,a=q,t=f,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`);
694+
await timeout(100);
695+
696+
const response: string = await ctx.page.evaluate('window.kittyResponse');
697+
strictEqual(response.startsWith('\x1b_Gi=200;EINVAL:'), true);
698+
});
699+
700+
test('query rejects t=s (shared memory)', async () => {
701+
await ctx.page.evaluate(() => {
702+
(window as any).kittyResponse = '';
703+
(window as any).term.onData((data: string) => { (window as any).kittyResponse = data; });
704+
});
705+
706+
await ctx.proxy.write(`\x1b_Gi=201,a=q,t=s,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`);
707+
await timeout(100);
708+
709+
const response: string = await ctx.page.evaluate('window.kittyResponse');
710+
strictEqual(response.startsWith('\x1b_Gi=201;EINVAL:'), true);
711+
});
712+
713+
test('query rejects t=t (temp file)', async () => {
714+
await ctx.page.evaluate(() => {
715+
(window as any).kittyResponse = '';
716+
(window as any).term.onData((data: string) => { (window as any).kittyResponse = data; });
717+
});
718+
719+
await ctx.proxy.write(`\x1b_Gi=202,a=q,t=t,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`);
720+
await timeout(100);
721+
722+
const response: string = await ctx.page.evaluate('window.kittyResponse');
723+
strictEqual(response.startsWith('\x1b_Gi=202;EINVAL:'), true);
724+
});
725+
726+
test('query accepts t=d (direct transmission)', async () => {
727+
await ctx.page.evaluate(() => {
728+
(window as any).kittyResponse = '';
729+
(window as any).term.onData((data: string) => { (window as any).kittyResponse = data; });
730+
});
731+
732+
await ctx.proxy.write(`\x1b_Gi=203,a=q,t=d,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`);
733+
await timeout(100);
734+
735+
const response = await ctx.page.evaluate('window.kittyResponse');
736+
strictEqual(response, '\x1b_Gi=203;OK\x1b\\');
737+
});
738+
739+
test('query without t key defaults to direct (OK)', async () => {
740+
await ctx.page.evaluate(() => {
741+
(window as any).kittyResponse = '';
742+
(window as any).term.onData((data: string) => { (window as any).kittyResponse = data; });
743+
});
744+
745+
await ctx.proxy.write(`\x1b_Gi=204,a=q,f=100;${KITTY_BLACK_1X1_BASE64}\x1b\\`);
746+
await timeout(100);
747+
748+
const response = await ctx.page.evaluate('window.kittyResponse');
749+
strictEqual(response, '\x1b_Gi=204;OK\x1b\\');
750+
});
751+
});
752+
753+
test.describe('Unimplemented action responses', () => {
754+
test('a=p with id responds EINVAL', async () => {
755+
await ctx.page.evaluate(() => {
756+
(window as any).kittyResponse = '';
757+
(window as any).term.onData((data: string) => { (window as any).kittyResponse = data; });
758+
});
759+
760+
await ctx.proxy.write(`\x1b_Gi=210,a=p\x1b\\`);
761+
await timeout(100);
762+
763+
const response: string = await ctx.page.evaluate('window.kittyResponse');
764+
strictEqual(response.startsWith('\x1b_Gi=210;EINVAL:'), true);
765+
});
766+
767+
test('a=p without id sends no response', async () => {
768+
await ctx.page.evaluate(() => {
769+
(window as any).kittyGotResponse = false;
770+
(window as any).term.onData(() => { (window as any).kittyGotResponse = true; });
771+
});
772+
773+
await ctx.proxy.write(`\x1b_Ga=p\x1b\\`);
774+
await timeout(100);
775+
776+
strictEqual(await ctx.page.evaluate('window.kittyGotResponse'), false);
777+
});
778+
});
779+
547780
test.describe('Cursor positioning', () => {
548781
// NOTE: Current tests document ACTUAL behavior (MVP - cursor doesn't move)
549782
// Per Kitty spec: cursor placed at first column after last image column,

0 commit comments

Comments
 (0)