diff --git a/docs/changelog/2026-09-15-ops110-f1.md b/docs/changelog/2026-09-15-ops110-f1.md new file mode 100644 index 00000000..d9bdc1ff --- /dev/null +++ b/docs/changelog/2026-09-15-ops110-f1.md @@ -0,0 +1 @@ +**OPS110-F1 — `agent:session` fail-high de flags, single-flight do `start` pinado e help/list completos (2026-09-15):** fecha os débitos do `/simplify` pós-OPS110 sem tocar o contrato de produto. (F1) o CLI agora rejeita **flag desconhecida por verbo** — `SESSION_FLAG_ALLOWLIST` + `validateSessionFlags` + `nearestAcceptedFlag` na lib pura (`scripts/lib/agent-session.mjs`), chamados no dispatch de `scripts/agent-session.mjs`: `--prupose=next` deixa de cair no default mudo `new` (criava sessão sem driver) e falha com sugestão Damerau de distância ≤1 (`--prupose` → `--purpose`) ou ponteiro para o USAGE; `--hostname/--port` são aceitos onde já eram lidos (`serve`/`start`/`list`) e o parser compartilhado `parseEqualsFlags` ficou intocado (contrato silencioso de 5–7 scripts). (F2) a decisão "reusar vs criar" saiu de dentro do `cmdStart` para a função pura `resolveStartDecision` (filesystem/probe/liveness injetados), com `reason` (`forced|missing|unreadable|server-mismatch|running|idle`) para a CLI preservar o aviso de "estado ilegível", e o reclaim do start-lock virou `isStaleLock` (PID morto/ilegível/não-numérico → stale) — o invariante "nunca dois runs no mesmo branch" ganha pin de unit puro (estado vivo→reuse, idle→fresh, `--new`→fresh, URL divergente→fresh, lock morto→reclaim, lock vivo→die); a corrida real entre dois processos permanece documentada como lacuna (o lock segue guard de produção). (F3) o USAGE do `start` ganhou `--hostname/--port` e a nota de `--model` obrigatório quando o purpose tem skill, e o `list` humano exibe a coluna `LOG` (`formatSessionList` acrescenta `logPath` como último campo) — o contrato `list --json`/`sessionListPayload` (OPS109) fica intocado. Unit: `tests/unit/agentSession.unit.spec.ts` +3 describes (allowlist exata, validador/sugestão, decisão de reuso, stale lock) e o pin de `formatSessionList` atualizado. Sem migration/UI/access/Consent/e2e; `pnpm gate:fast` verde (324 files / 3268 tests). Triage dos achados do `/simplify`: 0 registros (3 descartes de pré-existentes/tradeoffs aceitos, 1 defer com gatilho do USAGE×allowlist, 2 já rastreados na intenção). diff --git a/docs/plans/ops110-f1-agent-session-hardening-impl.md b/docs/plans/ops110-f1-agent-session-hardening-impl.md new file mode 100644 index 00000000..690ea4d5 --- /dev/null +++ b/docs/plans/ops110-f1-agent-session-hardening-impl.md @@ -0,0 +1,115 @@ +# Impl: OPS110-F1 — `agent:session`: flags desconhecidas fail-high, single-flight do `start` testado e polish de help/list + +Status: aprovado +Atualizado em: 2026-09-15 +Issue: #1026 +Intenção: docs/plans/ops110-f1-agent-session-hardening.md +Appetite restante: herdado (~1 dia eng, 3 fases fill-in: F1 ~2–3h, F2 ~3–4h, F3 ~1h) + +## Leitura da intenção + +- **Outcome:** fechar os débitos do `/simplify` pós-OPS110 sem tocar o contrato de produto — (a) flag desconhecida em `agent:session` falha alto em vez de cair no default mudo de `cmdStart`; (b) o invariante "nunca dois runs no mesmo branch" ganha pin automatizado da decisão de reuso e do reclaim do lock; (c) USAGE do `start` e `logPath` no `list` humano ficam completos. +- **O que NÃO negociar:** contrato `list --json`/`sessionListPayload` (OPS109) intocado; `parseEqualsFlags` mantém a semântica silenciosa (família auto-unblock depende dela); lib `scripts/lib/agent-session.mjs` permanece PURA (I/O só por injeção); credencial só por env (D9) e fail-closed de bind inalterados; sem migration/UI/access. +- **O que reavaliar:** a hipótese de que "sugestão distância-1" basta — o aceite cita literalmente `--prupose`→`--purpose`, que é uma transposição; Levenshtein puro daria distância 2. Decisão de engenharia abaixo (Damerau × limiar). + +## Abordagem recomendada + +```mermaid +flowchart LR + A[argv] --> B[parseEqualsFlags] + B --> C{subcommand conhecido?} + C -- não --> U[USAGE + exit 1] + C -- sim --> V[validateSessionFlags
whitelist + nearestFlag] + V -- desconhecida --> D[die: sugestão/USAGE] + V -- ok --> S{verbo} + S -- start --> L[acquireStartLock
isStaleLock injetável] + L --> R[resolveStartDecision
readFile/probeBusy/driverAlive injetados] + R -- reuse --> T[attach] + R -- fresh --> N[createSession + spawnDetached] + N --> T + S -- list --> F[formatSessionList + logPath] +``` + +**Opções consideradas:** A) 3 fases curtas no mesmo lote sobre as superfícies já existentes (`scripts/lib/agent-session.mjs` + `scripts/agent-session.mjs` + `tests/unit/agentSession.unit.spec.ts`) | B) só F1 agora, deferir F2/F3 | C) reescrever o CLI em módulos por verbo e embutir tudo. + +**Recomendação:** A — as três fases são cheap_polish na MESMA superfície já extraída na OPS110; F1 é a de maior risco (falha muda no caminho crítico), F2 pina o invariante mais caro de reverter (dois drivers no mesmo worktree) e F3 é cosmético. Toda a lógica nova nasce como função pura na lib (testável sem processo/HTTP) e a CLI fina só injeta I/O — exatamente o desenho atual. + +**Rejeitadas:** B — deixaria sem pin o único invariante de segurança de processo do item e a falha muda (F1) sozinha não fecha o outcome; C — o CLI fino é intencional na OPS110 e o refactor não remove volatilidade, só adiciona cerimônia ([decision-quality.md] "Fronteira de módulo em volatilidade real" / "Pass-through raso"). + +### Decisões de engenharia (caras de reverter → com rejeitadas) + +- **Onde validar flags (F1).** + Opções: A) validador puro novo em `scripts/lib/agent-session.mjs` (`SESSION_FLAG_WHITELIST` + `validateSessionFlags`), chamado pela CLI logo após resolver o verbo | B) endurecer `parseEqualsFlags` para rejeitar desconhecidas | C) rejeitar dentro de cada `cmd*`. + Recomendação: A — mantém a lib testável sem argv real e não muda o parser compartilhado; a CLI já é a dona do `die`/USAGE. + Rejeitadas: B porque `parseEqualsFlags` é contrato silencioso de 5–7 scripts (B5/Pass3) e a intenção proíbe mudar a semântica; C porque espalha a whitelist por verbo e duplica o dispatch. +- **Forma da sugestão (F1).** + Opções: A) distância de edição com transposição (Damerau) e limiar ≤1 (`nearestAcceptedFlag`) | B) Levenshtein puro com limiar ≤2 | C) sem sugestão. + Recomendação: A — cobre `--prupose` (transposição = 1) mantendo limiar estrito; menos falso-positivo que ≤2. + Rejeitadas: B porque limiar 2 sugere em substituições duplas legítimas (ruído acionável falso); C porque o aceite exige mensagem acionável. +- **Seam do single-flight (F2).** + Opções: A) extrair `resolveStartDecision` (async, puro) + `isStaleLock` (puro) com `readFile`/`probeBusy`/`driverAlive`/`forceNew` injetados; CLI mantém `openSync`/`unlinkSync`/HTTP | B) mover fs/probe para a lib | C) testar via integração spawnando processos reais. + Recomendação: A — pina a decisão sem violar a pureza da lib e sem flakiness de processo. + Rejeitadas: B porque quebra o contrato "lib pura" (cabeçalho do módulo) e a fronteira CLI/servidor; C porque acopla a suíte ao SO/`/proc` e não isola a regra. +- **Sugestão no `list` humano (F3).** + Opções: A) acrescentar `logPath` como última coluna em `formatSessionList` + header da CLI | B) coluna conforme/`logPath` só no `--json`. + Recomendação: A — o dado já existe no estado (spread do payload) e o pedido é enxergá-lo no humano. + Rejeitadas: B porque `--json` já o carrega; não fecha o outcome. + +### Componentes / mudanças + +- **`validateSessionFlags` / `SESSION_FLAG_WHITELIST` / `nearestAcceptedFlag`** (`scripts/lib/agent-session.mjs`): puros. Whitelist por verbo (nomes, valores irrelevantes): `serve`→{hostname,port}; `start`→{purpose,dir,model,issue,argument,new,hostname,port}; `attach`/`stop`→{session,branch,issue}; `list`→{json}. Retorna/`throw` mensagem acionável (`sugestão: --purpose` ou lista do USAGE). Reusa o padrão de whitelist manual fail-high já existente em `scripts/lib/imageFill.mjs:70-72`. +- **`resolveStartDecision` + `isStaleLock`** (`scripts/lib/agent-session.mjs`): `resolveStartDecision({ forceNew, statePath, readFile, serverUrl, probeBusy, driverAlive })` async puro → `{ action: 'reuse'|'fresh', state? }`; preserva a semântica atual de `readReusable` (`scripts/agent-session.mjs:480-494`): `--new`→fresh; estado ilegível→fresh (warn fica na CLI); `state.url !== serverUrl`→fresh; vivo se `driverAlive || busy`. `isStaleLock({ holderPid, isPidAlive })` puro encapsula o ramo de reclaim (`:420-435`). +- **`scripts/agent-session.mjs`**: USAGE do `start`/`serve` completado (F3); chamada de `validateSessionFlags` no dispatch (`:655-674`); `cmdStart` (`:458-549`) passa a delegar a decisão; `acquireStartLock` (`:400-442`) usa `isStaleLock`; header do `list` (`:640`) ganha `LOG` e o repasse de `logPath`. +- **`formatSessionList`** (`scripts/lib/agent-session.mjs:318-323`): acrescenta `row.logPath` como último campo; `sessionListPayload` (`:308-315`) e o contrato OPS109 NÃO são tocados. Header literal da CLI alinhado. +- **Migration:** sem migration. +- **Access / Consent:** N/A (não toca collections/PII). +- **UI:** N/A (texto de CLI; apenas strings pt-BR). + +### Dados → forma (se aplicável) + +- N/A — não há dados/KPI/mapa/série; a saída é texto de terminal e o contrato JSON versionado já existente não muda. + +## Fases verificáveis + +1. **F1 — Flags estritas (tracer, ~2–3h).** Adicionar whitelist + `nearestAcceptedFlag` + `validateSessionFlags` na lib; chamar no dispatch só para verbos conhecidos (desconhecido segue → USAGE, `:671-673`). Unit novo cobre: `--prupose=next`→sugere `--purpose`; `--foo` sem vizinho→mensagem com USAGE; booleanos `--json`/`--new` aceitos; `serve --purpose=x` rejeitado; verbo desconhecido continua USAGE+exit 1. `pnpm gate:fast`. +2. **F2 — Single-flight com seam testável (~3–4h).** Extrair `resolveStartDecision`/`isStaleLock`; `cmdStart` e `acquireStartLock` passam a injetar as dependências reais sem mudar comportamento. Unit cobre: estado vivo (driverAlive/busy)→reuse; URL divergente→fresh; estado ilegível→fresh; `--new`→fresh; lock com PID morto→reclaim; lock com PID vivo→`die`. Comentar no spec a lacuna que fica (corrida real entre processos) como não-escopo. `pnpm gate:fast`. +3. **F3 — Help/list (~1h).** USAGE do `start` com `--hostname/--port` e "`--model` obrigatório quando há skill"; `formatSessionList` + header com `logPath`. Atualizar spec de `:462-489`. `pnpm gate:fast`. +4. **Gates finais.** `pnpm gate:fast` verde (lint → typecheck → unit) e push do branch via `pnpm push`. + +## Rabbit holes / Não escopo (engenharia) + +- Refactor do CLI em módulos por verbo; a CLI fina é intencional (OPS110). +- Mudar `parseEqualsFlags`/aceitar `--flag valor` — contrato de outros scripts. +- Sugestão por Levenshtein sem limiar ou "fuzzy" amplo — ruído acionável. +- Mover fs/HTTP/`process.kill` para a lib (quebraria a pureza). +- Teste de integração spawnando dois `start` reais — flaky, depende de `/proc`/porta. +- Validar semanticamente valores de flag (e-mail de branch, range de issue) — fora. +- Unificar a fronteira dupla `purposeInvocation` (`scripts/lib/worktree.mjs` ∥ lib) — adiada com gatilho na intenção. +- Tocar `sessionListPayload`/contrato `list --json` ou o painel OPS109. + +### Débitos do `/simplify` (triage OPS110-F1 — 0 registros) + +- **Allowlist × texto do USAGE** (lib ⟥ CLI): duas fontes do mesmo conjunto; o spec pina a allowlist exata, o USAGE textual não. Não criar pin agora — **gatilho:** ao adicionar/remover flag, derivar/pinar o USAGE. (cheap_polish, defer) +- **`parseEqualsFlags` e `--`**: pré-existente no parser compartilhado de 5–7 scripts; fora de escopo (não mudar a semântica). (cheap_polish, descartado) +- **`osaDistance` O(n·m)**: tradeoff aceito — strings de flag curtas, caminho só de erro, sem dep de Levenshtein no `package.json`. (cheap_polish, descartado) +- **`isStaleLock` com 1 call site**: seam de teste deliberado (pina o reclaim sem processo real), não reuso perdido. (cheap_polish, descartado) + +## Riscos e mitigação + +- **Whitelist divergir do parser:** a whitelist valida NOMES, não a forma `=`; testes pinam booleanos `--json`/`--new` e o `--flag` sem valor. Nenhuma mudança em `parseEqualsFlags`. +- **Sugestão errada em `--prupose`:** Damerau com limiar ≤1 cobre a transposição exigida pelo aceite; casos sem vizinho caem no USAGE. +- **F2 alterar comportamento ao extrair:** testes espelham cada ramo atual de `readReusable` (URL, ilegível, busy, driver morto) e o reclaim do lock (PID morto/vivo/ilegível); `--new` curto-circuita. +- **Quebrar a pureza da lib:** revisor de import — proibido `node:fs`/`fetch`/`child_process` novos na lib; I/O só por parâmetro. +- **Regressão no `list`:** `formatSessionList` é consumido só pela CLI e pelo spec; payload OPS109 intocado (spread já traz `logPath`). +- **Corrida real de dois processos:** permanece fora do teste (lacuna documentada no spec) — o lock continua sendo o guard de produção. + +## Aceite de engenharia + +- [ ] Flag desconhecida em qualquer verbo falha com mensagem acionável (inclui `--prupose`→`--purpose`); verbo desconhecido segue USAGE+exit 1. +- [ ] `resolveStartDecision` + `isStaleLock` pinados em unit puro (reuse/fresh/`--new`/reclaim); corrida real documentada como lacuna. +- [ ] USAGE do `start` completo (`--hostname/--port`, `--model` obrigatório) e `list` humano exibindo `logPath`. +- [ ] Contrato `list --json`/`sessionListPayload` (OPS109) e `parseEqualsFlags` inalterados; lib mantida pura; identificadores em inglês, strings em pt-BR. +- [ ] `pnpm gate:fast` verde; sem migration/UI/access. +- [ ] Invariantes AGENTS/engineering-standards: nenhuma collection/PII/transação envolvida; nada reescrito como twin (edita-se o dono existente). + +**Self-score decision-quality (gate ≥4):** decisões caras (onde validar, forma da sugestão, seam do lock, coluna do list) têm Opções A/B/C + Recomendação + Rejeitadas (1,0); cabe no appetite ~1 dia em 3 fases (1,0); rabbit holes nomeados (1,0); depth check reusa a lib/CLI e o padrão de whitelist existentes, sem twin (1,0); o aceite de produto da intenção permanece coberto e o contrato OPS110/OPS109 preservado (1,0) → **5,0/5**. diff --git a/scripts/agent-session.mjs b/scripts/agent-session.mjs index 80fc000c..622de53a 100644 --- a/scripts/agent-session.mjs +++ b/scripts/agent-session.mjs @@ -6,7 +6,7 @@ * Sobe (ou reaproveita) o `opencode serve` COMPARTILHADO em loopback — * ele é o dono das sessões e sobrevive a qualquer cliente. Bind fora do * loopback exige `OPENCODE_SERVER_PASSWORD` (fail-closed). Nunca `--mdns`. - * pnpm agent:session start --purpose= --dir= --model= [--issue=N] [--argument=""] [--new] + * pnpm agent:session start --purpose= --dir= [--model=] [--issue=N] [--argument=""] [--new] [--hostname=H] [--port=P] * Cria a sessão endereçável, sobe o driver destacado * (`opencode run --attach … --auto --command -- …`) quando o * purpose tem skill, e abre o TUI anexado (`opencode attach -s `). @@ -18,14 +18,15 @@ * ele estiver fora (o histórico da sessão persiste). * pnpm agent:session stop [--session=|--branch=|--issue=N] * Ato explícito de encerrar: abort via API + fim do driver + `stoppedAt`. - * pnpm agent:session list [--json] + * pnpm agent:session list [--json] [--hostname=H] [--port=P] * Estado dos runs (orquestrado pelo `serve`); `--json` é o contrato * versionado que o painel OPS109 consome. * * Estado por run em `~/.local/state/teqo/agent-sessions/.json` (índice de * endereçamento: branch/dir/sessionID/url/driverPid/log); nenhuma credencial é * persistida. Os fatos de semântica (cliente pode morrer, `abort` interrompe, - * `--` entrega `$ARGUMENTS`) foram verificados na 1.18.31. + * `--` entrega `$ARGUMENTS`) foram verificados na 1.18.31. Flag desconhecida + * falha alto por verbo (OPS110-F1) — nunca cai num default mudo. */ import { spawn, spawnSync } from 'node:child_process' import { @@ -47,11 +48,13 @@ import { driverArgs, driverLogPath, formatSessionList, + isStaleLock, parseServerState, parseSessionState, purposeInvocation, resolveServerConfig, resolveSessionRef, + resolveStartDecision, serializeServerState, serializeSessionState, serverArgs, @@ -62,6 +65,7 @@ import { startLockPath, STATE_VERSION, validateServerBind, + validateSessionFlags, } from './lib/agent-session.mjs' import { dieWithLabel, parseEqualsFlags } from './lib/cli.mjs' @@ -70,11 +74,13 @@ const die = dieWithLabel('agent:session') const USAGE = `Uso: pnpm agent:session [flags] serve [--hostname=H] [--port=P] sobe/reaproveita o opencode serve compartilhado - start --purpose= --dir= --model= [--issue=N] [--argument=""] [--new] + start --purpose= --dir= [--model=] [--issue=N] [--argument=""] [--new] [--hostname=H] [--port=P] cria a sessão, sobe o driver destacado e anexa o TUI attach [--session=|--branch=|--issue=N] (re)entra na sessão (default: branch do cwd) stop [--session=|--branch=|--issue=N] encerra de forma explícita (abort + fim do driver) - list [--json] status dos runs registrados` + list [--json] [--hostname=H] [--port=P] status dos runs registrados + + --model é obrigatório quando o purpose tem skill (next/plan/fix); 'new' não dispara driver.` /** Loopback by default; the credential rides the env, never the state/log. */ const serverPassword = () => process.env.OPENCODE_SERVER_PASSWORD ?? '' @@ -419,13 +425,13 @@ const acquireStartLock = ({ sessionDir, branch }) => { } } catch (error) { if (error?.code !== 'EEXIST') throw error - let holder = null + let holder = NaN try { holder = Number(readFileSync(lockPath, 'utf8').trim()) } catch { - // unreadable lock — treat as stale below + // unreadable lock — treated as stale by isStaleLock } - if (!pidAlive(holder)) { + if (isStaleLock({ holderPid: holder, isPidAlive: pidAlive })) { try { unlinkSync(lockPath) } catch { @@ -477,21 +483,17 @@ const cmdStart = async (flags) => { warnVersionDrift(server.version) const statePath = sessionStatePath({ sessionDir, branch }) - const readReusable = async () => { - if (flags.new || !existsSync(statePath)) return null - let existing - try { - existing = parseSessionState(readFileSync(statePath, 'utf8')) - } catch (error) { - console.warn( - `[agent:session] estado ilegível em ${statePath} (${error.message}) — tratando como sessão nova.`, - ) - return null - } - if (existing.url !== server.url) return null - const busy = (await sessionStatuses(server.url))[existing.sessionID]?.type === 'busy' - return driverAlive(existing.driverPid, existing.sessionID) || busy ? existing : null - } + const decideReuse = () => + resolveStartDecision({ + forceNew: Boolean(flags.new), + statePath, + readFile: (path) => readFileSync(path, 'utf8'), + exists: existsSync, + serverUrl: server.url, + probeBusy: async (sessionID) => + (await sessionStatuses(server.url))[sessionID]?.type === 'busy', + driverAlive, + }) // A decisão de reuso acontece DENTRO do lock: dois `start`s concorrentes do // mesmo branch não podem ambos ler "não vivo" e criar dois drivers/sessões. @@ -499,10 +501,15 @@ const cmdStart = async (flags) => { let state let reused = false try { - const existing = await readReusable() - if (existing) { + const decision = await decideReuse() + if (decision.reason === 'unreadable') { + console.warn( + `[agent:session] estado ilegível em ${statePath} (${decision.error?.message}) — tratando como sessão nova.`, + ) + } + if (decision.action === 'reuse') { reused = true - state = existing + state = decision.state } else { const sessionID = await createSession(server.url, dir) state = { @@ -637,7 +644,7 @@ const cmdList = async (flags) => { console.log(`nenhuma sessão registrada em ${sessionDir}`) return } - console.log('STATUS REF SESSÃO BRANCH DIR') + console.log('STATUS REF SESSÃO BRANCH DIR LOG') for (const line of formatSessionList( states.map((state) => ({ ...state, status: derived[state.sessionID] })), )) { @@ -661,6 +668,11 @@ try { `argumento posicional inesperado: ${positional.slice(1).join(' ')} (use --flag=valor; veja o USAGE).`, ) } + try { + validateSessionFlags({ subcommand, flags }) + } catch (error) { + die(`${error.message} Veja o USAGE: \`pnpm agent:session\`.`) + } if (subcommand === 'serve') await cmdServe(flags) else if (subcommand === 'start') { process.exitCode = await cmdStart(flags) diff --git a/scripts/lib/agent-session.mjs b/scripts/lib/agent-session.mjs index 214ad9d0..b73dc497 100644 --- a/scripts/lib/agent-session.mjs +++ b/scripts/lib/agent-session.mjs @@ -198,6 +198,84 @@ export const purposeInvocation = ({ purpose = null, issueNumber = null, argument return { command, arguments: '' } } +/** + * Accepted flag NAMES per subcommand (OPS110-F1). Names only — the `=`/boolean + * form stays `parseEqualsFlags`' concern. `list`/`start`/`serve` legitimately + * take `--hostname/--port` (they override the probe/spawn target). An unknown + * name must fail high: `--prupose=next` used to fall through to the `new` + * default and create a driverless session. + */ +export const SESSION_FLAG_ALLOWLIST = { + serve: ['hostname', 'port'], + start: ['purpose', 'dir', 'model', 'issue', 'argument', 'new', 'hostname', 'port'], + attach: ['session', 'branch', 'issue'], + stop: ['session', 'branch', 'issue'], + list: ['json', 'hostname', 'port'], +} + +/** Optimal string alignment (Damerau–Levenshtein, no adjacent transposition twice). */ +const osaDistance = (left, right) => { + const a = String(left ?? '') + const b = String(right ?? '') + if (a === b) return 0 + if (a.length === 0) return b.length + if (b.length === 0) return a.length + const rows = Array.from({ length: a.length + 1 }, () => new Array(b.length + 1).fill(0)) + for (let i = 0; i <= a.length; i += 1) rows[i][0] = i + for (let j = 0; j <= b.length; j += 1) rows[0][j] = j + for (let i = 1; i <= a.length; i += 1) { + for (let j = 1; j <= b.length; j += 1) { + const cost = a[i - 1] === b[j - 1] ? 0 : 1 + rows[i][j] = Math.min(rows[i - 1][j] + 1, rows[i][j - 1] + 1, rows[i - 1][j - 1] + cost) + if (i > 1 && j > 1 && a[i - 1] === b[j - 2] && a[i - 2] === b[j - 1]) { + rows[i][j] = Math.min(rows[i][j], rows[i - 2][j - 2] + 1) + } + } + } + return rows[a.length][b.length] +} + +/** + * Closest accepted flag at edit distance ≤1, or null. Damerau (transposition + * counts as 1) so the required `--prupose` → `--purpose` suggestion fires; + * limiar estrito evita sugerir em erros grandes (ruído acionável falso). + * @param {string} name + * @param {readonly string[]} accepted + */ +export const nearestAcceptedFlag = (name, accepted) => { + let best = null + let bestDistance = Number.POSITIVE_INFINITY + for (const candidate of accepted) { + const distance = osaDistance(name, candidate) + if (distance < bestDistance || (distance === bestDistance && candidate < String(best))) { + best = candidate + bestDistance = distance + } + } + return bestDistance <= 1 ? best : null +} + +/** + * Fail high on any flag the subcommand does not accept. Throws a message with + * a suggestion when a near neighbor exists; unknown subcommands return silently + * (the CLI prints the USAGE for those). A bare `--` parses to an empty name and + * is ignored, not reported as a typo. + * @param {{ subcommand?: string | null, flags?: Record }} [options] + */ +export const validateSessionFlags = ({ subcommand = null, flags = {} } = {}) => { + const accepted = SESSION_FLAG_ALLOWLIST[subcommand] + if (!accepted) return + for (const name of Object.keys(flags)) { + if (name === '' || accepted.includes(name)) continue + const suggestion = nearestAcceptedFlag(name, accepted) + throw new Error( + suggestion + ? `flag desconhecida: --${name} (sugestão: --${suggestion}).` + : `flag desconhecida: --${name}.`, + ) + } +} + /** * argv of the detached run driver (sem o binário — o caller usa * `spawn('opencode', …)`). The `--` separator is required so a command @@ -289,6 +367,63 @@ export const resolveSessionRef = ({ return null } +/** + * Decide "reuse the existing run vs create a fresh one" — the single-flight + * invariant's brain (OPS110-F1). Pure: the caller injects the filesystem read, + * the busy probe and the driver liveness check. Mirrors `cmdStart`'s in-lock + * decision: `--new`/no state/unreadable state/different server → fresh; an + * alive driver or a busy session on the SAME server → reuse. `reason` lets the + * CLI keep its "estado ilegível" warning without the lib doing I/O. + * @param {{ + * forceNew?: boolean, + * statePath: string, + * readFile: (path: string) => string, + * exists: (path: string) => boolean, + * parse?: (text: string) => Record, + * serverUrl: string, + * probeBusy: (sessionID: string) => boolean | Promise, + * driverAlive?: (pid: number | null, sessionID: string | null) => boolean, + * }} options + * @returns {Promise<{ action: 'reuse' | 'fresh', reason: 'forced' | 'missing' | 'unreadable' | 'server-mismatch' | 'running' | 'idle', state?: Record, error?: Error }>} + */ +export const resolveStartDecision = async ({ + forceNew = false, + statePath, + readFile, + exists, + parse = parseSessionState, + serverUrl, + probeBusy, + driverAlive = () => false, +}) => { + if (forceNew) return { action: 'fresh', reason: 'forced' } + if (!exists(statePath)) return { action: 'fresh', reason: 'missing' } + let existing + try { + existing = parse(readFile(statePath)) + } catch (error) { + return { action: 'fresh', reason: 'unreadable', error } + } + if (existing.url !== serverUrl) return { action: 'fresh', reason: 'server-mismatch' } + const busy = Boolean(await probeBusy(existing.sessionID)) + if (driverAlive(existing.driverPid, existing.sessionID) || busy) { + return { action: 'reuse', reason: 'running', state: existing } + } + return { action: 'fresh', reason: 'idle' } +} + +/** + * Whether an existing start-lock's holder is gone and the lock can be + * reclaimed. Pure: PID liveness is injected. A missing/non-numeric/dead holder + * counts as stale — the same branch the CLI used to inline. + * @param {{ holderPid?: number | string | null, isPidAlive: (pid: number) => boolean }} options + */ +export const isStaleLock = ({ holderPid, isPidAlive }) => { + const pid = Number(holderPid) + if (!Number.isInteger(pid) || pid <= 0) return true + return !isPidAlive(pid) +} + /** * Status shown by `list`: `stopped` (explicit stop) wins, then `unknown` * (server unreachable), then `working` (busy on the server or driver alive), @@ -314,10 +449,10 @@ export const sessionListPayload = ({ states, statuses = {}, server }) => ({ })), }) -/** Human `list` lines — one per run, status first, `—` for namespace runs. */ +/** Human `list` lines — status first, `—` for namespace runs; `logPath` last. */ export const formatSessionList = (rows) => (Array.isArray(rows) ? rows : []).map((row) => { const reference = [row.code, row.issue ? `#${row.issue}` : null].filter(Boolean).join(' ') || '—' - return `${String(row.status).padEnd(8)} ${reference} ${row.sessionID} ${row.branch} ${row.dir}` + return `${String(row.status).padEnd(8)} ${reference} ${row.sessionID} ${row.branch} ${row.dir} ${row.logPath ?? '—'}` }) diff --git a/tests/unit/agentSession.unit.spec.ts b/tests/unit/agentSession.unit.spec.ts index 42412112..fb1ec7f4 100644 --- a/tests/unit/agentSession.unit.spec.ts +++ b/tests/unit/agentSession.unit.spec.ts @@ -12,16 +12,20 @@ import { driverLogPath, formatSessionList, isLoopbackHost, + isStaleLock, + nearestAcceptedFlag, parseServerState, parseSessionState, purposeInvocation, resolveServerConfig, resolveSessionRef, + resolveStartDecision, serializeServerState, serializeSessionState, serverArgs, serverStatePath, SESSION_COMMAND_BY_PURPOSE, + SESSION_FLAG_ALLOWLIST, sessionDirFromEnv, sessionListPayload, sessionSlug, @@ -29,6 +33,7 @@ import { startLockPath, STATE_VERSION, validateServerBind, + validateSessionFlags, } from '../../scripts/lib/agent-session.mjs' const state = (over: Record = {}) => ({ @@ -239,6 +244,91 @@ describe('purposeInvocation (mapa purpose→comando movido do worktree)', () => }) }) +describe('validateSessionFlags + nearestAcceptedFlag (OPS110-F1 — flag desconhecida falha alto)', () => { + it('pins the accepted flag names per verb', () => { + expect(SESSION_FLAG_ALLOWLIST).toEqual({ + serve: ['hostname', 'port'], + start: ['purpose', 'dir', 'model', 'issue', 'argument', 'new', 'hostname', 'port'], + attach: ['session', 'branch', 'issue'], + stop: ['session', 'branch', 'issue'], + list: ['json', 'hostname', 'port'], + }) + }) + + it('accepts every documented flag per verb (names only, valued or boolean)', () => { + expect(() => + validateSessionFlags({ + subcommand: 'start', + flags: { + purpose: 'next', + dir: '/d', + model: 'm', + issue: '1', + argument: 'x', + new: true, + hostname: 'h', + port: '1', + }, + }), + ).not.toThrow() + expect(() => + validateSessionFlags({ subcommand: 'serve', flags: { hostname: 'h', port: '1' } }), + ).not.toThrow() + expect(() => + validateSessionFlags({ subcommand: 'list', flags: { json: true, hostname: 'h', port: '1' } }), + ).not.toThrow() + expect(() => + validateSessionFlags({ subcommand: 'attach', flags: { session: 'ses_x' } }), + ).not.toThrow() + expect(() => validateSessionFlags({ subcommand: 'stop', flags: { branch: 'b' } })).not.toThrow() + }) + + it('suggests the transposition neighbor: --prupose → --purpose', () => { + expect(nearestAcceptedFlag('prupose', ['purpose', 'dir', 'model'])).toBe('purpose') + expect(() => validateSessionFlags({ subcommand: 'start', flags: { prupose: 'next' } })).toThrow( + /--prupose[\s\S]*--purpose/, + ) + }) + + it('suggests substitution/insertion neighbors at distance 1, nothing at distance 2', () => { + expect(nearestAcceptedFlag('purposee', ['purpose'])).toBe('purpose') + expect(nearestAcceptedFlag('purpos', ['purpose'])).toBe('purpose') + expect(nearestAcceptedFlag('prpose', ['purpose'])).toBe('purpose') + expect(nearestAcceptedFlag('purposxx', ['purpose'])).toBeNull() + expect(nearestAcceptedFlag('totallyWrong', ['purpose', 'dir'])).toBeNull() + }) + + it('reports an unknown flag without a suggestion as-is', () => { + expect(() => + validateSessionFlags({ subcommand: 'start', flags: { totallyWrong: true } }), + ).toThrow('flag desconhecida: --totallyWrong.') + }) + + it('rejects a flag that belongs to another verb', () => { + expect(() => validateSessionFlags({ subcommand: 'serve', flags: { purpose: 'next' } })).toThrow( + /--purpose/, + ) + expect(() => validateSessionFlags({ subcommand: 'list', flags: { dir: '/d' } })).toThrow( + /--dir/, + ) + }) + + it('ignores a bare `--` (empty flag name), never a typo', () => { + expect(() => + validateSessionFlags({ subcommand: 'start', flags: { '': true, purpose: 'next' } }), + ).not.toThrow() + }) + + it('stays silent for an unknown verb (the CLI prints the USAGE itself)', () => { + expect(() => + validateSessionFlags({ subcommand: 'bogus', flags: { whatever: true } }), + ).not.toThrow() + expect(() => + validateSessionFlags({ subcommand: null, flags: { whatever: true } }), + ).not.toThrow() + }) +}) + describe('driverArgs + attachArgs (o argv verificado ao vivo)', () => { it('drives the run through the server with --auto and --command + `--` separator', () => { expect( @@ -385,6 +475,93 @@ describe('resolveSessionRef (--session > --branch > --issue > branch do cwd)', ( }) }) +describe('resolveStartDecision (OPS110-F1 — single-flight "reusar vs criar")', () => { + const liveState = state() + const base = { + statePath: '/state/OPS110.json', + serverUrl: liveState.url, + readFile: () => JSON.stringify(liveState), + exists: () => true, + probeBusy: async () => false, + driverAlive: () => false, + } + + it('reuses when the driver is alive on the same server', async () => { + expect(await resolveStartDecision({ ...base, driverAlive: () => true })).toMatchObject({ + action: 'reuse', + reason: 'running', + state: liveState, + }) + }) + + it('reuses when the session is busy even with the driver gone', async () => { + expect(await resolveStartDecision({ ...base, probeBusy: async () => true })).toMatchObject({ + action: 'reuse', + reason: 'running', + }) + }) + + it('creates fresh when idle (driver dead and not busy)', async () => { + expect(await resolveStartDecision(base)).toMatchObject({ action: 'fresh', reason: 'idle' }) + }) + + it('--new forces fresh without reading the state', async () => { + let read = false + const decision = await resolveStartDecision({ + ...base, + forceNew: true, + readFile: () => { + read = true + return 'ignored' + }, + }) + expect(decision).toMatchObject({ action: 'fresh', reason: 'forced' }) + expect(read).toBe(false) + }) + + it('creates fresh when there is no state file', async () => { + expect(await resolveStartDecision({ ...base, exists: () => false })).toMatchObject({ + action: 'fresh', + reason: 'missing', + }) + }) + + it('creates fresh on unreadable state and surfaces the error for the CLI warning', async () => { + const decision = await resolveStartDecision({ + ...base, + readFile: () => { + throw new Error('boom') + }, + }) + expect(decision.action).toBe('fresh') + expect(decision.reason).toBe('unreadable') + expect(decision.error?.message).toBe('boom') + }) + + it('creates fresh when the state points at another server', async () => { + expect(await resolveStartDecision({ ...base, serverUrl: 'http://other:1' })).toMatchObject({ + action: 'fresh', + reason: 'server-mismatch', + }) + }) +}) + +describe('isStaleLock (OPS110-F1 — reclaim do lock com PID morto)', () => { + it('treats a dead PID as stale', () => { + expect(isStaleLock({ holderPid: 4242, isPidAlive: () => false })).toBe(true) + }) + + it('keeps a live holder (not stale)', () => { + expect(isStaleLock({ holderPid: 4242, isPidAlive: () => true })).toBe(false) + }) + + it('treats a missing/non-numeric holder as stale (never deadlocks forever)', () => { + expect(isStaleLock({ holderPid: NaN, isPidAlive: () => true })).toBe(true) + expect(isStaleLock({ holderPid: null, isPidAlive: () => true })).toBe(true) + expect(isStaleLock({ holderPid: 0, isPidAlive: () => true })).toBe(true) + }) +}) + describe('deriveSessionStatus + list (contrato OPS109)', () => { it('stopped > unknown (server down) > working (busy or driver alive) > idle', () => { const live = state() @@ -459,7 +636,7 @@ describe('deriveSessionStatus + list (contrato OPS109)', () => { expect(payload.sessions[0].status).toBe('unknown') }) - it('human list prints one line per run with status/session/branch/dir', () => { + it('human list prints one line per run with status/session/branch/dir/log', () => { const lines = formatSessionList([ { status: 'working', @@ -468,6 +645,7 @@ describe('deriveSessionStatus + list (contrato OPS109)', () => { branch: 'OPS110-acompanhar-runs', sessionID: 'ses_abc', dir: '/work/OPS110', + logPath: '/state/ops110-acompanhar-runs.log', }, { status: 'idle', @@ -484,7 +662,9 @@ describe('deriveSessionStatus + list (contrato OPS109)', () => { expect(lines[0]).toContain('#1019') expect(lines[0]).toContain('ses_abc') expect(lines[0]).toContain('/work/OPS110') + expect(lines[0]).toContain('/state/ops110-acompanhar-runs.log') expect(lines[1]).toContain('plans/plan-issue-1') expect(lines[1]).not.toContain('#null') + expect(lines[1].endsWith('—')).toBe(true) }) })