From 95328d33f72cf1eeb930b4a8bafa13003f5c52c4 Mon Sep 17 00:00:00 2001 From: Cosimo Attanasi Date: Sun, 11 Oct 2026 21:14:54 +0200 Subject: [PATCH 1/2] Bug Fix - an IDE freeze caused by a stdin/stdout pipe deadlock between the plugin and the claude process. Restructure the agent documentation so its cross-references resolve. PROBLEM During long sessions IntelliJ IDEA froze, sometimes for over an hour, and only recovered when the claude process was restarted. idea.log records 41 UI freezes. All the long ones are blocked in ClaudeProcess.writeLine / FileOutputStream.writeBytes. ROOT CAUSE The stdout reader thread wrote auto-approved permission replies to claude's stdin synchronously, while processing claude's output. When the stdin pipe was full, that thread blocked and stopped draining stdout. claude then blocked on its own stdout and stopped reading stdin, so neither side could proceed. The UI thread, sending a periodic usage query, then blocked on the same write lock and froze the IDE. Teardown also closed stdin under that lock, so it could stall behind the stuck write. CHANGES - New controller/process/StdinWriter.kt: writes newline-terminated UTF-8 lines to stdin on a single dedicated thread, preserving order. Write failures are logged and do not stop later lines. - controller/process/ClaudeProcess.kt: writeLine queues the line on a bounded single-thread application pool executor and returns immediately; it still returns false when the process is dead. The write lock is removed. Teardown destroys the process before closing stdin, so a write stuck on a full pipe fails instead of stalling the shutdown. - New test controller/process/StdinWriterTest.kt: a pipe that stops draining does not block the caller; lines arrive whole and in order; a broken pipe does not stop later lines. - No security code was changed. The fix sits below PermissionBroker, which is untouched. - Documentation: CLAUDE.md gains the standard header. DIRECTIVES.md gains a "How the plugin is put together" architecture section. AGENTS.md points to DIRECTIVES.md for architecture and adds commands for a single test class, integration tests, a single frontend test file, and running builds and tests through the IDE's run server. CONTRIBUTING.md points to CLAUDE.md and DIRECTIVES.md and drops the link to the missing PROJECTMAP.md. BEHAVIOUR CHANGE writeLine now reports that a line was queued, not that it was written. A later pipe failure is logged and handled by the existing process-termination path. VERIFICATION Built with JBR 21.0.11 on Windows. StdinWriterTest (3), EdtProcessTeardownContractTest (4), and the NoComments, PackageDependency and FileSize contract tests pass. spotlessCheck and detekt pass. Full JVM suite: 4881 tests, 21 failed, 8 skipped. The same 21 fail on the original code: they depend on a POSIX environment (the bin/fake-claude script, Unix socket permissions and path length, symlinks, path separators). buildPlugin produced build/distributions/claude-code-native-6.0.1.zip, which contains StdinWriter and the updated ClaudeProcess. OPEN POINTS Not yet verified end to end in a running IDE. The integration tests that drive a real process cannot run on Windows. Manual check: install the zip and run a long task that previously froze. --- AGENTS.md | 15 +++- CONTRIBUTING.md | 6 +- DIRECTIVES.md | 24 ++++++ .../controller/process/ClaudeProcess.kt | 15 ++-- .../controller/process/StdinWriter.kt | 20 +++++ .../controller/process/StdinWriterTest.kt | 73 +++++++++++++++++++ 6 files changed, 136 insertions(+), 17 deletions(-) create mode 100644 src/main/kotlin/dev/lain/claudejb/controller/process/StdinWriter.kt create mode 100644 src/test/kotlin/dev/lain/claudejb/controller/process/StdinWriterTest.kt diff --git a/AGENTS.md b/AGENTS.md index 4af10045..61acc166 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,8 +6,9 @@ how to verify, and what you must not do. Division of labour, so neither file rots: -- **[`CLAUDE.md`](CLAUDE.md)** — the *architecture*: what the plugin is, the protocol contract, the - collaborator layout, and the history of why things are the way they are. Read it before changing code. +- **[`CLAUDE.md`](CLAUDE.md)** — the absolute prohibitions. Read it first. +- **[`DIRECTIVES.md`](DIRECTIVES.md)** — the standing directives and the *architecture*: what the plugin is, + the protocol contract and the collaborator layout. Read it before changing code. - **`AGENTS.md`** (this file) — the *operations*: commands, gates, conventions, boundaries. - **[`docs/adr/`](docs/adr/README.md)** — the decisions that are hard to reverse or easy to reverse by accident. If a change contradicts an ADR, the ADR is updated in the same change or the change is wrong. @@ -29,7 +30,10 @@ will miss and report as a successful build. ```sh ./gradlew test # unit + headless component + integration. The gate. +./gradlew test --tests "dev.lain.claudejb.model.mcp.BatchTest" # one class; a * glob works too +./gradlew integrationTest # only the headless + fake-claude integration packages npm test # frontend tests (vitest + jsdom) over the real resources/jcef/*.js +npm run build && npx vitest run src/test/frontend/chat # one directory or file of the frontend suite ./gradlew detekt spotlessCheck # static analysis + formatting, both gated in CI npm run lint && npm run format:check # the same two, for the shipped JCEF JavaScript ./gradlew buildPlugin # → build/distributions/*.zip @@ -39,6 +43,9 @@ npm run lint && npm run format:check # the same two, for the shipped JCEF Java ./gradlew koverHtmlReport # coverage (koverVerify is the gate, and runs with `test` in CI) ``` +Inside Claude Code Native, the same builds and tests go through the IDE's `run` server rather than Gradle by +hand. + Test counts are deliberately not written here: they change every release and a number in a runbook is a claim nobody re-checks. `./gradlew test` and `npm test` report their own. @@ -152,8 +159,8 @@ enforced rather than trusted. The architectural rules — `ClaudeSession` stays a delegating orchestrator, never mirror raw CLI output, diffs stay native while everything else is the JCEF web app, no Swing in the UI — live in -**[`CLAUDE.md`](CLAUDE.md)** and are **not repeated here**. Two documents saying the same thing is how both -end up stale, and CLAUDE.md is the one that carries the reasoning. Read it; the rules below are the ones +**[`DIRECTIVES.md`](DIRECTIVES.md)** and are **not repeated here**. Two documents saying the same thing is how +both end up stale, and DIRECTIVES.md is the one that carries the reasoning. Read it; the rules below are the ones that are about *working*, not about the design. - **Frontend changes need frontend tests.** The JS↔CSS class contract test exists because a missing CSS rule diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 700b6cd1..1d966e6b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,11 +4,11 @@ Thanks for considering a contribution. This plugin is GPLv3 and community driven; PRs that improve correctness, UX parity with the original Claude Code, or coverage of the stream-json/control protocol are very welcome. -Please skim [`CLAUDE.md`](CLAUDE.md) before writing code — it documents the +Please skim [`CLAUDE.md`](CLAUDE.md) and [`DIRECTIVES.md`](DIRECTIVES.md) +before writing code — the first holds what is forbidden, the second the architectural decisions and the behavioural contract with the `claude` binary. Doing so will save a review round-trip. -[`PROJECTMAP.md`](PROJECTMAP.md) is the shorter answer to "where does this live", -and [`AGENTS.md`](AGENTS.md) lists the commands CI runs. +[`AGENTS.md`](AGENTS.md) lists the commands CI runs. ## Branching model diff --git a/DIRECTIVES.md b/DIRECTIVES.md index 9f6adf07..ddc73307 100644 --- a/DIRECTIVES.md +++ b/DIRECTIVES.md @@ -74,6 +74,30 @@ requests are not killed. It reaches the helper on the **first line of stdin**, never through an environment variable: `/proc//environ` is readable by any process running as the same user. +## How the plugin is put together + +The commands that build and test it are in [`AGENTS.md`](AGENTS.md). + +- **Each chat tab runs the user's own `claude` binary** over stream-json + (`model/session/launch/SessionLauncher`), one process per tab. `controller/session/ClaudeSession` stays a + **delegating orchestrator**: behaviour goes into its collaborators, not into it. +- **Never mirror raw CLI output.** State is rebuilt from structured stream-json fields; + `system/local_command_output` is the antipattern. +- **The permission mode is the plugin's.** The binary is always launched with `default`, so every tool call + arrives as a control request and `SensitiveGuard` decides before any auto-approval. +- **The UI is JCEF; diffs are the only native UI.** `view/jcef/JcefHost` loads scripts and stylesheets it + names explicitly (`appNames`, `CSS_PARTS`) and globs nothing. JS↔Kotlin goes through + `model/bridge/JcefBridge` and `controller/bridge/*`, routed by `ChatBridgeRouter`. The TypeScript in + `src/main/ts/jcef` compiles to `build/web/jcef` and is packaged with the resources. +- **The MCP servers live in `controller/mcp/`.** A domain is one row in `IdeToolCatalog`, per server. + `ToolDomain` enforces the four-tool ceiling and that every list parameter is declared in + `Batch.ONE_CALL_LISTS`. Results are TOON (`model/mcp/toon`), and the guard runs at `GuardGate`. +- **The layering is a test.** `PackageDependencyContractTest` holds the allowed imports per package: + `model/` never imports `controller/` or `view/`, and `model/protocol`, `model/permission`, `model/bridge` + and `model/mcp` import nothing from the platform so they test on a plain JVM. A new package is added to its + `ALLOWED` map. The other `*ContractTest.kt` files at the root of `src/test/kotlin/dev/lain/claudejb/` + enforce no comments, the file-size ceiling, no internal platform API and reachability. + ## Code - **No comments.** The reasoning goes into a name, a test, or the commit message. See `CLAUDE.md`. diff --git a/src/main/kotlin/dev/lain/claudejb/controller/process/ClaudeProcess.kt b/src/main/kotlin/dev/lain/claudejb/controller/process/ClaudeProcess.kt index c042800b..e6801189 100644 --- a/src/main/kotlin/dev/lain/claudejb/controller/process/ClaudeProcess.kt +++ b/src/main/kotlin/dev/lain/claudejb/controller/process/ClaudeProcess.kt @@ -7,6 +7,7 @@ import com.intellij.execution.process.ProcessListener import com.intellij.execution.process.ProcessOutputTypes import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.util.Key +import com.intellij.util.concurrency.AppExecutorUtil import dev.lain.claudejb.model.protocol.ClaudeEvent import dev.lain.claudejb.model.protocol.parse.ProtocolParser import dev.lain.claudejb.util.LogRedaction @@ -24,7 +25,7 @@ class ClaudeProcess( private val extraEnv: Map = emptyMap(), ) { private val log = thisLogger() - private val writeLock = Any() + private val stdin = StdinWriter(AppExecutorUtil.createBoundedApplicationPoolExecutor("Claude stdin", 1)) private val stdoutBuffer = StringBuilder() private var stderrLines = 0 @@ -116,14 +117,8 @@ class ClaudeProcess( log.warn("Dropping a ${line.length}-char line to dead claude stdin") return false } - return runCatching { - synchronized(writeLock) { - stream.write(line.toByteArray(StandardCharsets.UTF_8)) - stream.write('\n'.code) - stream.flush() - } - }.onFailure { log.warn("Could not write to claude stdin; the process is gone or its pipe is closed", it) } - .isSuccess + stdin.enqueue(stream, line) + return true } fun isRunning(): Boolean = handler?.let { !it.isProcessTerminated } ?: false @@ -141,7 +136,7 @@ class ClaudeProcess( } private fun endProcess(dying: KillableProcessHandler) { - runCatching { synchronized(writeLock) { dying.processInput?.close() } } dying.destroyProcess() + runCatching { dying.processInput?.close() } } } diff --git a/src/main/kotlin/dev/lain/claudejb/controller/process/StdinWriter.kt b/src/main/kotlin/dev/lain/claudejb/controller/process/StdinWriter.kt new file mode 100644 index 00000000..394ea77e --- /dev/null +++ b/src/main/kotlin/dev/lain/claudejb/controller/process/StdinWriter.kt @@ -0,0 +1,20 @@ +package dev.lain.claudejb.controller.process + +import dev.lain.claudejb.util.thisLogger +import java.io.OutputStream +import java.nio.charset.StandardCharsets +import java.util.concurrent.Executor + +class StdinWriter(private val executor: Executor) { + private val log = thisLogger() + + fun enqueue(stream: OutputStream, line: String) { + val bytes = "$line\n".toByteArray(StandardCharsets.UTF_8) + executor.execute { + runCatching { + stream.write(bytes) + stream.flush() + }.onFailure { log.warn("Could not write to claude stdin; the process is gone or its pipe is closed", it) } + } + } +} diff --git a/src/test/kotlin/dev/lain/claudejb/controller/process/StdinWriterTest.kt b/src/test/kotlin/dev/lain/claudejb/controller/process/StdinWriterTest.kt new file mode 100644 index 00000000..c1068c7d --- /dev/null +++ b/src/test/kotlin/dev/lain/claudejb/controller/process/StdinWriterTest.kt @@ -0,0 +1,73 @@ +package dev.lain.claudejb.controller.process + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTimeoutPreemptively +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.io.ByteArrayOutputStream +import java.io.IOException +import java.io.OutputStream +import java.time.Duration +import java.util.concurrent.CountDownLatch +import java.util.concurrent.ExecutorService +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit + +class StdinWriterTest { + + @Test + fun `a pipe that stops draining never blocks the caller`() { + val release = CountDownLatch(1) + val stalled = object : OutputStream() { + override fun write(b: Int) { + release.await() + } + } + val executor = Executors.newSingleThreadExecutor() + val writer = StdinWriter(executor) + + assertTimeoutPreemptively(Duration.ofSeconds(2)) { + repeat(LINES) { writer.enqueue(stalled, "{\"type\":\"control_response\"}") } + } + + release.countDown() + drain(executor) + } + + @Test + fun `lines reach the pipe whole, in order, one per line`() { + val sink = ByteArrayOutputStream() + val executor = Executors.newSingleThreadExecutor() + val writer = StdinWriter(executor) + + listOf("first", "è second", "third").forEach { writer.enqueue(sink, it) } + drain(executor) + + assertEquals("first\nè second\nthird\n", sink.toString(Charsets.UTF_8)) + } + + @Test + fun `a broken pipe does not stop the lines queued after it`() { + val broken = object : OutputStream() { + override fun write(b: Int) = throw IOException("pipe closed") + } + val sink = ByteArrayOutputStream() + val executor = Executors.newSingleThreadExecutor() + val writer = StdinWriter(executor) + + writer.enqueue(broken, "lost") + writer.enqueue(sink, "kept") + drain(executor) + + assertEquals("kept\n", sink.toString(Charsets.UTF_8)) + } + + private fun drain(executor: ExecutorService) { + executor.shutdown() + assertTrue(executor.awaitTermination(5, TimeUnit.SECONDS)) { "the writer never drained its queue" } + } + + private companion object { + const val LINES = 64 + } +} From 8b18d82c6a1f7054a0c710fbd988894774b24c0e Mon Sep 17 00:00:00 2001 From: Cosimo Attanasi Date: Mon, 26 Oct 2026 11:48:25 +0100 Subject: [PATCH 2/2] Bug Fix - The model label shows the model's name and version, and every question card offers an Other answer with a free-text field. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The model label could show a tagline from the catalog description instead of the model's name, because it took whatever came before " · ". It now uses only a segment that names the model with a version. Question cards had no way to give an answer outside the offered options; each question now ends with an Other choice that opens a text field, and the typed text is what gets sent. src/main/kotlin/dev/lain/claudejb/view/payload/composer/JcefModelLabels.kt - modelDisplayLabel picks the first description segment that starts with the model family and carries a version. Failing that, it uses the display name, then the name derived from the model id, but only if they carry a version. Failing that, it uses the bare name. A description segment without the family and a version is never used, because the catalog sometimes puts only a tagline there. The version pattern needs a word boundary after the digits, so a context size such as "1M" is not read as a version. src/main/ts/jcef/views/permissions/cards/question.ts - Every question ends with one fixed Other option. An Other supplied by the model (matched case-insensitively) is dropped rather than shown twice. Choosing Other reveals the text field and focuses it, and deselecting it hides the field. On submit, Other is replaced by the trimmed text typed into the field, so the model gets the answer and not the word Other. If the field is empty, Other is left out. On a multi-select the typed text is joined with the other chosen options. The click logic and option construction were moved into toggle, optionButton, otherInput and answerFor, keeping the builder readable and inside the file-size ceiling. src/main/resources/jcef/css/views/permissions/cards.css - Layout and hidden state of .q-other-input: full width, sitting under the options. src/main/resources/jcef/css/views/permissions/elicit.css - .q-other-input joins the existing input, focus and placeholder selectors, reusing the existing field style instead of defining a second one. src/test/kotlin/dev/lain/claudejb/view/payload/composer/JcefModelLabelTest.kt src/test/frontend/permissions/question-card.test.js Verification: npm test 46 files / 686 tests passed; eslint and prettier --check clean; gradlew test on JcefModelLabelTest and all *ContractTest passed; spotlessCheck passed. Not verified: detekt fails before analysing anything (Gradle appears to run on JDK 25 instead of JBR 21); the IDE's own analysis of the files had not finished; the keyboard pass on the new field has not been done. --- CHANGELOG.md | 14 +++ CLAUDE.md | 4 + .../view/payload/composer/JcefModelLabels.kt | 19 ++- .../jcef/css/views/permissions/cards.css | 9 ++ .../jcef/css/views/permissions/elicit.css | 3 + .../jcef/views/permissions/cards/question.ts | 112 ++++++++++++------ .../permissions/question-card.test.js | 72 +++++++++++ .../payload/composer/JcefModelLabelTest.kt | 20 ++++ 8 files changed, 213 insertions(+), 40 deletions(-) create mode 100644 src/test/frontend/permissions/question-card.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 40fad6a0..dc371f80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,20 @@ All notable changes to this project will be documented in this file. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Versioning follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed +- **The model's name and version are shown once Claude is connected**, not its tagline. The label took + whatever came before ` · ` in the catalog's description, so a description that carries only a tagline + ("Fastest for quick answers") became the label. It now takes the description's segment that names the + model with a version, else a display name or the id's derived name that carries a version, and never a + bare description. + +### Added +- **Every question Claude asks ends with an Other choice.** Picking it opens a text field, and the answer + sent back is what was typed, joined with any options also chosen on a multi-select. An `Other` the model + supplied itself is folded into this one, so the card never shows two. + ## [6.0.1] — 2026-09-22 ### Fixed diff --git a/CLAUDE.md b/CLAUDE.md index 91f45d05..8c6fbe17 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,3 +1,7 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + # Project rules > **Read [`DIRECTIVES.md`](DIRECTIVES.md) before starting.** It holds every standing directive for this diff --git a/src/main/kotlin/dev/lain/claudejb/view/payload/composer/JcefModelLabels.kt b/src/main/kotlin/dev/lain/claudejb/view/payload/composer/JcefModelLabels.kt index aa5f69ca..d9565546 100644 --- a/src/main/kotlin/dev/lain/claudejb/view/payload/composer/JcefModelLabels.kt +++ b/src/main/kotlin/dev/lain/claudejb/view/payload/composer/JcefModelLabels.kt @@ -7,14 +7,21 @@ import dev.lain.claudejb.model.session.history.LegacyModels object JcefModelLabels { fun modelDisplayLabel(m: ModelInfo): String { - val descHead = m.description.substringBefore(" · ").trim() - return when { - descHead.isNotBlank() -> descHead - m.displayName.isNotBlank() -> m.displayName - else -> deriveModelLabel(m.value) - } + val derived = deriveModelLabel(m.value) + val name = m.displayName.ifBlank { derived } + val family = name.substringBefore(' ') + val described = m.description.split(" · ").map { it.trim() } + .firstOrNull { it.startsWith(family, ignoreCase = true) && hasVersion(it) } + return described + ?: m.displayName.takeIf { hasVersion(it) } + ?: derived.takeIf { hasVersion(it) } + ?: name } + private val VERSION = Regex("""\b\d+(\.\d+)?\b""") + + private fun hasVersion(label: String): Boolean = VERSION.containsMatchIn(label) + fun modelLabel(session: ClaudeSession): String { val id = session.launch.model ?: session.catalog.preferredDefaultModel() session.catalog.models.firstOrNull { it.value == id }?.let { return modelDisplayLabel(it) } diff --git a/src/main/resources/jcef/css/views/permissions/cards.css b/src/main/resources/jcef/css/views/permissions/cards.css index 76176396..1967bca2 100644 --- a/src/main/resources/jcef/css/views/permissions/cards.css +++ b/src/main/resources/jcef/css/views/permissions/cards.css @@ -219,3 +219,12 @@ color: var(--dim); line-height: 1.45; } +.q-other-input { + display: block; + box-sizing: border-box; + width: 100%; + margin: -3px 0 10px; +} +.q-other-input[hidden] { + display: none; +} diff --git a/src/main/resources/jcef/css/views/permissions/elicit.css b/src/main/resources/jcef/css/views/permissions/elicit.css index 259a1017..fd84444e 100644 --- a/src/main/resources/jcef/css/views/permissions/elicit.css +++ b/src/main/resources/jcef/css/views/permissions/elicit.css @@ -23,6 +23,7 @@ color: var(--dim); } .elicit-card input, +.q-card .q-other-input, .elicit-card select, .boot-path-controls input, .auth-row input { @@ -38,6 +39,7 @@ box-shadow 0.14s; } .elicit-card input:focus, +.q-card .q-other-input:focus, .elicit-card select:focus, .boot-path-controls input:focus, .auth-row input:focus { @@ -45,6 +47,7 @@ box-shadow: 0 0 0 3px var(--accent-soft); } .elicit-card input::placeholder, +.q-card .q-other-input::placeholder, .boot-path-controls input::placeholder, .auth-row input::placeholder { color: var(--dim); diff --git a/src/main/ts/jcef/views/permissions/cards/question.ts b/src/main/ts/jcef/views/permissions/cards/question.ts index e0640a73..4c94937e 100644 --- a/src/main/ts/jcef/views/permissions/cards/question.ts +++ b/src/main/ts/jcef/views/permissions/cards/question.ts @@ -6,10 +6,61 @@ const h = CC.h; + const OTHER = 'Other'; + + function isOther(label: string): boolean { + return label.trim().toLowerCase() === OTHER.toLowerCase(); + } + PM.buildQuestionCard = function (card: PermissionCard): HTMLElement { const id = card.id; const questions = Array.isArray(card.questions) ? card.questions : []; const selections: Record = {}; + const otherInputs: Record = {}; + + function toggle(qText: string, label: string, multi: boolean): void { + const arr = selections[qText]; + if (multi) { + const i = arr.indexOf(label); + if (i >= 0) arr.splice(i, 1); + else arr.push(label); + } else { + selections[qText] = [label]; + } + syncSelected(); + if (label === OTHER && selections[qText].indexOf(OTHER) >= 0) otherInputs[qText].focus(); + } + + function optionButton(qText: string, label: string, desc: string, preview: string, multi: boolean) { + const props: HProps = { + class: 'q-option', + attrs: { type: 'button' }, + on: { + click: function () { + toggle(qText, label, multi); + }, + }, + }; + if (preview) props.title = preview; + const el = h( + 'button', + props, + h('span', { class: 'q-option-label', text: label }), + desc ? h('div', { class: 'q-desc', text: desc }) : null + ) as QuestionOptionEl; + el.__qText = qText; + el.__label = label; + return el; + } + + function otherInput(qText: string): HTMLInputElement { + const input = h('input', { + class: 'q-other-input', + attrs: { type: 'text', placeholder: 'Type your own answer', hidden: 'hidden' }, + }) as HTMLInputElement; + otherInputs[qText] = input; + return input; + } const qBlocks = questions.map(function (q) { const qText = q && q.question != null ? String(q.question) : ''; @@ -17,48 +68,37 @@ const options = q && Array.isArray(q.options) ? q.options : []; if (!(qText in selections)) selections[qText] = []; - const optionEls = options.map(function (opt) { - const label = opt && opt.label != null ? String(opt.label) : ''; - const desc = opt && opt.description != null ? String(opt.description) : ''; - const preview = opt && opt.preview != null ? String(opt.preview) : ''; - - const children = [h('span', { class: 'q-option-label', text: label })]; - if (desc) children.push(h('div', { class: 'q-desc', text: desc })); - - const props: HProps = { - class: 'q-option', - attrs: { type: 'button' }, - on: { - click: function () { - const arr = selections[qText]; - if (multi) { - const i = arr.indexOf(label); - if (i >= 0) arr.splice(i, 1); - else arr.push(label); - } else { - selections[qText] = [label]; - } - syncSelected(); - }, - }, - }; - if (preview) props.title = preview; - - const el = h('button', props, children[0], children[1] || null) as QuestionOptionEl; - el.__qText = qText; - el.__label = label; - return el; - }); + const optionEls = options + .map(function (opt) { + const label = opt && opt.label != null ? String(opt.label) : ''; + if (isOther(label)) return null; + const desc = opt && opt.description != null ? String(opt.description) : ''; + const preview = opt && opt.preview != null ? String(opt.preview) : ''; + return optionButton(qText, label, desc, preview, multi); + }) + .filter(Boolean) as HTMLElement[]; + optionEls.push(optionButton(qText, OTHER, 'None of these: give your own answer', '', multi)); return h( 'div', { class: 'q-block' }, q && q.header ? h('div', { class: 'q-header', text: String(q.header) }) : null, h('div', { class: 'q-question', text: qText }), - h('div', { class: 'q-options' }, optionEls) + h('div', { class: 'q-options' }, optionEls), + otherInput(qText) ); }); + function answerFor(qText: string): string { + const typed = otherInputs[qText] ? otherInputs[qText].value.trim() : ''; + return selections[qText] + .map(function (label) { + return label === OTHER ? typed : label; + }) + .filter(Boolean) + .join(', '); + } + const root = h( 'div', { class: 'perm-card q-card' }, @@ -70,7 +110,7 @@ PM.button({ class: 'btn primary', text: 'Submit' }, function () { const answers: Record = {}; Object.keys(selections).forEach(function (qText) { - answers[qText] = selections[qText].join(', '); + answers[qText] = answerFor(qText); }); PM.sendFor(card, { type: 'resolveQuestion', id: id, answers: answers }); }), @@ -88,6 +128,10 @@ if (el.__label != null && arr.indexOf(el.__label) >= 0) el.classList.add('selected'); else el.classList.remove('selected'); } + Object.keys(otherInputs).forEach(function (qText) { + if (selections[qText].indexOf(OTHER) >= 0) otherInputs[qText].removeAttribute('hidden'); + else otherInputs[qText].setAttribute('hidden', 'hidden'); + }); } syncSelected(); return root; diff --git a/src/test/frontend/permissions/question-card.test.js b/src/test/frontend/permissions/question-card.test.js new file mode 100644 index 00000000..f9654ac9 --- /dev/null +++ b/src/test/frontend/permissions/question-card.test.js @@ -0,0 +1,72 @@ +const { loadFrontend } = require('../helpers/load'); + +const questionCard = (multiSelect, options) => ({ + id: 'q1', + title: 'Question', + questions: [{ question: 'Which one?', header: 'Pick', multiSelect, options }], +}); + +const optionLabels = (win) => + [...win.CC.els.permissions.querySelectorAll('.q-option .q-option-label')].map((el) => el.textContent); + +const clickOption = (win, label) => + [...win.CC.els.permissions.querySelectorAll('.q-option')] + .find((el) => el.querySelector('.q-option-label').textContent === label) + .click(); + +const submit = (win) => { + const sent = []; + win.CC.send = (m) => sent.push(m); + [...win.CC.els.permissions.querySelectorAll('.btn')].find((b) => b.textContent.trim() === 'Submit').click(); + return sent; +}; + +describe('question card — the Other answer', () => { + it('always ends with Other, even when the model offered none', () => { + const win = loadFrontend(['app-permissions.js']); + win.cc.permissions([questionCard(false, [{ label: 'A' }, { label: 'B' }])]); + expect(optionLabels(win)).toEqual(['A', 'B', 'Other']); + }); + + it('keeps a single Other when the model already offered one', () => { + const win = loadFrontend(['app-permissions.js']); + win.cc.permissions([questionCard(false, [{ label: 'A' }, { label: 'other' }])]); + expect(optionLabels(win)).toEqual(['A', 'Other']); + }); + + it('shows the text field only while Other is chosen', () => { + const win = loadFrontend(['app-permissions.js']); + win.cc.permissions([questionCard(false, [{ label: 'A' }])]); + const input = win.CC.els.permissions.querySelector('.q-other-input'); + expect(input.hasAttribute('hidden')).toBe(true); + clickOption(win, 'Other'); + expect(input.hasAttribute('hidden')).toBe(false); + clickOption(win, 'A'); + expect(input.hasAttribute('hidden')).toBe(true); + }); + + it('answers with the typed text, not the word Other', () => { + const win = loadFrontend(['app-permissions.js']); + win.cc.permissions([questionCard(false, [{ label: 'A' }])]); + clickOption(win, 'Other'); + win.CC.els.permissions.querySelector('.q-other-input').value = ' something else '; + expect(submit(win)).toContainEqual({ + type: 'resolveQuestion', + id: 'q1', + answers: { 'Which one?': 'something else' }, + }); + }); + + it('joins the typed text with the chosen options on a multi-select', () => { + const win = loadFrontend(['app-permissions.js']); + win.cc.permissions([questionCard(true, [{ label: 'A' }, { label: 'B' }])]); + clickOption(win, 'A'); + clickOption(win, 'Other'); + win.CC.els.permissions.querySelector('.q-other-input').value = 'C'; + expect(submit(win)).toContainEqual({ + type: 'resolveQuestion', + id: 'q1', + answers: { 'Which one?': 'A, C' }, + }); + }); +}); diff --git a/src/test/kotlin/dev/lain/claudejb/view/payload/composer/JcefModelLabelTest.kt b/src/test/kotlin/dev/lain/claudejb/view/payload/composer/JcefModelLabelTest.kt index 61fe41c6..ac400dc0 100644 --- a/src/test/kotlin/dev/lain/claudejb/view/payload/composer/JcefModelLabelTest.kt +++ b/src/test/kotlin/dev/lain/claudejb/view/payload/composer/JcefModelLabelTest.kt @@ -17,6 +17,26 @@ class JcefModelLabelTest { assertEquals("Haiku 4.5", JcefModelLabels.modelDisplayLabel(haiku())) } + @Test + fun `a description without the model name never becomes the label`() { + assertEquals("Haiku 4.5", JcefModelLabels.modelDisplayLabel(ModelInfo("haiku", "Haiku 4.5", "Fastest for quick answers"))) + assertEquals( + "Sonnet 5", + JcefModelLabels.modelDisplayLabel(ModelInfo("claude-sonnet-5", "Sonnet", "For complex work and everyday tasks")), + ) + assertEquals("Sonnet", JcefModelLabels.modelDisplayLabel(ModelInfo("sonnet", "Sonnet", "For complex work and everyday tasks"))) + } + + @Test + fun `the named segment wins wherever it sits in the description`() { + assertEquals("Opus 5.5", JcefModelLabels.modelDisplayLabel(ModelInfo("opus", "Opus", "Best for complex work · Opus 5.5"))) + } + + @Test + fun `a context size is not mistaken for a version`() { + assertEquals("Opus 4.8", JcefModelLabels.modelDisplayLabel(ModelInfo("claude-opus-4-8[1m]", "Opus (1M context)", "For long sessions"))) + } + @Test fun `label falls back to displayName when there is no description`() { assertEquals("Sonnet", JcefModelLabels.modelDisplayLabel(ModelInfo("sonnet", "Sonnet", "")))