| author | |
| committer | |
| log | 263097a8da038a143f2b7b7817250f0ad390ea32 |
| tree | 4473027cf8d4a55e56e19f8da42578ce01331f89 |
| parent | f68e24fb419c985269cd0acb841e203af1419c43 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>6 files changed, 178 insertions(+), 28 deletions(-)
apps/server/src/keybindings.test.ts+71-1| ... | @@ -11,7 +11,7 @@ import * as Path from "effect/Path"; | ... | @@ -11,7 +11,7 @@ import * as Path from "effect/Path"; |
| 11 | import * as Schema from "effect/Schema"; | 11 | import * as Schema from "effect/Schema"; |
| 12 | import * as ServerConfig from "./config.ts"; | 12 | import * as ServerConfig from "./config.ts"; |
| 13 | import * as Keybindings from "./keybindings.ts"; | 13 | import * as Keybindings from "./keybindings.ts"; |
| 14 | import { KeybindingsConfigError } from "@t3tools/contracts"; | 14 | import { KeybindingsConfigError, MAX_KEYBINDINGS_COUNT } from "@t3tools/contracts"; |
| 15 | import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; | 15 | import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; |
| 16 | 16 | ||
| 17 | const KeybindingsConfigJson = Schema.fromJsonString(KeybindingsConfig); | 17 | const KeybindingsConfigJson = Schema.fromJsonString(KeybindingsConfig); |
| ... | @@ -283,6 +283,76 @@ it.layer(NodeServices.layer)("keybindings", (it) => { | ... | @@ -283,6 +283,76 @@ it.layer(NodeServices.layer)("keybindings", (it) => { |
| 283 | }).pipe(Effect.provide(makeKeybindingsLayer())), | 283 | }).pipe(Effect.provide(makeKeybindingsLayer())), |
| 284 | ); | 284 | ); |
| 285 | 285 | ||
| 286 | it.effect("adds a late default to an existing command once", () => | ||
| 287 | Effect.gen(function* () { | ||
| 288 | const { keybindingsConfigPath } = yield* ServerConfig.ServerConfig; | ||
| 289 | const keybindings = yield* Keybindings.Keybindings; | ||
| 290 | const when = "composerFocus && draftThreadRoute"; | ||
| 291 | const existing = { key: "mod+alt+enter", command: "composer.sendBackground", when } as const; | ||
| 292 | const backgroundRules = Effect.map(readKeybindingsConfig(keybindingsConfigPath), (rules) => | ||
| 293 | rules.filter((entry) => entry.command === "composer.sendBackground"), | ||
| 294 | ); | ||
| 295 | yield* writeKeybindingsConfig(keybindingsConfigPath, [existing]); | ||
| 296 | |||
| 297 | yield* keybindings.syncDefaultKeybindingsOnStartup; | ||
| 298 | assert.deepStrictEqual(yield* backgroundRules, [ | ||
| 299 | existing, | ||
| 300 | { key: "mod+enter", command: "composer.sendBackground", when }, | ||
| 301 | ]); | ||
| 302 | |||
| 303 | // Removing the added rule later must survive the next startup. | ||
| 304 | yield* writeKeybindingsConfig(keybindingsConfigPath, [existing]); | ||
| 305 | yield* keybindings.syncDefaultKeybindingsOnStartup; | ||
| 306 | assert.deepStrictEqual(yield* backgroundRules, [existing]); | ||
| 307 | }).pipe(Effect.provide(makeKeybindingsLayer())), | ||
| 308 | ); | ||
| 309 | |||
| 310 | it.effect("leaves a customized command without the late default", () => | ||
| 311 | Effect.gen(function* () { | ||
| 312 | const { keybindingsConfigPath } = yield* ServerConfig.ServerConfig; | ||
| 313 | const keybindings = yield* Keybindings.Keybindings; | ||
| 314 | const custom = { | ||
| 315 | key: "alt+b", | ||
| 316 | command: "composer.sendBackground", | ||
| 317 | when: "composerFocus && draftThreadRoute", | ||
| 318 | } as const; | ||
| 319 | yield* writeKeybindingsConfig(keybindingsConfigPath, [custom]); | ||
| 320 | |||
| 321 | yield* keybindings.syncDefaultKeybindingsOnStartup; | ||
| 322 | const persisted = yield* readKeybindingsConfig(keybindingsConfigPath); | ||
| 323 | assert.deepStrictEqual( | ||
| 324 | persisted.filter((entry) => entry.command === "composer.sendBackground"), | ||
| 325 | [custom], | ||
| 326 | ); | ||
| 327 | }).pipe(Effect.provide(makeKeybindingsLayer())), | ||
| 328 | ); | ||
| 329 | |||
| 330 | it.effect("keeps a late default pending while the config is full", () => | ||
| 331 | Effect.gen(function* () { | ||
| 332 | const { keybindingsConfigPath } = yield* ServerConfig.ServerConfig; | ||
| 333 | const keybindings = yield* Keybindings.Keybindings; | ||
| 334 | const when = "composerFocus && draftThreadRoute"; | ||
| 335 | const existing = { key: "mod+alt+enter", command: "composer.sendBackground", when } as const; | ||
| 336 | const fillers = Array.from({ length: MAX_KEYBINDINGS_COUNT - 1 }, (_, index) => ({ | ||
| 337 | key: "mod+alt+f1", | ||
| 338 | command: `script.filler-${index}.run` as const, | ||
| 339 | })); | ||
| 340 | const hasModEnter = Effect.map(readKeybindingsConfig(keybindingsConfigPath), (rules) => | ||
| 341 | rules.some( | ||
| 342 | (entry) => entry.command === "composer.sendBackground" && entry.key === "mod+enter", | ||
| 343 | ), | ||
| 344 | ); | ||
| 345 | |||
| 346 | yield* writeKeybindingsConfig(keybindingsConfigPath, [existing, ...fillers]); | ||
| 347 | yield* keybindings.syncDefaultKeybindingsOnStartup; | ||
| 348 | assert.isFalse(yield* hasModEnter); | ||
| 349 | |||
| 350 | yield* writeKeybindingsConfig(keybindingsConfigPath, [existing]); | ||
| 351 | yield* keybindings.syncDefaultKeybindingsOnStartup; | ||
| 352 | assert.isTrue(yield* hasModEnter); | ||
| 353 | }).pipe(Effect.provide(makeKeybindingsLayer())), | ||
| 354 | ); | ||
| 355 | |||
| 286 | it.effect("skips conflicting default keybindings on startup and logs a detailed warning", () => { | 356 | it.effect("skips conflicting default keybindings on startup and logs a detailed warning", () => { |
| 287 | const messages: string[] = []; | 357 | const messages: string[] = []; |
| 288 | const logger = Logger.make(({ message }) => { | 358 | const logger = Logger.make(({ message }) => { |
apps/server/src/keybindings.ts+95-20| ... | @@ -104,6 +104,31 @@ function isSameKeybindingRule(left: KeybindingRule, right: KeybindingRule): bool | ... | @@ -104,6 +104,31 @@ function isSameKeybindingRule(left: KeybindingRule, right: KeybindingRule): bool |
| 104 | ); | 104 | ); |
| 105 | } | 105 | } |
| 106 | 106 | ||
| 107 | // Default rules added to a command after startup sync had already persisted | ||
| 108 | // that command. Backfill skips commands a config already has, so startup adds | ||
| 109 | // each rule once per config, only next to the untouched earlier default | ||
| 110 | // (`alongside`), and records its id. Customized commands and later removals | ||
| 111 | // stay as the user set them. | ||
| 112 | const LATE_DEFAULT_KEYBINDINGS: ReadonlyArray<{ | ||
| 113 | readonly id: string; | ||
| 114 | readonly alongside: KeybindingRule; | ||
| 115 | readonly rule: KeybindingRule; | ||
| 116 | }> = [ | ||
| 117 | { | ||
| 118 | id: "composer.sendBackground:mod+enter", | ||
| 119 | alongside: { | ||
| 120 | key: "mod+alt+enter", | ||
| 121 | command: "composer.sendBackground", | ||
| 122 | when: "composerFocus && draftThreadRoute", | ||
| 123 | }, | ||
| 124 | rule: { | ||
| 125 | key: "mod+enter", | ||
| 126 | command: "composer.sendBackground", | ||
| 127 | when: "composerFocus && draftThreadRoute", | ||
| 128 | }, | ||
| 129 | }, | ||
| 130 | ]; | ||
| 131 | |||
| 107 | function keybindingShortcutContext(rule: KeybindingRule): string | null { | 132 | function keybindingShortcutContext(rule: KeybindingRule): string | null { |
| 108 | const parsed = parseKeybindingShortcut(rule.key); | 133 | const parsed = parseKeybindingShortcut(rule.key); |
| 109 | if (!parsed) return null; | 134 | if (!parsed) return null; |
| ... | @@ -399,6 +424,32 @@ const make = Effect.gen(function* () { | ... | @@ -399,6 +424,32 @@ const make = Effect.gen(function* () { |
| 399 | return { keybindings, issues }; | 424 | return { keybindings, issues }; |
| 400 | }); | 425 | }); |
| 401 | 426 | ||
| 427 | // One applied migration id per line, next to the config file. | ||
| 428 | const appliedMigrationsPath = path.join( | ||
| 429 | path.dirname(keybindingsConfigPath), | ||
| 430 | "keybindings-migrations", | ||
| 431 | ); | ||
| 432 | const readAppliedMigrationIds = fs.readFileString(appliedMigrationsPath).pipe( | ||
| 433 | Effect.map((contents) => new Set(contents.split("\n").filter((line) => line.length > 0))), | ||
| 434 | Effect.orElseSucceed(() => new Set<string>()), | ||
| 435 | ); | ||
| 436 | const recordAppliedMigrations = (ids: Iterable<string>) => | ||
| 437 | writeFileStringAtomically({ | ||
| 438 | filePath: appliedMigrationsPath, | ||
| 439 | contents: `${[...new Set(ids)].join("\n")}\n`, | ||
| 440 | }).pipe( | ||
| 441 | Effect.provideService(FileSystem.FileSystem, fs), | ||
| 442 | Effect.provideService(Path.Path, path), | ||
| 443 | Effect.mapError( | ||
| 444 | (cause) => | ||
| 445 | new KeybindingsConfigError({ | ||
| 446 | configPath: keybindingsConfigPath, | ||
| 447 | detail: "failed to record keybinding migrations", | ||
| 448 | cause, | ||
| 449 | }), | ||
| 450 | ), | ||
| 451 | ); | ||
| 452 | |||
| 402 | const writeConfigAtomically = (rules: readonly KeybindingRule[]) => { | 453 | const writeConfigAtomically = (rules: readonly KeybindingRule[]) => { |
| 403 | return encodeKeybindingsConfigPrettyJson(rules).pipe( | 454 | return encodeKeybindingsConfigPrettyJson(rules).pipe( |
| 404 | Effect.map((encoded) => `${encoded}\n`), | 455 | Effect.map((encoded) => `${encoded}\n`), |
| ... | @@ -450,9 +501,19 @@ const make = Effect.gen(function* () { | ... | @@ -450,9 +501,19 @@ const make = Effect.gen(function* () { |
| 450 | 501 | ||
| 451 | const syncDefaultKeybindingsOnStartup = upsertSemaphore.withPermits(1)( | 502 | const syncDefaultKeybindingsOnStartup = upsertSemaphore.withPermits(1)( |
| 452 | Effect.gen(function* () { | 503 | Effect.gen(function* () { |
| 504 | const appliedMigrationIds = yield* readAppliedMigrationIds; | ||
| 505 | const pendingLateDefaults = LATE_DEFAULT_KEYBINDINGS.filter( | ||
| 506 | (late) => !appliedMigrationIds.has(late.id), | ||
| 507 | ); | ||
| 453 | const configExists = yield* readConfigExists; | 508 | const configExists = yield* readConfigExists; |
| 454 | if (!configExists) { | 509 | if (!configExists) { |
| 455 | yield* writeConfigAtomically(DEFAULT_KEYBINDINGS); | 510 | yield* writeConfigAtomically(DEFAULT_KEYBINDINGS); |
| 511 | if (pendingLateDefaults.length > 0) { | ||
| 512 | yield* recordAppliedMigrations([ | ||
| 513 | ...appliedMigrationIds, | ||
| 514 | ...LATE_DEFAULT_KEYBINDINGS.map((late) => late.id), | ||
| 515 | ]); | ||
| 516 | } | ||
| 456 | yield* Cache.invalidate(resolvedConfigCache, resolvedConfigCacheKey); | 517 | yield* Cache.invalidate(resolvedConfigCache, resolvedConfigCacheKey); |
| 457 | return; | 518 | return; |
| 458 | } | 519 | } |
| ... | @@ -496,6 +557,16 @@ const make = Effect.gen(function* () { | ... | @@ -496,6 +557,16 @@ const make = Effect.gen(function* () { |
| 496 | } | 557 | } |
| 497 | missingDefaults.push(defaultRule); | 558 | missingDefaults.push(defaultRule); |
| 498 | } | 559 | } |
| 560 | // The loop above backfills whole missing commands. Late defaults join | ||
| 561 | // an untouched earlier default, when their shortcut is still free. | ||
| 562 | for (const { alongside, rule } of pendingLateDefaults) { | ||
| 563 | if ( | ||
| 564 | customConfig.some((entry) => isSameKeybindingRule(entry, alongside)) && | ||
| 565 | !customConfig.some((entry) => hasSameShortcutContext(entry, rule)) | ||
| 566 | ) { | ||
| 567 | missingDefaults.push(rule); | ||
| 568 | } | ||
| 569 | } | ||
| 499 | for (const conflict of shortcutConflictWarnings) { | 570 | for (const conflict of shortcutConflictWarnings) { |
| 500 | yield* Effect.logWarning("skipping default keybinding due to shortcut conflict", { | 571 | yield* Effect.logWarning("skipping default keybinding due to shortcut conflict", { |
| 501 | path: keybindingsConfigPath, | 572 | path: keybindingsConfigPath, |
| ... | @@ -506,21 +577,18 @@ const make = Effect.gen(function* () { | ... | @@ -506,21 +577,18 @@ const make = Effect.gen(function* () { |
| 506 | reason: "shortcut context already used by existing rule", | 577 | reason: "shortcut context already used by existing rule", |
| 507 | }); | 578 | }); |
| 508 | } | 579 | } |
| 509 | if (missingDefaults.length === 0) { | 580 | if (missingDefaults.length > 0) { |
| 510 | yield* Cache.invalidate(resolvedConfigCache, resolvedConfigCacheKey); | 581 | const matchingDefaults = Array.filterMap(DEFAULT_KEYBINDINGS, (defaultRule) => |
| 511 | return; | 582 | customConfig.some((entry) => isSameKeybindingRule(entry, defaultRule)) |
| 512 | } | 583 | ? Result.succeed(defaultRule.command) |
| 513 | 584 | : Result.failVoid, | |
| 514 | const matchingDefaults = Array.filterMap(DEFAULT_KEYBINDINGS, (defaultRule) => | 585 | ); |
| 515 | customConfig.some((entry) => isSameKeybindingRule(entry, defaultRule)) | 586 | if (matchingDefaults.length > 0) { |
| 516 | ? Result.succeed(defaultRule.command) | 587 | yield* Effect.logWarning("default keybinding rule already defined in user config", { |
| 517 | : Result.failVoid, | 588 | path: keybindingsConfigPath, |
| 518 | ); | 589 | commands: matchingDefaults, |
| 519 | if (matchingDefaults.length > 0) { | 590 | }); |
| 520 | yield* Effect.logWarning("default keybinding rule already defined in user config", { | 591 | } |
| 521 | path: keybindingsConfigPath, | ||
| 522 | commands: matchingDefaults, | ||
| 523 | }); | ||
| 524 | } | 592 | } |
| 525 | 593 | ||
| 526 | // Startup backfill must never evict persisted user rules: append only | 594 | // Startup backfill must never evict persisted user rules: append only |
| ... | @@ -535,12 +603,19 @@ const make = Effect.gen(function* () { | ... | @@ -535,12 +603,19 @@ const make = Effect.gen(function* () { |
| 535 | commands: skippedDefaults.map((rule) => rule.command), | 603 | commands: skippedDefaults.map((rule) => rule.command), |
| 536 | }); | 604 | }); |
| 537 | } | 605 | } |
| 538 | if (defaultsToAppend.length === 0) { | 606 | if (defaultsToAppend.length > 0) { |
| 539 | yield* Cache.invalidate(resolvedConfigCache, resolvedConfigCacheKey); | 607 | yield* writeConfigAtomically([...customConfig, ...defaultsToAppend]); |
| 540 | return; | 608 | } |
| 609 | // A late default skipped at max entries stays pending for a later start. | ||
| 610 | const settledLateDefaults = pendingLateDefaults.filter( | ||
| 611 | (late) => !skippedDefaults.includes(late.rule), | ||
| 612 | ); | ||
| 613 | if (settledLateDefaults.length > 0) { | ||
| 614 | yield* recordAppliedMigrations([ | ||
| 615 | ...appliedMigrationIds, | ||
| 616 | ...settledLateDefaults.map((late) => late.id), | ||
| 617 | ]); | ||
| 541 | } | 618 | } |
| 542 | |||
| 543 | yield* writeConfigAtomically([...customConfig, ...defaultsToAppend]); | ||
| 544 | yield* Cache.invalidate(resolvedConfigCache, resolvedConfigCacheKey); | 619 | yield* Cache.invalidate(resolvedConfigCache, resolvedConfigCacheKey); |
| 545 | }), | 620 | }), |
| 546 | ); | 621 | ); |
apps/web/src/composer-logic.test.ts+1-1| ... | @@ -140,7 +140,7 @@ describe("composerSubmissionIntentForKey", () => { | ... | @@ -140,7 +140,7 @@ describe("composerSubmissionIntentForKey", () => { |
| 140 | isDraftThread: true, | 140 | isDraftThread: true, |
| 141 | event: modEnter, | 141 | event: modEnter, |
| 142 | }), | 142 | }), |
| 143 | ).toBe("foreground"); | 143 | ).toBe("background"); |
| 144 | expect( | 144 | expect( |
| 145 | composerSubmissionIntentForKey({ ...running, event: { ...enter, shiftKey: true } }), | 145 | composerSubmissionIntentForKey({ ...running, event: { ...enter, shiftKey: true } }), |
| 146 | ).toBeNull(); | 146 | ).toBeNull(); |
apps/web/src/keybindings.test.ts+1-1| ... | @@ -1287,7 +1287,7 @@ describe("composer and pull request shortcuts", () => { | ... | @@ -1287,7 +1287,7 @@ describe("composer and pull request shortcuts", () => { |
| 1287 | ctrlKey: platform !== "MacIntel", | 1287 | ctrlKey: platform !== "MacIntel", |
| 1288 | }; | 1288 | }; |
| 1289 | const queuedKey = event({ key: "Enter", shiftKey: true, ...modifier }); | 1289 | const queuedKey = event({ key: "Enter", shiftKey: true, ...modifier }); |
| 1290 | const backgroundKey = event({ key: "Enter", altKey: true, ...modifier }); | 1290 | const backgroundKey = event({ key: "Enter", ...modifier }); |
| 1291 | assert.strictEqual( | 1291 | assert.strictEqual( |
| 1292 | resolveShortcutCommand(queuedKey, DEFAULT_RESOLVED_KEYBINDINGS, { | 1292 | resolveShortcutCommand(queuedKey, DEFAULT_RESOLVED_KEYBINDINGS, { |
| 1293 | platform, | 1293 | platform, |
docs/user/keybindings.md+5-5| ... | @@ -12,11 +12,11 @@ inserts a new line. This applies to the web and desktop composer at desktop widt | ... | @@ -12,11 +12,11 @@ inserts a new line. This applies to the web and desktop composer at desktop widt |
| 12 | **Follow-up behavior** chooses Queue or Steer while the agent runs. Use | 12 | **Follow-up behavior** chooses Queue or Steer while the agent runs. Use |
| 13 | `mod+Enter` to do the opposite for one message, even when the send shortcut | 13 | `mod+Enter` to do the opposite for one message, even when the send shortcut |
| 14 | requires a modifier. `mod+Alt+Enter` sends, keeps that thread running in the | 14 | requires a modifier. `mod+Alt+Enter` sends, keeps that thread running in the |
| 15 | background, and opens a fresh new-thread composer. Change these | 15 | background, and opens a fresh new-thread composer. In a new thread, `mod+Enter` |
| 16 | shortcuts in **Settings → Keybindings** under **Composer: Opposite Queue or | 16 | does the same. Change these shortcuts in **Settings → Keybindings** under |
| 17 | Steer Action**, **Composer: Start in Background**, or **Composer: Send and Start | 17 | **Composer: Opposite Queue or Steer Action**, **Composer: Start in Background**, |
| 18 | New Thread**. These bindings take priority over the send shortcut. Click the | 18 | or **Composer: Send and Start New Thread**. These bindings take priority over the |
| 19 | send button to use the configured follow-up behavior. | 19 | send shortcut. Click the send button to use the configured follow-up behavior. |
| 20 | 20 | ||
| 21 | When an active turn has queued messages, `mod+Shift+Enter` sends the first as a | 21 | When an active turn has queued messages, `mod+Shift+Enter` sends the first as a |
| 22 | steer. Change it under **Queue: Send First Queued Message as Steer** in Keybindings. | 22 | steer. Change it under **Queue: Send First Queued Message as Steer** in Keybindings. |
packages/shared/src/keybindings.ts+5| ... | @@ -48,6 +48,11 @@ export const DEFAULT_KEYBINDINGS: ReadonlyArray<KeybindingRule> = [ | ... | @@ -48,6 +48,11 @@ export const DEFAULT_KEYBINDINGS: ReadonlyArray<KeybindingRule> = [ |
| 48 | { key: "mod+shift+enter", command: "thread.steerQueuedMessage", when: "!terminalFocus" }, | 48 | { key: "mod+shift+enter", command: "thread.steerQueuedMessage", when: "!terminalFocus" }, |
| 49 | { key: "alt+arrowup", command: "thread.editQueuedMessage", when: "composerFocus" }, | 49 | { key: "alt+arrowup", command: "thread.editQueuedMessage", when: "composerFocus" }, |
| 50 | { key: "mod+enter", command: "composer.sendAlternate", when: "composerFocus && turnRunning" }, | 50 | { key: "mod+enter", command: "composer.sendAlternate", when: "composerFocus && turnRunning" }, |
| 51 | { | ||
| 52 | key: "mod+enter", | ||
| 53 | command: "composer.sendBackground", | ||
| 54 | when: "composerFocus && draftThreadRoute", | ||
| 55 | }, | ||
| 51 | { | 56 | { |
| 52 | key: "mod+alt+enter", | 57 | key: "mod+alt+enter", |
| 53 | command: "composer.sendBackground", | 58 | command: "composer.sendBackground", |