From cf3a41fe4442c5e8ac80e9c61c665de6b3b72da9 Mon Sep 17 00:00:00 2001 From: Romuald Priol Date: Mon, 13 Jul 2026 16:22:27 +0200 Subject: [PATCH] Fix Run lifeCycle Signed-off-by: Romuald Priol --- .github/workflows/ci-test.yml | 15 +- .pnpm-store/v11/index.db | Bin 0 -> 4096 bytes .pnpm-store/v11/index.db-shm | Bin 0 -> 32768 bytes .pnpm-store/v11/index.db-wal | Bin 0 -> 8272 bytes CHANGELOG.md | 28 +- Makefile | 9 +- README.fr.md | 2 +- README.md | 4 +- backend/scripts/disc-introspection-mcp.py | 30 +- backend/src/agents/runner.rs | 29 +- backend/src/agents/runner_test.rs | 41 +- backend/src/api/disc_invite.rs | 89 +- backend/src/api/disc_source.rs | 193 +- backend/src/api/federation.rs | 27 +- backend/src/api/learnings.rs | 6 +- backend/src/api/mcp_remote.rs | 11 +- backend/src/api/mcps.rs | 3 +- backend/src/api/setup.rs | 256 +- backend/src/api/workflows.rs | 70 + backend/src/api_tests.rs | 27 + backend/src/core/config.rs | 37 +- backend/src/db/contacts.rs | 48 + backend/src/db/learnings.rs | 28 + backend/src/db/quick_prompts.rs | 48 + backend/src/db/tests.rs | 154 + backend/src/db/workflows.rs | 181 +- backend/src/lib.rs | 99 +- backend/src/main.rs | 1 + backend/src/models/db.rs | 11 +- backend/src/models/learnings.rs | 12 + backend/src/models/workflows.rs | 30 + backend/src/workflows/api_call_executor.rs | 244 +- backend/src/workflows/api_call_step.rs | 3 +- backend/src/workflows/runner.rs | 265 +- backend/src/workflows/steps.rs | 6 +- backend/src/workflows/sub_workflow_step.rs | 266 +- backend/src/workflows/template.rs | 131 +- backend/tests/api_tests.rs | 164 +- backend/tests/config_write_guard.rs | 11 + backend/tests/learnings_api.rs | 12 + docs/design/adr-001-db-connection-model.md | 107 + ...20260630-apicall-link-header-pagination.md | 6 + ...TD-20260701-typegen-generated-aggregate.md | 6 + .../TD-20260713-typescript-7-native.md | 10 + frontend/scripts/assemble-generated-types.mjs | 89 + frontend/scripts/check-types-drift.mjs | 205 - .../__tests__/ActiveAuditsPopover.test.tsx | 2 +- .../ChatHeader.pendingFiles.test.tsx | 2 +- .../ChatHeader.profileEditor.test.tsx | 2 +- .../DiscussionSidebar.grouping.test.tsx | 6 +- .../DiscussionSidebar.markAllRead.test.tsx | 2 +- .../DiscussionSidebar.sourceBadge.test.tsx | 2 +- .../__tests__/HostSyncPreview.test.tsx | 2 +- .../__tests__/MessageBubble.lintPill.test.tsx | 77 +- .../__tests__/NewDiscussionForm.test.tsx | 4 +- .../ProjectCard.audit-resume.test.tsx | 2 +- .../ProjectCard.header-a11y.test.tsx | 2 +- .../__tests__/ProjectCard.migration.test.tsx | 2 +- .../__tests__/ProjectCard.remap.test.tsx | 2 +- .../ProjectList.missing-path.test.tsx | 2 +- .../__tests__/TestModeBanner.test.tsx | 2 +- .../src/components/settings/AgentsSection.tsx | 10 +- .../components/workflows/ApiCallStepCard.tsx | 5 +- .../src/components/workflows/QuickApiForm.tsx | 3 +- .../src/components/workflows/RunDetail.tsx | 15 +- .../components/workflows/WorkflowDetail.tsx | 13 + .../components/workflows/WorkflowWizard.tsx | 1 + .../QuickPromptForm.bindings.test.tsx | 1 + .../workflows/__tests__/RunDetail.test.tsx | 3 + .../WorkflowDetail.liveStepWaitingKey.test.ts | 2 +- .../WorkflowWizard.coverage.test.tsx | 4 +- .../__tests__/WorkflowWizard.test.tsx | 2 +- .../workflows/apiCallPlaceholders.ts | 5 +- .../src/lib/__tests__/batchTriage.test.ts | 2 +- frontend/src/lib/__tests__/regression.test.ts | 3 +- frontend/src/lib/__tests__/runFilters.test.ts | 12 +- frontend/src/lib/api.ts | 6 + frontend/src/lib/i18n.ts | 6 + frontend/src/pages/Dashboard.tsx | 2 +- frontend/src/pages/WorkflowsPage.tsx | 3 +- .../src/pages/__tests__/Dashboard.test.tsx | 3 +- .../pages/__tests__/DiscussionsPage.test.tsx | 12 +- frontend/src/pages/__tests__/McpPage.test.tsx | 96 +- .../WorkflowsPage.qp-launch.test.tsx | 8 +- .../pages/__tests__/WorkflowsPage.test.tsx | 4 +- .../types/__tests__/generated-hygiene.test.ts | 33 + .../types/__tests__/typegen-transform.test.ts | 118 + frontend/src/types/generated.ts | 5433 +++++++++++------ site/en.html | 6 +- site/es.html | 6 +- site/index.html | 6 +- 91 files changed, 6491 insertions(+), 2457 deletions(-) create mode 100644 .pnpm-store/v11/index.db create mode 100644 .pnpm-store/v11/index.db-shm create mode 100644 .pnpm-store/v11/index.db-wal create mode 100644 backend/tests/config_write_guard.rs create mode 100644 docs/design/adr-001-db-connection-model.md create mode 100644 docs/tech-debt/TD-20260713-typescript-7-native.md delete mode 100644 frontend/scripts/check-types-drift.mjs create mode 100644 frontend/src/types/__tests__/generated-hygiene.test.ts create mode 100644 frontend/src/types/__tests__/typegen-transform.test.ts diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index 7fae51aa..ea4ac89b 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -154,16 +154,19 @@ jobs: # --locked so a drift in Cargo.lock vs Cargo.toml also fails here, # not just at release. - # 0.8.11 (C8) — typegen drift guard. Regenerate the per-type ts-rs - # bindings from the Rust models, then fail if a frontend-declared type in - # generated.ts is missing a field the Rust model has (the recurring - # "added a Rust field, forgot generated.ts" drift — hit 4× in 0.8.x). - - name: Types — ts-rs bindings + drift guard + # C1 (plan 0.9) — typegen drift guard. `generated.ts` is now FULLY + # regenerated from the ts-rs bindings; a PR that changes a Rust model + # without running `make typegen` fails here on the diff. + - name: Types — regenerate generated.ts and fail on drift # NOTE: this job's default working-directory is `backend` (see defaults # at the top of the job) — paths below are relative to it. run: | cargo test export_bindings -- --nocapture - node ../frontend/scripts/check-types-drift.mjs + node ../frontend/scripts/assemble-generated-types.mjs + git diff --exit-code ../frontend/src/types/generated.ts || { + echo "::error::frontend/src/types/generated.ts drifted from the Rust models — run 'make typegen' and commit the result." + exit 1 + } # ── Anti-regression lints ───────────────────────────────────────── # diff --git a/.pnpm-store/v11/index.db b/.pnpm-store/v11/index.db new file mode 100644 index 0000000000000000000000000000000000000000..4a3492ca9ee32d110ce3484c72386f9ff8651a2c GIT binary patch literal 4096 zcmWFz^vNtqRY=P(%1ta$FlG>7U}9o$P*7lCU|@t|AVoG{WYC*>iWj6^fNV2HHI9bB nXb6mkz-S1JhQMeDjE2By2#kinXb6mkz-S1JhQMeDP#6LLGPMVA literal 0 HcmV?d00001 diff --git a/.pnpm-store/v11/index.db-shm b/.pnpm-store/v11/index.db-shm new file mode 100644 index 0000000000000000000000000000000000000000..2c169b70a936294b5dc21f21fc1836aa82ae12dd GIT binary patch literal 32768 zcmeI)u?fOJ6b9ha2~q|dCDwvX8n>`^7F$V|+D6325iDH82^=JvCmf`;3*`IZxVy*k z9()5_`QD_QSxUvczr@-*i7I>eZ$E^L_3eJwJmC>v8U9|K{ep zod5v>1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U zAV7cs0RjXF5FkK+009C72oNAZUx2-Pra=73A_NE!AV7cs0RjXF5FkK+009C7 z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk;}ZA+QUN2c literal 0 HcmV?d00001 diff --git a/.pnpm-store/v11/index.db-wal b/.pnpm-store/v11/index.db-wal new file mode 100644 index 0000000000000000000000000000000000000000..8f7f5039a3b998f5e914c16f2e471dc670a83040 GIT binary patch literal 8272 zcmXr7XKP~6eI&uaAiw|uGq3z>lQ&r2AN}Cm(+&1wa1~1;0=qAGoBZLbzv$hks~@LXdyBrwddZ80g@BK5V?8 zcG2!Ysc6#$T%g#13?&T1cp3H5Xb6mkz-S1JhQMeDjE2By2#kinXb6mkz-S1JhQMeD JjD`Rx1OQrWPcQ%g literal 0 HcmV?d00001 diff --git a/CHANGELOG.md b/CHANGELOG.md index 22fa8831..4f108027 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,9 +7,33 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html). --- -## [0.8.11] - 2026-07-09 +## [0.8.11] - 2026-07-13 + +_« Socle clean » — the Phase-1 hardening batch from the 2026-07 full audit (security defaults, honest CI, operational reliability, the typegen/step-dispatch structural taxes), extended by the run-lifecycle campaign reviewed pass-by-pass with Codex: run state machine + resume, envelope contract, typegen contract, Link-header pagination, and the API/auth surface._ + +### Added + +- **Source verification now covers multi-agent rooms.** Messages posted by agents over MCP (`disc_append`, live single-Agent appends) are linted against the discussion's project tree, exactly like streamed replies — the report shows as the usual badge in the UI, and a compact summary is echoed back in the agent's TOOL RESULT so it can self-correct unverifiable `[src:]` citations in its next message. Bulk transcript imports, User/System messages and project-less discussions are exempt; the insert is never blocked. +- **Codex gpt-5.6 model generation.** Tier suggestions now cover `gpt-5.6-sol` (frontier), `gpt-5.6-terra` (balanced), `gpt-5.6-luna` (fast) plus the 5.5/5.4 line; built-in tier fallbacks move to luna (economy) and sol (reasoning). +- **`ApiCall` steps can walk GitHub-style pagination** (TD-20260630 resolved). New `LinkHeader` pagination spec: follows the `Link: rel="next"` response header and accumulates bare top-level arrays — the two GitHub REST traits the executor couldn't handle, which silently truncated every list endpoint at one page (the root cause behind the PR-Review bot re-reviewing long PRs: reviews past page 1 were invisible to its dedup). Existing `max_pages` cap and `PAGINATION_TRUNCATED` signal apply. +- **Impossible state transitions are observable.** New `kronn::invariant` tracing target groups every "should never happen" signal: run-progress writes blocked by the status guard (with the held vs attempted statuses), batch counter bumps after the batch left Running, refused P2P fetch-file calls, agents emitting broken `---STEP_OUTPUT---` envelopes (and fence-strip salvages), and the boot-reconcile orphan probe. +- **Interrupted runs are resumable.** A run killed by a backend restart/crash (status `Interrupted`) can now be resumed — « Reprendre » button on the run, `POST /api/workflow-runs/:id/resume`, and the `workflow_resume_run` MCP tool. The resume claims the run atomically (a double-click gets one resume + one clear error), re-attaches the preserved worktree (refused if it's gone — never a silent fallback to the main checkout), replays completed steps into the template context and restarts at the interrupted step. A foreach fan-out re-runs ONLY the items not yet done, reconciled from three sources: the git `[item_id]` commit ledger, `Success` child runs (covers a child that finished right before the crash), and a durable done-set written to `run.state` after each item's durable effect — a done-set entry nothing confirms is treated as stale and the item re-runs. `Cancelled`/`Failed` stay sticky; children resume through their parent. + +### Fixed + +- **`cargo test` can no longer wipe the developer's real config.** Integration and lib tests exercising config-saving handlers wrote to the REAL `config.toml` (KRONN_DATA_DIR unset → platform dir): every full test run clobbered pseudo/avatar/model-tiers — and only the 0.8.11 keystore reconcile kept the encryption key alive. Tests are now isolated to a tempdir, and a write-chokepoint guard makes any future offender fail loudly instead of clobbering. +- **Backend test suite fully stabilized** (8 consecutive full runs green): remaining env races serialized, and the `data_dir_lock` flake root-caused to transient fd exhaustion under load (retry window added, real error surfaced). +- **`generated.ts` is auto-generated again** (TD-20260701 resolved). `make typegen` now really regenerates `frontend/src/types/generated.ts` from the ts-rs bindings (281 types — the hand-maintained aggregate carried 194 and drifted since June), and CI fails on any Rust-model change committed without it (`git diff` guard). The assembler reproduces the serde contract ts-rs can't see: on request-direction types, `#[serde(default)]` and plain `Option` fields (omittable on the wire) become optional in TS, while response types stay strict. Three request/response types the frontend already used are now exported from their Rust source of truth, and the whole frontend type-checks against the regenerated aggregate (tsc exit 0). +- **P2P invite-code auth requires an ACCEPTED contact.** The auth-exempt federation routes (`claim-by-token`, `fetch-file`) and the F8 file-fetch client accepted any contact whose invite code was known — including `pending`/`refused` ones, which keep their code. One sanctioned predicate now decides (`authenticate_invite_code`); a non-accepted caller gets exactly the unknown-code answer (no status oracle) plus a `kronn::invariant` log line. The WS Presence auto-add-as-pending flow is untouched — pending is its purpose. +- **DB export/import no longer loses Quick Prompt history and learning rejection counters** (export format v5). `quick_prompt_versions` (the lineage behind version metrics) and `learning_rejections` (the anti-repetition threshold) were silently dropped, so a migrated instance lost its QP history and re-armed rejected learnings from zero. Both tables now round-trip verbatim; v4 archives stay importable (`serde(default)`). +- **Config import is zip-bomb safe.** The 512 MiB upload limit only bounds the COMPRESSED archive; `extract_zip` read entries without a decompressed ceiling, so a tiny highly-compressible ZIP could force an arbitrary allocation before any validation. Per-entry decompressed caps now apply (512 MiB `data.json`, 1 MiB `config.toml`, 64 KiB `recovery.key`) with a hard refusal past the cap. +- **`X-Real-IP` can no longer mint local trust on native binds.** The auth middleware trusted the header unconditionally — correct behind the bundled nginx (which overwrites it), but on a native bind axum faces clients directly, so any LAN peer could send `X-Real-IP: 127.0.0.1` and bypass auth entirely, including the destructive-endpoint gate. The header is now only honored in Docker (`KRONN_IN_DOCKER`/container markers); native trust comes from the real socket address. +- **Every DELETE endpoint now requires local trust or the token, even with auth disabled.** `DESTRUCTIVE_PATHS` listed 4 paths while dozens of irreversible deletions (projects, discussions, workflows, MCP configs, contacts…) stayed open to any LAN peer on an auth-off bind. The criterion is now the VERB — every `DELETE`, plus an inventory of destructive POSTs (`audit-runs/cleanup`, `api-call-logs/purge`, `debug/logs/clear`, `agents/uninstall`, `rtk/deactivate`, `cleanup-orphan-env`) — so future delete routes are covered by construction. +- **`cargo test` no longer opens a browser tab.** The `/api/open-url` contract test exercised the real handler, which really launched the system browser on `https://example.com` at every full test run (silent on WSL — visible on macOS). The handler now suppresses the launch inside test binaries, same chokepoint mechanism as the config-write guard. +- **A foreach step's output no longer breaks downstream template references.** The step envelope carries the child's full output — `---STEP_OUTPUT---` markers included — inside a JSON string; the extractor matched the FIRST end-marker, cut the JSON mid-string and rejected the valid envelope into the repair loop (so `{{steps..data.*}}` silently never resolved). End-marker candidates are now tried outermost-first. Locked by a degenerate-output corpus (CRLF, fenced JSON, truncations, emoji, quoted examples) and a no-panic mutation sweep over the extractor. +- **Ollama prompt truncation is now detected exactly.** The pre-flight chars/3 estimate is blind to token-dense content; the terminal response chunk now compares `prompt_eval_count` against the requested `num_ctx` — a prompt that filled the window logs a warning with both numbers and marks the step logs (`Ollama truncation:`), instead of silently degrading output quality. +- **Resuming a run interrupted during its FIRST step re-attaches its worktree.** Resume detection keyed on `step_results` alone missed the no-result-yet case and would have created a second worktree; `workspace_path` presence now also marks a resume. Same latent bug fixed on the gate `request_changes` path when the target is the first step. -_« Socle clean » — the Phase-1 hardening batch from the 2026-07 full audit: security defaults, honest CI, operational reliability, and the two structural taxes (typegen, step dispatch) paid before the continual-learning work starts._ ### Fixed (hardening pass — post-merge adversarial review, 7 reviewers over the whole codebase) diff --git a/Makefile b/Makefile index 9fb2322e..e11d1c35 100644 --- a/Makefile +++ b/Makefile @@ -260,12 +260,9 @@ check: typegen: @echo "$(GREEN)▸ Generating ts-rs bindings from Rust models...$(RESET)" cd backend && cargo test export_bindings -- --nocapture - @echo "$(GREEN)▸ Checking generated.ts for field drift vs the Rust models...$(RESET)" - @node frontend/scripts/check-types-drift.mjs || { \ - echo "$(YELLOW)▸ generated.ts is missing fields — add them (see backend/bindings/.ts),$(RESET)"; \ - echo "$(YELLOW) or run 'node frontend/scripts/assemble-generated-types.mjs' for a full regen.$(RESET)"; \ - exit 1; } - @echo "$(GREEN)▸ generated.ts is in sync with the Rust models.$(RESET)" + @echo "$(GREEN)▸ Assembling frontend/src/types/generated.ts from the bindings...$(RESET)" + @node frontend/scripts/assemble-generated-types.mjs + @echo "$(GREEN)▸ generated.ts regenerated — commit it with your Rust model change.$(RESET)" ## Run backend tests (skips ts-rs exports to avoid overwriting generated.ts) test-backend: diff --git a/README.fr.md b/README.fr.md index 14b497df..d8cfd431 100644 --- a/README.fr.md +++ b/README.fr.md @@ -27,7 +27,7 @@ **Prompts plus petits, code déterministe quand c'est possible : moins d'hallucinations, facture tokens divisée, écoconception par conception.** -> **Statut : 0.8.8.** Fonctionnel mais pré-1.0. Les versions mineures peuvent introduire des breaking changes ; les patch versions sont safe. +> **Statut : 0.8.11.** Fonctionnel mais pré-1.0. Les versions mineures peuvent introduire des breaking changes ; les patch versions sont safe. > **Licence : AGPL-3.0.** Utiliser Kronn localement pour développer *ton propre* produit ne déclenche pas le copyleft ; il ne s'applique que si tu redistribues une version modifiée à d'autres. Voir [Notes sur la licence](#notes-sur-la-licence-agpl-3-0). ## Sommaire diff --git a/README.md b/README.md index 4e81fb3b..4aff7c8b 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ **Smaller prompts, more code where code is enough: fewer hallucinations, lower token bill, eco-design by default.** -> **Status: 0.8.8.** Functional but pre-1.0. Breaking changes happen between minor versions; patch versions are safe. +> **Status: 0.8.11.** Functional but pre-1.0. Breaking changes happen between minor versions; patch versions are safe. > **License: AGPL-3.0.** Using Kronn locally to build *your own* product is fine; the copyleft only kicks in if you distribute a modified Kronn to others. See [License notes](#license-notes-agpl-3-0). ## Contents @@ -248,7 +248,7 @@ Three modes, defaulting to `warn`, picked in **Settings → Sourcing & Anti- - **`off`** — no directive, no checking. The pre-0.8.7 behaviour. - **`warn` (default)** — each agent receives a sourcing directive : *"cite a `file:line`, URL, or `user-confirmed` for every non-trivial claim; if you can't, say so plainly and verify before asserting."* Kronn then mechanically inspects each `[src: …]` citation the agent emits — path-jailed to the project root (no FS escape), checks the file exists and the line range is in bounds. A non-blocking per-message pill surfaces problems : **red** for fabricated citations (the cited file/line does not exist, or the source type is `training-data` = model's prior knowledge, auto-rejected), **amber** for confident claims left without any anchor. Clicking the pill opens a detail panel listing the bad citations. -- **`enforce` (preview · 0.8.8)** — same as warn today ; in 0.8.8 will refuse writes to `AGENTS.md curated="ai"` sections whose citations don't resolve. +- **`enforce` (0.8.8)** — warn, plus refuses writes to `AGENTS.md curated="ai"` sections whose citations don't resolve. **Honest by design** : `verified` means the citation *exists*, not that the claim is *true* (catching "real but irrelevant" citations is the job of a future LLM-judge layer). The pill is non-blocking — it's a signal you decide to act on, not a hard gate. Language-agnostic + compression-proof + ungameable : you can't fabricate a file or line number that actually exists in your repo. diff --git a/backend/scripts/disc-introspection-mcp.py b/backend/scripts/disc-introspection-mcp.py index 2f798302..1a045981 100755 --- a/backend/scripts/disc-introspection-mcp.py +++ b/backend/scripts/disc-introspection-mcp.py @@ -511,6 +511,24 @@ "required": ["workflow_id", "run_id"], }, }, + { + "name": "workflow_resume_run", + "description": ( + "Resume an INTERRUPTED run (backend restart/crash killed it " + "mid-flight). Continues from the step after the last completed " + "one, re-attached to the preserved worktree; a foreach step " + "re-runs only the items not yet done. Refused when the run is " + "not Interrupted, is a sub-workflow child (resume the parent) " + "or a batch, or when its worktree is gone." + ), + "inputSchema": { + "type": "object", + "properties": { + "run_id": {"type": "string"}, + }, + "required": ["run_id"], + }, + }, { "name": "qp_list", "description": ( @@ -1194,7 +1212,7 @@ "api_headers": {"type": "object", "description": "Extra request headers. NEVER pass auth — Kronn injects per the plugin spec."}, "api_body": {"description": "JSON body for POST/PUT/PATCH (object/array). String leaves can contain `{{var}}` placeholders."}, "api_extract": {"type": "object", "description": "Optional JSONPath extract spec: `{path: \"$.items\", fail_on_empty: false}`."}, - "api_pagination": {"type": "object", "description": "Optional pagination spec (page/offset/cursor strategies)."}, + "api_pagination": {"type": "object", "description": "Optional pagination spec — internally tagged `{\"type\": ...}`: Auto | Offset | Cursor | Page | LinkHeader (GitHub-style bare array + `Link: rel=next` header; fields page_size_param/page_size/max_pages)."}, "api_timeout_ms": {"type": "integer", "description": "Optional per-call timeout in ms. Defaults to plugin default."}, "api_max_retries": {"type": "integer", "description": "Optional retry count on transient HTTP errors."}, "variables": { @@ -2934,6 +2952,15 @@ def call_workflow_cancel_run(args): return _unwrap(_http("POST", f"/api/workflows/{wid}/runs/{rid}/cancel")) +def call_workflow_resume_run(args): + """Resume an Interrupted run — atomic claim backend-side, so a double + call gets one resume + one clear error.""" + rid = args.get("run_id") + if not rid: + raise RuntimeError("workflow_resume_run: missing required 'run_id'") + return _unwrap(_http("POST", f"/api/workflow-runs/{rid}/resume")) + + def call_workflow_update(args): """Patch an existing workflow. `UpdateWorkflowRequest` is already a TRUE patch backend-side (any omitted field preserves its current @@ -3947,6 +3974,7 @@ def call_workflow_step_schema(_args): "workflow_runs": call_workflow_runs, "workflow_run_get": call_workflow_run_get, "workflow_cancel_run": call_workflow_cancel_run, + "workflow_resume_run": call_workflow_resume_run, "qp_list": call_qp_list, "qa_list": call_qa_list, "mcp_list": call_mcp_list, diff --git a/backend/src/agents/runner.rs b/backend/src/agents/runner.rs index c195e1f8..be18ce4a 100644 --- a/backend/src/agents/runner.rs +++ b/backend/src/agents/runner.rs @@ -509,9 +509,10 @@ pub(crate) fn resolve_model_flag(agent_type: &AgentType, tier: ModelTier, overri (AgentType::ClaudeCode, ModelTier::Economy) => Some("haiku".into()), (AgentType::ClaudeCode, ModelTier::Default) => Some("sonnet".into()), (AgentType::ClaudeCode, ModelTier::Reasoning) => Some("opus".into()), - (AgentType::Codex, ModelTier::Economy) => Some("gpt-5-codex-mini".into()), + // 2026-07: gpt-5.6 generation (sol=frontier, terra=balanced, luna=fast). + (AgentType::Codex, ModelTier::Economy) => Some("gpt-5.6-luna".into()), (AgentType::Codex, ModelTier::Default) => None, // Codex default is fine - (AgentType::Codex, ModelTier::Reasoning) => Some("gpt-5.4".into()), + (AgentType::Codex, ModelTier::Reasoning) => Some("gpt-5.6-sol".into()), (AgentType::GeminiCli, ModelTier::Economy) => Some("gemini-2.5-flash".into()), (AgentType::GeminiCli, ModelTier::Default) => None, // Gemini default is fine (AgentType::GeminiCli, ModelTier::Reasoning) => Some("gemini-3.1-pro-preview".into()), @@ -1388,6 +1389,10 @@ pub(crate) async fn forward_ollama_line( stderr: &Arc>>, got_done: &mut bool, got_error: &mut bool, + // Requested num_ctx — lets the terminal chunk detect ACTUAL truncation + // (prompt_eval_count at the window) instead of relying on the pre-flight + // chars/3 estimate, which is blind to token-dense content. 0 = unknown. + num_ctx: u64, ) -> bool { if line.trim().is_empty() { return true; } if let Ok(json) = serde_json::from_str::(line) { @@ -1414,6 +1419,22 @@ pub(crate) async fn forward_ollama_line( if let Ok(mut stderr) = stderr.lock() { stderr.push(format!("ollama_tokens:{}:{}", prompt_tokens, eval_tokens)); } + // A prompt that FILLED the window was almost certainly cut by + // Ollama (it silently drops the overflow) — exact signal, unlike + // the pre-flight estimate. + if num_ctx > 0 && prompt_tokens >= num_ctx.saturating_sub(64) { + tracing::warn!( + target: "kronn::ollama", + prompt_tokens, num_ctx, + "prompt filled the context window — input was silently TRUNCATED by Ollama; \ + reduce the step's input or raise KRONN_OLLAMA_NUM_CTX_CAP" + ); + if let Ok(mut stderr) = stderr.lock() { + stderr.push(format!( + "Ollama truncation: prompt_eval_count {prompt_tokens} filled num_ctx {num_ctx}" + )); + } + } } } true @@ -1564,7 +1585,7 @@ async fn start_ollama_http( let line = buffer[..newline_pos].to_string(); buffer = buffer[newline_pos + 1..].to_string(); // Consumer gone (cancel) → stop reading the HTTP body. - if !forward_ollama_line(&line, &tx, &stderr_clone, &mut got_done, &mut got_error).await { + if !forward_ollama_line(&line, &tx, &stderr_clone, &mut got_done, &mut got_error, ctx_cap).await { finish(&mut lifeline, false).await; return; } @@ -1574,7 +1595,7 @@ async fn start_ollama_http( // Non-streaming responses (format-constrained / TypedSchema steps set // stream:false) arrive as a single JSON object with no trailing // newline, so the line loop above never fires — flush the remainder. - let _ = forward_ollama_line(buffer.trim(), &tx, &stderr_clone, &mut got_done, &mut got_error).await; + let _ = forward_ollama_line(buffer.trim(), &tx, &stderr_clone, &mut got_done, &mut got_error, ctx_cap).await; // A stream that ends without the terminal `done` chunk was truncated // (server closed the connection, proxy cut it, model unloaded) — that diff --git a/backend/src/agents/runner_test.rs b/backend/src/agents/runner_test.rs index 5a1905eb..7ee0281a 100644 --- a/backend/src/agents/runner_test.rs +++ b/backend/src/agents/runner_test.rs @@ -126,13 +126,13 @@ mod tests { let stderr = Arc::new(Mutex::new(Vec::::new())); let (mut done, mut err) = (false, false); // A streamed content chunk... - forward_ollama_line(r#"{"message":{"content":"391"},"done":false}"#, &tx, &stderr, &mut done, &mut err).await; + forward_ollama_line(r#"{"message":{"content":"391"},"done":false}"#, &tx, &stderr, &mut done, &mut err, 0).await; assert!(!done && !err); // ...then the terminal `done` object (identical shape to a non-stream // single-object response), carrying the token counts. forward_ollama_line( r#"{"message":{"content":""},"done":true,"prompt_eval_count":12,"eval_count":3}"#, - &tx, &stderr, &mut done, &mut err, + &tx, &stderr, &mut done, &mut err, 0, ).await; assert!(done && !err, "terminal chunk sets got_done, no error"); drop(tx); @@ -148,14 +148,41 @@ mod tests { let (tx, mut rx) = tokio::sync::mpsc::channel::(8); let stderr = Arc::new(Mutex::new(Vec::::new())); let (mut done, mut err) = (false, false); - forward_ollama_line(" ", &tx, &stderr, &mut done, &mut err).await; // blank tail buffer - forward_ollama_line("{not json", &tx, &stderr, &mut done, &mut err).await; // partial/garbage + forward_ollama_line(" ", &tx, &stderr, &mut done, &mut err, 0).await; // blank tail buffer + forward_ollama_line("{not json", &tx, &stderr, &mut done, &mut err, 0).await; // partial/garbage drop(tx); assert!(rx.recv().await.is_none(), "no content forwarded"); assert!(stderr.lock().unwrap().is_empty(), "no token line captured"); assert!(!done && !err, "garbage neither completes nor fails the stream"); } + #[tokio::test] + async fn forward_ollama_line_flags_prompt_that_filled_the_window() { + use std::sync::{Arc, Mutex}; + let (tx, _rx) = tokio::sync::mpsc::channel::(8); + let stderr = Arc::new(Mutex::new(Vec::::new())); + let (mut done, mut err) = (false, false); + // prompt_eval_count within 64 of num_ctx → Ollama silently dropped the + // overflow; the terminal chunk must surface it (exact, unlike the + // pre-flight estimate). + forward_ollama_line( + r#"{"message":{"content":""},"done":true,"prompt_eval_count":8180,"eval_count":3}"#, + &tx, &stderr, &mut done, &mut err, 8192, + ).await; + assert!(done && !err, "truncation warns, it does not fail the step"); + let lines = stderr.lock().unwrap().clone(); + assert!(lines.iter().any(|l| l.contains("Ollama truncation")), "{lines:?}"); + + // Comfortable margin → no truncation marker. + let stderr2 = Arc::new(Mutex::new(Vec::::new())); + let (mut done2, mut err2) = (false, false); + forward_ollama_line( + r#"{"message":{"content":""},"done":true,"prompt_eval_count":4000,"eval_count":3}"#, + &tx, &stderr2, &mut done2, &mut err2, 8192, + ).await; + assert!(!stderr2.lock().unwrap().iter().any(|l| l.contains("truncation"))); + } + #[tokio::test] async fn forward_ollama_line_surfaces_in_band_error() { use std::sync::{Arc, Mutex}; @@ -166,7 +193,7 @@ mod tests { // be silently ignored → step "succeeded" with empty output. forward_ollama_line( r#"{"error":"model runner has unexpectedly stopped"}"#, - &tx, &stderr, &mut done, &mut err, + &tx, &stderr, &mut done, &mut err, 0, ).await; drop(tx); assert!(err, "in-band error object must mark the stream failed"); @@ -853,9 +880,9 @@ Suite de la réponse."; #[test] fn resolve_model_flag_codex_tiers() { use crate::models::ModelTier; - assert_eq!(resolve_model_flag(&AgentType::Codex, ModelTier::Economy, None), Some("gpt-5-codex-mini".into())); + assert_eq!(resolve_model_flag(&AgentType::Codex, ModelTier::Economy, None), Some("gpt-5.6-luna".into())); assert_eq!(resolve_model_flag(&AgentType::Codex, ModelTier::Default, None), None); - assert_eq!(resolve_model_flag(&AgentType::Codex, ModelTier::Reasoning, None), Some("gpt-5.4".into())); + assert_eq!(resolve_model_flag(&AgentType::Codex, ModelTier::Reasoning, None), Some("gpt-5.6-sol".into())); } #[test] diff --git a/backend/src/api/disc_invite.rs b/backend/src/api/disc_invite.rs index 41416715..2bfa01ca 100644 --- a/backend/src/api/disc_invite.rs +++ b/backend/src/api/disc_invite.rs @@ -683,11 +683,22 @@ pub async fn claim_by_token( let code_lookup = from_code.clone(); let caller = match state .db - .with_conn(move |conn| crate::db::contacts::find_contact_by_invite_code(conn, &code_lookup)) + .with_conn(move |conn| crate::db::contacts::authenticate_invite_code(conn, &code_lookup)) .await { - Ok(Some(c)) => c, - Ok(None) => return Json(ApiResponse::err("unknown peer (invite code not in contacts)")), + Ok(crate::db::contacts::InviteAuth::Accepted(c)) => c, + // Pending/refused answers EXACTLY like unknown — no status oracle. + Ok(crate::db::contacts::InviteAuth::NotAccepted { pseudo, status }) => { + tracing::warn!( + target: "kronn::invariant", + caller = %pseudo, status = %status, route = "claim-by-token", + "invite-code auth refused — contact is not accepted" + ); + return Json(ApiResponse::err("unknown peer (invite code not in contacts)")); + } + Ok(crate::db::contacts::InviteAuth::Unknown) => { + return Json(ApiResponse::err("unknown peer (invite code not in contacts)")) + } Err(e) => return Json(ApiResponse::err(format!("contact lookup error: {e}"))), }; @@ -800,11 +811,22 @@ pub async fn fetch_file( // being a contact is not enough to read arbitrary files (see below). let caller = match state .db - .with_conn(move |conn| crate::db::contacts::find_contact_by_invite_code(conn, &from_code)) + .with_conn(move |conn| crate::db::contacts::authenticate_invite_code(conn, &from_code)) .await { - Ok(Some(c)) => c, - Ok(None) => return Json(ApiResponse::err("unknown peer (invite code not in contacts)")), + Ok(crate::db::contacts::InviteAuth::Accepted(c)) => c, + // Pending/refused answers EXACTLY like unknown — no status oracle. + Ok(crate::db::contacts::InviteAuth::NotAccepted { pseudo, status }) => { + tracing::warn!( + target: "kronn::invariant", + caller = %pseudo, status = %status, route = "fetch-file", + "invite-code auth refused — contact is not accepted" + ); + return Json(ApiResponse::err("unknown peer (invite code not in contacts)")); + } + Ok(crate::db::contacts::InviteAuth::Unknown) => { + return Json(ApiResponse::err("unknown peer (invite code not in contacts)")) + } Err(e) => return Json(ApiResponse::err(format!("contact lookup error: {e}"))), }; @@ -840,8 +862,9 @@ pub async fn fetch_file( }; if !shared_with_caller { tracing::warn!( - "fetch-file refused: contact '{}' asked for file {} of a discussion not shared with it", - caller.pseudo, cf.id + target: "kronn::invariant", + caller = %caller.pseudo, file_id = %cf.id, disc_id = %cf.discussion_id, + "fetch-file refused — discussion not shared with the caller" ); return Json(ApiResponse::ok(FetchFileResponse { found: false, @@ -961,6 +984,56 @@ mod tests { assert_eq!(bytes, b"BYTES"); } + #[tokio::test] + async fn non_accepted_contacts_are_refused_like_unknown_codes() { + // Passe D (Codex constat n°1) — a pending/refused contact keeps its + // invite code but must NOT pass the auth-exempt P2P routes, and the + // refusal must be indistinguishable from an unknown code (no oracle). + let state = make_state_with_disc("d-auth-1").await; + state.db.with_conn(|conn| { + for (id, code, status) in [ + ("c-pend", "kr-inv-pend", "pending"), + ("c-ref", "kr-inv-ref", "refused"), + ] { + crate::db::contacts::insert_contact(conn, &crate::models::Contact { + id: id.into(), + pseudo: format!("peer-{status}"), + avatar_email: None, + kronn_url: "http://peer.local".into(), + invite_code: code.into(), + status: status.into(), + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + })?; + } + Ok(()) + }).await.unwrap(); + + for code in ["kr-inv-pend", "kr-inv-ref", "kr-inv-ghost"] { + // fetch-file: same error string for pending, refused and unknown. + let resp = fetch_file(State(state.clone()), Json(FetchFileRequest { + file_id: "f-any".into(), + from_invite_code: code.into(), + })).await; + assert_eq!( + resp.0.error.as_deref(), + Some("unknown peer (invite code not in contacts)"), + "{code} must be refused with the unknown-code message" + ); + + // claim-by-token: same contract. + let resp = claim_by_token(State(state.clone()), Json(ClaimByTokenRequest { + token: "kr-join-whatever".into(), + from_invite_code: code.into(), + })).await; + assert_eq!( + resp.0.error.as_deref(), + Some("unknown peer (invite code not in contacts)"), + "{code} must be refused with the unknown-code message" + ); + } + } + #[tokio::test] async fn invite_peer_returns_plain_token_for_existing_disc() { let state = make_state_with_disc("d-invite-1").await; diff --git a/backend/src/api/disc_source.rs b/backend/src/api/disc_source.rs index 5283de5c..d61e0153 100644 --- a/backend/src/api/disc_source.rs +++ b/backend/src/api/disc_source.rs @@ -174,6 +174,17 @@ pub struct DiscAppendRequest { pub messages: Vec, } +/// Compact lint feedback echoed to the POSTING agent (tool result), so it can +/// self-correct unverifiable `[src:]` citations in its next message. The full +/// report rides the stored message (UI badge), same as streaming replies. +#[derive(Debug, Clone, Serialize, Deserialize, TS)] +#[ts(export)] +pub struct AppendLintSummary { + pub fabricated_count: u32, + pub unsourced_count: u32, + pub note: String, +} + #[derive(Debug, Clone, Serialize, Deserialize, TS)] #[ts(export)] pub struct DiscAppendResponse { @@ -183,6 +194,10 @@ pub struct DiscAppendResponse { /// last import — the caller should warn the user before pushing /// MORE messages (they might be applying stale state on top). pub diverged: bool, + /// Present only for a live single Agent append whose lint had a signal. + #[serde(skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub lint: Option, } /// `POST /api/disc/append` @@ -194,13 +209,11 @@ pub async fn disc_append( let exists = state.db.with_conn(move |conn| { crate::db::discussions::get_discussion(conn, &did) }).await; - if !matches!(exists, Ok(Some(_))) { - return match exists { - Ok(None) => Json(ApiResponse::err("Discussion not found")), - Err(e) => Json(ApiResponse::err(format!("DB error: {}", e))), - _ => unreachable!(), - }; - } + let disc = match exists { + Ok(Some(d)) => d, + Ok(None) => return Json(ApiResponse::err("Discussion not found")), + Err(e) => return Json(ApiResponse::err(format!("DB error: {}", e))), + }; // 0.8.4 (#294) — `diverged_at` lives on the table but NOT on the // `Discussion` struct (see migration 054 + the model comment). @@ -210,6 +223,50 @@ pub async fn disc_append( let diverged = state.db.with_conn(move |conn| { crate::db::disc_source::get_diverged_at(conn, &did_div) }).await.ok().flatten().is_some(); + // Lint-on-append (contract 2026-07-13): ONLY a live single Agent append + // is linted — bulk imports, User/System messages and project-less discs + // are exempt — and the insert is NEVER blocked. The full report rides the + // stored message (UI badge); the summary rides the response (tool result) + // so the posting agent can self-correct. + let live_agent_append = req.messages.len() == 1 + && matches!(req.messages[0].role, MessageRole::Agent); + let mut live_lint_report: Option = None; + let mut lint_summary: Option = None; + if live_agent_append && crate::core::anti_halluc::current_mode().is_active() { + if let Some(pid) = disc.project_id.clone() { + let roots = state.db.with_conn(move |conn| { + let p = crate::db::projects::get_project(conn, &pid)?; + Ok(p.map(|p| { + let linked = p.linked_repos.iter() + .map(|lr| lr.location.clone()) + .filter(|loc| !loc.starts_with("http://") && !loc.starts_with("https://")) + .collect::>(); + (p.path, linked) + })) + }).await.ok().flatten(); + if let Some((project_path, linked)) = roots.filter(|(p, _)| !p.is_empty()) { + live_lint_report = crate::core::anti_halluc::finalize_lint_report( + &req.messages[0].content, + None, + &project_path, + &linked, + ); + // Echo a summary only when something actually FAILED — a + // report with soft signals but zero failures would pair a + // scary note with 0/0 counts (caught by live dogfooding). + if let Some(ref r) = live_lint_report { + if r.fabricated_count > 0 || r.unsourced_count > 0 { + lint_summary = Some(AppendLintSummary { + fabricated_count: r.fabricated_count, + unsourced_count: r.unsourced_count, + note: "Some citations in your message could not be verified against the discussion's project tree — re-check the [src:] paths/lines and correct in your next message if needed.".into(), + }); + } + } + } + } + } + let mut appended = 0u32; let mut skipped = 0u32; // Freshly-inserted messages, federated to peers after the loop IF this is a @@ -230,7 +287,8 @@ pub async fn disc_append( let msg = DiscussionMessage { model: None, - lint_report: None, + // Only a live single Agent append carries a report (loop runs once). + lint_report: live_lint_report.take(), id: Uuid::new_v4().to_string(), role: incoming.role.clone(), content: incoming.content.clone(), @@ -307,6 +365,7 @@ pub async fn disc_append( appended, skipped_as_duplicates: skipped, diverged, + lint: lint_summary, })) } @@ -569,6 +628,124 @@ mod tests { //! invariants (no I/O). use super::*; + use serial_test::serial; + + /// In-memory state with a project rooted at a real tempdir + one disc. + async fn lint_state(bind_project: bool) -> (crate::AppState, tempfile::TempDir) { + use std::sync::Arc; + use tokio::sync::RwLock; + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::create_dir_all(tmp.path().join("src")).unwrap(); + std::fs::write(tmp.path().join("src/real.rs"), "fn real() {}\n").unwrap(); + let db = Arc::new(crate::db::Database::open_in_memory().unwrap()); + let path = tmp.path().to_string_lossy().to_string(); + let bind = bind_project; + db.with_conn(move |conn| { + let now = chrono::Utc::now().to_rfc3339(); + conn.execute( + "INSERT INTO projects (id, name, path, created_at, updated_at) + VALUES ('p-lint', 'LintProj', ?1, ?2, ?2)", + rusqlite::params![path, now], + )?; + conn.execute( + "INSERT INTO discussions (id, project_id, title, agent, language, participants_json, + created_at, updated_at, message_count, workspace_mode) + VALUES ('d-lint', ?1, 'T', 'ClaudeCode', 'fr', '[]', datetime('now'), datetime('now'), 0, 'Direct')", + rusqlite::params![if bind { Some("p-lint") } else { None }], + )?; + Ok(()) + }).await.unwrap(); + let cfg = Arc::new(RwLock::new(crate::core::config::default_config())); + (crate::AppState::new_defaults(cfg, db, crate::DEFAULT_MAX_CONCURRENT_AGENTS), tmp) + } + + fn agent_msg(id: &str, content: &str) -> DiscAppendMessage { + DiscAppendMessage { + source_msg_id: id.into(), + role: MessageRole::Agent, + content: content.into(), + agent_type: Some(AgentType::Codex), + } + } + + async fn append(state: &crate::AppState, msgs: Vec) -> DiscAppendResponse { + let resp = disc_append( + axum::extract::State(state.clone()), + Json(DiscAppendRequest { disc_id: "d-lint".into(), messages: msgs }), + ).await; + resp.0.data.expect("append succeeds") + } + + #[tokio::test] + #[serial] // global anti-halluc mode cell + async fn live_agent_append_with_fabricated_source_carries_lint() { + crate::core::anti_halluc::set_mode("warn"); + let (state, _tmp) = lint_state(true).await; + let out = append(&state, vec![agent_msg("m1", + "Confirmed the bug. [src: file: src/does-not-exist.rs:42]")]).await; + assert_eq!(out.appended, 1, "insert is NEVER blocked"); + let lint = out.lint.expect("fabricated citation must produce a summary"); + assert!(lint.fabricated_count >= 1, "{lint:?}"); + // The stored message carries the full report (UI badge). + let msg = state.db.with_conn(|conn| { + crate::db::discussions::list_messages(conn, "d-lint") + }).await.unwrap().pop().unwrap(); + assert!(msg.lint_report.is_some()); + crate::core::anti_halluc::set_mode("off"); + } + + #[tokio::test] + #[serial] + async fn live_agent_append_with_valid_source_has_no_fabricated() { + crate::core::anti_halluc::set_mode("warn"); + let (state, _tmp) = lint_state(true).await; + let out = append(&state, vec![agent_msg("m1", + "Verified. [src: file: src/real.rs:1]")]).await; + assert_eq!(out.appended, 1); + if let Some(l) = out.lint { + assert_eq!(l.fabricated_count, 0, "valid citation must not read as fabricated: {l:?}"); + } + crate::core::anti_halluc::set_mode("off"); + } + + #[tokio::test] + #[serial] + async fn bulk_import_and_user_messages_are_never_linted() { + crate::core::anti_halluc::set_mode("warn"); + let (state, _tmp) = lint_state(true).await; + // Bulk (2 messages) with a fabricated citation → no lint. + let out = append(&state, vec![ + agent_msg("b1", "one [src: file: src/ghost.rs:1]"), + agent_msg("b2", "two"), + ]).await; + assert!(out.lint.is_none(), "bulk import must not lint"); + // Single USER message with a fabricated citation → no lint. + let user = DiscAppendMessage { + source_msg_id: "u1".into(), + role: MessageRole::User, + content: "look at [src: file: src/ghost.rs:1]".into(), + agent_type: None, + }; + let out = append(&state, vec![user]).await; + assert!(out.lint.is_none(), "user messages must not lint"); + crate::core::anti_halluc::set_mode("off"); + } + + #[tokio::test] + #[serial] + async fn projectless_disc_and_off_mode_skip_lint() { + // Project-less disc: no roots → no lint, no false fabricated. + crate::core::anti_halluc::set_mode("warn"); + let (state, _tmp) = lint_state(false).await; + let out = append(&state, vec![agent_msg("m1", "x [src: file: src/ghost.rs:1]")]).await; + assert!(out.lint.is_none(), "no project → no lint"); + // Mode off: bound project but lint disabled. + crate::core::anti_halluc::set_mode("off"); + let (state2, _tmp2) = lint_state(true).await; + let out = append(&state2, vec![agent_msg("m1", "x [src: file: src/ghost.rs:1]")]).await; + assert!(out.lint.is_none(), "mode off → no lint"); + } + #[test] fn disc_create_request_deserializes_with_optional_source_binding() { diff --git a/backend/src/api/federation.rs b/backend/src/api/federation.rs index 8ac84081..94a82eea 100644 --- a/backend/src/api/federation.rs +++ b/backend/src/api/federation.rs @@ -175,17 +175,28 @@ pub async fn fetch_and_store_attachment( return; } - // Resolve the announcing host's URL from its invite code. + // Resolve the announcing host's URL from its invite code — accepted + // contacts only (passe D): a pending/refused peer must not be able to + // make us download bytes from it via a FileAttached announce. let code = host_invite_code.to_string(); let host = state .db - .with_conn(move |conn| crate::db::contacts::find_contact_by_invite_code(conn, &code)) - .await - .ok() - .flatten(); - let Some(host) = host else { - tracing::warn!("F8: FileAttached from unknown host, cannot fetch {file_id}"); - return; + .with_conn(move |conn| crate::db::contacts::authenticate_invite_code(conn, &code)) + .await; + let host = match host { + Ok(crate::db::contacts::InviteAuth::Accepted(c)) => c, + Ok(crate::db::contacts::InviteAuth::NotAccepted { pseudo, status }) => { + tracing::warn!( + target: "kronn::invariant", + host = %pseudo, status = %status, file_id = %file_id, + "F8: FileAttached from a non-accepted contact — fetch refused" + ); + return; + } + _ => { + tracing::warn!("F8: FileAttached from unknown host, cannot fetch {file_id}"); + return; + } }; // Our own code authenticates the fetch (same trust model as claim-by-token). diff --git a/backend/src/api/learnings.rs b/backend/src/api/learnings.rs index 68807f7c..ad6acc98 100644 --- a/backend/src/api/learnings.rs +++ b/backend/src/api/learnings.rs @@ -21,7 +21,8 @@ use crate::AppState; const NEGATIVE_LEARNING_THRESHOLD: i64 = 3; -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, ts_rs::TS)] +#[ts(export, rename = "LearningProposeRequest")] pub struct ProposeRequest { pub claim: String, pub evidence: Vec, @@ -36,7 +37,8 @@ pub struct ProposeRequest { pub confidence: Option, } -#[derive(Debug, Serialize)] +#[derive(Debug, Serialize, ts_rs::TS)] +#[ts(export)] pub struct ProposeResult { pub accepted: bool, #[serde(skip_serializing_if = "Option::is_none")] diff --git a/backend/src/api/mcp_remote.rs b/backend/src/api/mcp_remote.rs index ca7bc3e4..3f9fd995 100644 --- a/backend/src/api/mcp_remote.rs +++ b/backend/src/api/mcp_remote.rs @@ -326,14 +326,9 @@ fn excerpt(s: &str) -> String { } fn is_terminal_status(status: &RunStatus) -> bool { - matches!( - status, - RunStatus::Success - | RunStatus::Failed - | RunStatus::Cancelled - | RunStatus::StoppedByGuard - | RunStatus::Interrupted - ) + // Single source of truth — RunStatus::is_terminal is exhaustive-matched, + // so a future variant can't diverge between this surface and the guards. + status.is_terminal() } /// GET /api/mcp/workflow-run-status/:run_id diff --git a/backend/src/api/mcps.rs b/backend/src/api/mcps.rs index 343980fb..4742b1be 100644 --- a/backend/src/api/mcps.rs +++ b/backend/src/api/mcps.rs @@ -1033,7 +1033,8 @@ pub async fn host_discovery( } } -#[derive(Deserialize)] +#[derive(Deserialize, ts_rs::TS)] +#[ts(export)] pub struct AdoptHostMcpRequest { /// Source file as reported by host_discovery (e.g. "/home/user/.claude.json"). pub source_file: String, diff --git a/backend/src/api/setup.rs b/backend/src/api/setup.rs index 1dc49e8a..5c0712a9 100644 --- a/backend/src/api/setup.rs +++ b/backend/src/api/setup.rs @@ -1175,6 +1175,12 @@ async fn build_export(state: &AppState) -> Result { // matter on a migrated box (None filters = no status / no project narrowing). let learnings = state.db.with_conn(|conn| crate::db::learnings::list(conn, None, None)).await .map_err(|e| format!("DB error: {}", e))?; + // v5 (passe D) — QP version lineage + rejection counters, previously lost. + let quick_prompt_versions = state.db + .with_conn(crate::db::quick_prompts::list_all_quick_prompt_versions).await + .map_err(|e| format!("DB error: {}", e))?; + let learning_rejections = state.db.with_conn(crate::db::learnings::list_rejections).await + .map_err(|e| format!("DB error: {}", e))?; let custom_skills: Vec<_> = crate::core::skills::list_all_skills() .into_iter().filter(|s| !s.is_builtin).collect(); @@ -1198,6 +1204,8 @@ async fn build_export(state: &AppState) -> Result { quick_prompts, quick_apis, learnings, + quick_prompt_versions, + learning_rejections, }) } @@ -1354,8 +1362,15 @@ fn import_clear_statements(data: &DbExport) -> Vec<&'static str> { } if !data.contacts.is_empty() { stmts.push("DELETE FROM contacts"); } if !data.quick_prompts.is_empty() { stmts.push("DELETE FROM quick_prompts"); } + // Versions clear ONLY when the archive carries some (v5+): a v4 export + // must not wipe local lineage it knows nothing about. Referential + // integrity vs the replaced parents is handled by the post-import prune. + if !data.quick_prompt_versions.is_empty() { + stmts.push("DELETE FROM quick_prompt_versions"); + } if !data.quick_apis.is_empty() { stmts.push("DELETE FROM quick_apis"); } if !data.learnings.is_empty() { stmts.push("DELETE FROM learnings"); } + if !data.learning_rejections.is_empty() { stmts.push("DELETE FROM learning_rejections"); } if !data.projects.is_empty() { stmts.push("DELETE FROM projects"); } stmts } @@ -1481,6 +1496,14 @@ async fn do_import_db(state: &AppState, data: &DbExport) -> Result Result 0 => warnings.push(format!( + "{n} quick-prompt version row(s) dropped — their prompts are not part of this import" + )), + Ok(_) => {} + Err(e) => tracing::warn!("Import version prune error: {}", e), + } + + // Import rejection counters (v5) — verbatim, keeps the anti-repetition + // threshold armed across a migration. + for rej in &data.learning_rejections { + let r = rej.clone(); + if let Err(e) = state.db.with_conn(move |conn| crate::db::learnings::insert_rejection_row(conn, &r)).await { + tracing::warn!("Import learning rejection error: {}", e); + } + } + if !invalid_paths.is_empty() { warnings.push(format!("{} project(s) have invalid paths — remap them in the Projects page", invalid_paths.len())); } @@ -1556,6 +1605,32 @@ async fn merge_import_config(state: &AppState, imported: &AppConfig) -> Vec Result { + let mut contents = String::new(); + std::io::Read::read_to_string(&mut f.take(cap + 1), &mut contents) + .map_err(|e| format!("Failed to read {name}: {e}"))?; + if contents.len() as u64 > cap { + return Err(format!( + "{name} exceeds the {cap}-byte decompressed limit — refusing (zip bomb?)" + )); + } + Ok(contents) +} + /// Extract data.json, config.toml and the optional recovery blob from a ZIP /// file (synchronous, no await). fn extract_zip(file_bytes: &[u8]) -> Result<(DbExport, Option, Option), String> { @@ -1567,35 +1642,27 @@ fn extract_zip(file_bytes: &[u8]) -> Result<(DbExport, Option, Option let data: DbExport = { let mut f = archive.by_name("data.json") .map_err(|e| format!("data.json not found in ZIP: {}", e))?; - let mut contents = String::new(); - std::io::Read::read_to_string(&mut f, &mut contents) - .map_err(|e| format!("Failed to read data.json: {}", e))?; + let contents = read_zip_entry_capped(&mut f, "data.json", ZIP_CAP_DATA_JSON)?; serde_json::from_str(&contents) .map_err(|e| format!("Invalid data.json: {}", e))? }; - // Read config.toml (optional) + // Read config.toml (optional) — but an OVERSIZED one is a hard error, + // not a silent None (that would downgrade a bomb into "no config"). let imported_config = if let Ok(mut f) = archive.by_name("config.toml") { - let mut contents = String::new(); - if std::io::Read::read_to_string(&mut f, &mut contents).is_ok() { - toml::from_str::(&contents).ok() - } else { - None - } + let contents = read_zip_entry_capped(&mut f, "config.toml", ZIP_CAP_CONFIG_TOML)?; + toml::from_str::(&contents).ok() } else { None }; // Read recovery.key (optional, P2) — kept only if it parses as a valid // recovery code; a corrupt entry is dropped, never an import error. + // An oversized one is refused like the others. let recovery_code = if let Ok(mut f) = archive.by_name("recovery.key") { - let mut contents = String::new(); - if std::io::Read::read_to_string(&mut f, &mut contents).is_ok() { - let trimmed = contents.trim().to_string(); - crate::core::recovery::from_code(&trimmed).ok().map(|_| trimmed) - } else { - None - } + let contents = read_zip_entry_capped(&mut f, "recovery.key", ZIP_CAP_RECOVERY_KEY)?; + let trimmed = contents.trim().to_string(); + crate::core::recovery::from_code(&trimmed).ok().map(|_| trimmed) } else { None }; @@ -1789,6 +1856,16 @@ pub async fn open_url(Json(req): Json) -> Json> if !req.url.starts_with("http://") && !req.url.starts_with("https://") { return Json(ApiResponse::err("Only http/https URLs are allowed")); } + // Test binaries (target/*/deps) must never launch a real browser: the + // integration suite exercises this endpoint's contract, and on macOS the + // real open() popped an example.com tab at every `cargo test` run. + let in_test_binary = std::env::current_exe() + .map(|p| p.components().any(|c| c.as_os_str() == "deps")) + .unwrap_or(false); + if in_test_binary { + tracing::info!("open-url suppressed in a test binary: {}", req.url); + return Json(ApiResponse::ok(())); + } match open::that(&req.url) { Ok(()) => Json(ApiResponse::ok(())), Err(e) => { @@ -1804,6 +1881,144 @@ mod tests { use serial_test::serial; use crate::core::config; + // ─── extract_zip decompressed caps (passe D: zip bomb) ──────────────── + + fn zip_with(entries: &[(&str, &[u8])]) -> Vec { + use std::io::Write; + let mut buf = std::io::Cursor::new(Vec::new()); + { + let mut w = zip::ZipWriter::new(&mut buf); + let opts = zip::write::SimpleFileOptions::default() + .compression_method(zip::CompressionMethod::Deflated); + for (name, bytes) in entries { + w.start_file(*name, opts).unwrap(); + w.write_all(bytes).unwrap(); + } + w.finish().unwrap(); + } + buf.into_inner() + } + + #[test] + fn extract_zip_refuses_oversized_decompressed_entries() { + // A tiny COMPRESSED upload can hide megabytes of zeros — the + // route-layer 512 MiB body limit never sees the decompressed size. + let data = serde_json::to_vec(&empty_export()).unwrap(); + let bomb_key = vec![b' '; (ZIP_CAP_RECOVERY_KEY + 1024) as usize]; + let bytes = zip_with(&[("data.json", &data), ("recovery.key", &bomb_key)]); + let err = extract_zip(&bytes).expect_err("oversized recovery.key must refuse"); + assert!(err.contains("zip bomb"), "{err}"); + + let bomb_cfg = vec![b' '; (ZIP_CAP_CONFIG_TOML + 1024) as usize]; + let bytes = zip_with(&[("data.json", &data), ("config.toml", &bomb_cfg)]); + let err = extract_zip(&bytes).expect_err("oversized config.toml must refuse"); + assert!(err.contains("zip bomb"), "{err}"); + + // Within caps → the archive still imports (recovery dropped as + // non-parsing, config parsed, data ok). + let bytes = zip_with(&[("data.json", &data), ("recovery.key", b"not-a-code")]); + let (export, cfg, key) = extract_zip(&bytes).expect("valid archive imports"); + assert_eq!(export.version, crate::models::db::CURRENT_EXPORT_VERSION); + assert!(cfg.is_none()); + assert!(key.is_none(), "non-parsing recovery code is dropped, not an error"); + } + + // ─── export v5 round-trip (passe D: QP versions + rejection counters) ──── + + #[tokio::test] + async fn export_v5_round_trips_qp_versions_and_rejection_counters() { + let mk_state = || async { + let db = std::sync::Arc::new(crate::db::Database::open_in_memory().unwrap()); + let cfg = std::sync::Arc::new(tokio::sync::RwLock::new(crate::core::config::default_config())); + crate::AppState::new_defaults(cfg, db, crate::DEFAULT_MAX_CONCURRENT_AGENTS) + }; + let source = mk_state().await; + + // Seed: one QP + one snapshot version + one armed rejection counter. + source.db.with_conn(|conn| { + let qp: crate::models::QuickPrompt = serde_json::from_value(serde_json::json!({ + "id": "qp-1", "name": "QP", "icon": "x", "prompt_template": "T {{v}}", + "variables": [], "agent": "ClaudeCode", "project_id": null, + "skill_ids": [], "profile_ids": [], "directive_ids": [], + "tier": "default", "description": "", + "created_at": chrono::Utc::now().to_rfc3339(), + "updated_at": chrono::Utc::now().to_rfc3339(), + })).unwrap(); + // insert_quick_prompt auto-snapshots version 1; a second snapshot + // gives the lineage a real history to round-trip. + crate::db::quick_prompts::insert_quick_prompt(conn, &qp)?; + crate::db::quick_prompts::snapshot_quick_prompt_version(conn, &qp)?; + crate::db::learnings::record_rejection(conn, "hash-1", "too vague")?; + crate::db::learnings::record_rejection(conn, "hash-1", "too vague")?; + Ok(()) + }).await.unwrap(); + + let export = build_export(&source).await.expect("export"); + assert_eq!(export.version, 5); + assert_eq!(export.quick_prompt_versions.len(), 2, "version lineage exported"); + assert_eq!(export.learning_rejections.len(), 1); + assert_eq!(export.learning_rejections[0].count, 2, "cumulative count exported"); + + // Import into a FRESH instance — both tables restored verbatim. + let target = mk_state().await; + do_import_db(&target, &export).await.expect("import"); + let (versions, rej_count) = target.db.with_conn(|conn| { + let v = crate::db::quick_prompts::list_quick_prompt_versions(conn, "qp-1")?; + let c = crate::db::learnings::rejection_count(conn, "hash-1")?; + Ok((v, c)) + }).await.unwrap(); + assert_eq!(versions.len(), 2, "version history survives the migration"); + assert_eq!(versions[0].version_index, 2, "newest first, indices preserved verbatim"); + assert_eq!(rej_count, 2, "anti-repetition threshold stays armed"); + } + + #[tokio::test] + async fn v4_import_preserves_local_lineage_and_prunes_orphans() { + // Codex review (export v5): a v4 archive carries quick_prompts but no + // versions — importing it must NOT wipe the local lineage of QPs it + // re-imports (same id), while versions of QPs absent from the archive + // are pruned with their parents. + let db = std::sync::Arc::new(crate::db::Database::open_in_memory().unwrap()); + let cfg = std::sync::Arc::new(tokio::sync::RwLock::new(crate::core::config::default_config())); + let state = crate::AppState::new_defaults(cfg, db, crate::DEFAULT_MAX_CONCURRENT_AGENTS); + + let mk_qp = |id: &str| -> crate::models::QuickPrompt { + serde_json::from_value(serde_json::json!({ + "id": id, "name": id, "icon": "x", "prompt_template": "T", + "variables": [], "agent": "ClaudeCode", "project_id": null, + "skill_ids": [], "profile_ids": [], "directive_ids": [], + "tier": "default", "description": "", + "created_at": chrono::Utc::now().to_rfc3339(), + "updated_at": chrono::Utc::now().to_rfc3339(), + })).unwrap() + }; + + // Local state: two versioned QPs. + let (kept, dropped) = (mk_qp("qp-kept"), mk_qp("qp-dropped")); + { + let (kept, dropped) = (kept.clone(), dropped.clone()); + state.db.with_conn(move |conn| { + crate::db::quick_prompts::insert_quick_prompt(conn, &kept)?; + crate::db::quick_prompts::insert_quick_prompt(conn, &dropped)?; + Ok(()) + }).await.unwrap(); + } + + // v4 archive: carries qp-kept only, no version lineage at all. + let mut v4 = empty_export(); + v4.version = 4; + v4.quick_prompts = vec![kept]; + do_import_db(&state, &v4).await.expect("v4 import"); + + let (kept_versions, dropped_versions) = state.db.with_conn(|conn| { + let k = crate::db::quick_prompts::list_quick_prompt_versions(conn, "qp-kept")?; + let d = crate::db::quick_prompts::list_quick_prompt_versions(conn, "qp-dropped")?; + Ok((k, d)) + }).await.unwrap(); + assert!(!kept_versions.is_empty(), "v4 import must NOT wipe local lineage of a re-imported QP"); + assert!(dropped_versions.is_empty(), "orphaned lineage (parent gone) is pruned"); + } + // ─── import_clear_statements (I10: selective clear, no downgrade wipe) ──── fn empty_export() -> crate::models::db::DbExport { @@ -1814,6 +2029,7 @@ mod tests { mcp_servers: vec![], mcp_configs: vec![], custom_skills: vec![], custom_directives: vec![], custom_profiles: vec![], contacts: vec![], quick_prompts: vec![], quick_apis: vec![], learnings: vec![], + quick_prompt_versions: vec![], learning_rejections: vec![], } } @@ -1934,6 +2150,8 @@ mod tests { let data = DbExport { version: 4, exported_at: Utc::now(), + quick_prompt_versions: vec![], + learning_rejections: vec![], projects: vec![], discussions: vec![], workflows: vec![], @@ -2067,7 +2285,7 @@ mod tests { state.db.with_conn(move |conn| crate::db::learnings::insert(conn, &l)).await.unwrap(); let export = build_export(&state).await.expect("build_export"); - assert_eq!(export.version, 4, "payload bumped to v4"); + assert_eq!(export.version, crate::models::db::CURRENT_EXPORT_VERSION, "payload bumped to the current export version"); assert_eq!(export.quick_apis.len(), 1, "quick_apis must be exported"); assert_eq!(export.learnings.len(), 1, "learnings must be exported"); @@ -2138,6 +2356,8 @@ mod tests { let data = DbExport { version: 4, exported_at: Utc::now(), + quick_prompt_versions: vec![], + learning_rejections: vec![], projects: vec![], discussions: vec![], workflows: vec![], diff --git a/backend/src/api/workflows.rs b/backend/src/api/workflows.rs index 4a58c9f7..589c1431 100644 --- a/backend/src/api/workflows.rs +++ b/backend/src/api/workflows.rs @@ -2247,6 +2247,76 @@ pub async fn decide_run( })) } +/// Response for [`resume_interrupted`]. +#[derive(Debug, serde::Serialize, ts_rs::TS)] +#[ts(export)] +pub struct ResumeRunResponse { + pub run_id: String, + pub new_status: RunStatus, +} + +/// POST /api/workflow-runs/:run_id/resume +/// +/// A2 — manually resume an `Interrupted` run (backend restart, crash). The +/// atomic `Interrupted → Running` claim happens BEFORE this responds, so a +/// double-click gets exactly one resume + one clear error. Execution then +/// continues in the background from the step after the last completed result, +/// re-attached to the preserved worktree (refused if that worktree is gone). +pub async fn resume_interrupted( + State(state): State, + Path(run_id): Path, +) -> Json> { + let run_id_for_db = run_id.clone(); + let run = match state + .db + .with_conn(move |conn| crate::db::workflows::get_run(conn, &run_id_for_db)) + .await + { + Ok(Some(r)) => r, + Ok(None) => return Json(ApiResponse::err_coded(ApiErrorCode::NotFound, "Run not found")), + Err(e) => return Json(ApiResponse::err(format!("DB error: {}", e))), + }; + + let wf_id = run.workflow_id.clone(); + let workflow = match state + .db + .with_conn(move |conn| crate::db::workflows::get_workflow(conn, &wf_id)) + .await + { + Ok(Some(wf)) => wf, + Ok(None) => return Json(ApiResponse::err_coded(ApiErrorCode::NotFound, "Workflow not found")), + Err(e) => return Json(ApiResponse::err(format!("DB error: {}", e))), + }; + + // Validation + atomic claim — awaited, so the caller's answer reflects + // whether THIS call won the run. + if let Err(e) = crate::workflows::runner::claim_interrupted_run(&state, &run).await { + return Json(ApiResponse::err(e.to_string())); + } + + let state_clone = state.clone(); + let run_id_for_log = run.id.clone(); + let mut run_mut = run.clone(); + tokio::spawn(async move { + let cfg = state_clone.config.read().await; + let tokens = cfg.tokens.clone(); + let agents = cfg.agents.clone(); + drop(cfg); + if let Err(e) = crate::workflows::runner::resume_interrupted_run( + state_clone.clone(), &workflow, &mut run_mut, &tokens, &agents, None, + ).await { + tracing::error!("Resume of interrupted run {} failed: {}", run_id_for_log, e); + } + // Same unattended-failure contract as the gate resume path. + crate::core::run_notify::notify_if_failed(&state_clone, &workflow, &run_mut).await; + }); + + Json(ApiResponse::ok(ResumeRunResponse { + run_id: run.id, + new_status: RunStatus::Running, + })) +} + /// Response for [`delete_batch_run`]. #[derive(Debug, serde::Serialize)] pub struct DeletedBatchResponse { diff --git a/backend/src/api_tests.rs b/backend/src/api_tests.rs index f5eb9c3d..ea341638 100644 --- a/backend/src/api_tests.rs +++ b/backend/src/api_tests.rs @@ -13,6 +13,7 @@ mod tests { use tokio::sync::RwLock; use tower::ServiceExt; // for `oneshot` + use serial_test::serial; use crate::{ build_router_with_auth, core::config::default_config, @@ -22,6 +23,22 @@ mod tests { // ─── Helper: build a test AppState with an in-memory DB ────────────────── + /// Handlers under test call config::save — without KRONN_DATA_DIR that + /// writes the developer's REAL config.toml (2026-07-13 incident; the + /// persist_atomic guard now panics instead). + fn isolate_config_dir() { + // Called ONLY by the #[serial] tests whose handlers SAVE config — + // a global call from test_state() would mutate KRONN_DATA_DIR from + // 80 non-serial tests and race the serialized env family. Once-guarded + // so repeated calls never re-mutate the env mid-run (Copilot, PR 116). + static INIT: std::sync::Once = std::sync::Once::new(); + INIT.call_once(|| { + let dir = std::env::temp_dir().join(format!("kronn-libtest-cfg-{}", std::process::id())); + std::fs::create_dir_all(&dir).ok(); + std::env::set_var("KRONN_DATA_DIR", &dir); + }); + } + fn test_state() -> AppState { let db = Arc::new(Database::open_in_memory().expect("in-memory DB")); let config_arc = Arc::new(RwLock::new(default_config())); @@ -411,7 +428,9 @@ mod tests { } #[tokio::test] + #[serial] async fn config_language_set_and_get() { + isolate_config_dir(); let state = test_state(); // Set language to "en" @@ -784,7 +803,9 @@ mod tests { } #[tokio::test] + #[serial] async fn agents_toggle_changes_state() { + isolate_config_dir(); let state = test_state(); // Toggle Vibe off @@ -860,7 +881,9 @@ mod tests { // ─── Q13: Config API additional tests ───────────────────────────────────── #[tokio::test] + #[serial] async fn config_server_get_and_set() { + isolate_config_dir(); let state = test_state(); // GET current server config @@ -2142,7 +2165,9 @@ mod tests { /// existence is the frontend's job. Guards against a race where /// the frontend toggles a skill that was just deleted. #[tokio::test] + #[serial] async fn auto_trigger_toggle_unknown_skill_still_works() { + isolate_config_dir(); let state = test_state(); let req = Request::builder() .method("POST") @@ -2158,7 +2183,9 @@ mod tests { /// Sending the toggle twice ends at the original state (idempotent /// round-trip), and the GET endpoint returns the live list. #[tokio::test] + #[serial] async fn auto_trigger_toggle_flips_and_persists() { + isolate_config_dir(); let state = test_state(); // First toggle on kronn-docs → disables. diff --git a/backend/src/core/config.rs b/backend/src/core/config.rs index 2d949b36..b88db4d5 100644 --- a/backend/src/core/config.rs +++ b/backend/src/core/config.rs @@ -139,6 +139,28 @@ pub async fn save(config: &AppConfig) -> Result<()> { /// could truncate-then-fail and leave a corrupt config. Shared by `save()` /// and the migration re-save in `load()`. async fn persist_atomic(dir: PathBuf, path: PathBuf, content: String) -> Result<()> { + // Write chokepoint guard: a test writing config without KRONN_DATA_DIR + // clobbers the developer's REAL config.toml (a full `cargo test` wiped + // pseudo/avatar/model-tiers on the host, 2026-07-13). Two layers because + // integration binaries compile this lib WITHOUT cfg(test): the runtime + // check keys on the executable living in `target/*/deps/` — true for + // every cargo test/bench binary, never for `cargo run` or installed + // binaries. Reads stay free; only the destructive act is fenced. + #[cfg(test)] + if std::env::var("KRONN_DATA_DIR").is_err() { + panic!("test attempted to WRITE the real config.toml — set KRONN_DATA_DIR (tempdir) in this test"); + } + #[cfg(not(test))] + if std::env::var("KRONN_DATA_DIR").is_err() + && std::env::current_exe() + .map(|p| p.components().any(|c| c.as_os_str() == "deps")) + .unwrap_or(false) + { + anyhow::bail!( + "test binary attempted to WRITE the real config.toml — call isolate_config_dir() \ + (set KRONN_DATA_DIR to a tempdir) in this integration test" + ); + } tokio::task::spawn_blocking(move || write_config_atomic(&dir, &path, content.as_bytes())) .await .context("config write task panicked")? @@ -470,7 +492,20 @@ mod tests { "a second exclusive lock on the same data dir must be refused" ); drop(g1); // releasing lets a later acquire succeed - assert!(acquire_lock_in(&dir).is_ok(), "lock must be re-acquirable after release"); + // Retry briefly: under full-suite load the re-open can hit transient + // resource errors (EMFILE from parallel git/sqlite fds). A genuinely + // stuck lock still fails after the window — with the REAL error shown. + let mut last_err = None; + for _ in 0..20 { + match acquire_lock_in(&dir) { + Ok(_) => { last_err = None; break; } + Err(e) => { + last_err = Some(e); + std::thread::sleep(std::time::Duration::from_millis(10)); + } + } + } + assert!(last_err.is_none(), "lock must be re-acquirable after release: {last_err:?}"); let _ = std::fs::remove_dir_all(&dir); } diff --git a/backend/src/db/contacts.rs b/backend/src/db/contacts.rs index 6ce4f93d..7a96a5c4 100644 --- a/backend/src/db/contacts.rs +++ b/backend/src/db/contacts.rs @@ -52,6 +52,25 @@ pub fn get_contact(conn: &Connection, id: &str) -> Result> { Ok(rows.next().and_then(|r| r.ok())) } +/// Passe D — the ONE sanctioned way to authenticate a P2P caller by invite +/// code: a known contact whose status is `accepted`. A pending/refused +/// contact keeps its code but must not pass the auth-exempt routes +/// (claim-by-token, fetch-file). `NotAccepted` carries the real status for +/// telemetry; callers MUST answer exactly like `Unknown` (no oracle). +pub enum InviteAuth { + Accepted(Contact), + NotAccepted { pseudo: String, status: String }, + Unknown, +} + +pub fn authenticate_invite_code(conn: &Connection, invite_code: &str) -> Result { + Ok(match find_contact_by_invite_code(conn, invite_code)? { + Some(c) if c.status == "accepted" => InviteAuth::Accepted(c), + Some(c) => InviteAuth::NotAccepted { pseudo: c.pseudo, status: c.status }, + None => InviteAuth::Unknown, + }) +} + pub fn find_contact_by_invite_code(conn: &Connection, invite_code: &str) -> Result> { let mut stmt = conn.prepare( "SELECT id, pseudo, avatar_email, kronn_url, invite_code, status, created_at, updated_at @@ -131,6 +150,35 @@ mod tests { conn } + #[test] + fn only_accepted_contacts_authenticate_by_invite_code() { + // Passe D (Codex constat n°1) — pending/refused contacts keep their + // code but must not pass invite-code auth; the enum carries the real + // status so callers can log it without re-implementing the rule. + let conn = test_conn(); + for (id, code, status) in [ + ("a", "kr-a", "accepted"), ("b", "kr-b", "pending"), ("c", "kr-c", "refused"), + ] { + insert_contact(&conn, &crate::models::Contact { + id: id.into(), + pseudo: format!("p-{id}"), + avatar_email: None, + kronn_url: "http://x".into(), + invite_code: code.into(), + status: status.into(), + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + }).unwrap(); + } + assert!(matches!(authenticate_invite_code(&conn, "kr-a").unwrap(), + InviteAuth::Accepted(c) if c.id == "a")); + assert!(matches!(authenticate_invite_code(&conn, "kr-b").unwrap(), + InviteAuth::NotAccepted { ref status, .. } if status == "pending")); + assert!(matches!(authenticate_invite_code(&conn, "kr-c").unwrap(), + InviteAuth::NotAccepted { ref status, .. } if status == "refused")); + assert!(matches!(authenticate_invite_code(&conn, "kr-ghost").unwrap(), InviteAuth::Unknown)); + } + #[test] fn parse_invite_code_valid() { let (pseudo, url) = parse_invite_code("kronn:testuser@100.64.1.5:3456").unwrap(); diff --git a/backend/src/db/learnings.rs b/backend/src/db/learnings.rs index c2d09a2f..9bede318 100644 --- a/backend/src/db/learnings.rs +++ b/backend/src/db/learnings.rs @@ -259,6 +259,34 @@ pub fn record_rejection(conn: &Connection, claim_hash: &str, reason: &str) -> Re )?) } +/// Passe D (export v5) — every rejection counter row, for DB export. +pub fn list_rejections(conn: &Connection) -> Result> { + let mut stmt = conn.prepare( + "SELECT claim_hash, reason, count, last_at FROM learning_rejections", + )?; + let rows = stmt.query_map([], |r| { + Ok(crate::models::LearningRejection { + claim_hash: r.get(0)?, + reason: r.get(1)?, + count: r.get(2)?, + last_at: r.get(3)?, + }) + })?; + rows.collect::>>().map_err(Into::into) +} + +/// Passe D (import v5) — restore a rejection row VERBATIM (count and last_at +/// preserved — `record_rejection` would restart the counter at 1). +pub fn insert_rejection_row(conn: &Connection, rej: &crate::models::LearningRejection) -> Result<()> { + conn.execute( + "INSERT INTO learning_rejections (claim_hash, reason, count, last_at) \ + VALUES (?1, ?2, ?3, ?4) \ + ON CONFLICT(claim_hash) DO UPDATE SET reason = ?2, count = ?3, last_at = ?4", + params![rej.claim_hash, rej.reason, rej.count, rej.last_at], + )?; + Ok(()) +} + pub fn rejection_count(conn: &Connection, claim_hash: &str) -> Result { Ok(conn .query_row( diff --git a/backend/src/db/quick_prompts.rs b/backend/src/db/quick_prompts.rs index 4cef7e99..fc368317 100644 --- a/backend/src/db/quick_prompts.rs +++ b/backend/src/db/quick_prompts.rs @@ -181,6 +181,54 @@ pub fn snapshot_quick_prompt_version(conn: &Connection, qp: &QuickPrompt) -> Res Ok(next) } +/// Passe D (export v5) — every version row of every QP, for DB export. +pub fn list_all_quick_prompt_versions(conn: &Connection) -> Result> { + let mut ids = Vec::new(); + { + let mut stmt = conn.prepare("SELECT DISTINCT quick_prompt_id FROM quick_prompt_versions")?; + let rows = stmt.query_map([], |r| r.get::<_, String>(0))?; + for r in rows { ids.push(r?); } + } + let mut all = Vec::new(); + for id in ids { + all.extend(list_quick_prompt_versions(conn, &id)?); + } + Ok(all) +} + +/// Passe D (import v5) — insert a version row VERBATIM (id, version_index and +/// created_at preserved), unlike `snapshot_quick_prompt_version` which mints +/// the next index. Import-only. +pub fn insert_quick_prompt_version_row(conn: &Connection, v: &QuickPromptVersion) -> Result<()> { + let agent_str = serde_json::to_string(&v.agent)?; + let tier_str = serde_json::to_string(&v.tier)?; + conn.execute( + "INSERT INTO quick_prompt_versions ( + id, quick_prompt_id, version_index, name, icon, prompt_template, variables_json, + agent, project_id, skill_ids_json, profile_ids_json, directive_ids_json, + tier, description, created_at + ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15)", + params![ + v.id, + v.quick_prompt_id, + v.version_index as i64, + v.name, + v.icon, + v.prompt_template, + serde_json::to_string(&v.variables)?, + agent_str.trim_matches('"'), + v.project_id, + serde_json::to_string(&v.skill_ids)?, + serde_json::to_string(&v.profile_ids)?, + serde_json::to_string(&v.directive_ids)?, + tier_str.trim_matches('"'), + v.description, + v.created_at.to_rfc3339(), + ], + )?; + Ok(()) +} + /// Return all stored versions for a QP, newest first. pub fn list_quick_prompt_versions(conn: &Connection, qp_id: &str) -> Result> { let mut stmt = conn.prepare( diff --git a/backend/src/db/tests.rs b/backend/src/db/tests.rs index dbbdcdd1..db94c631 100644 --- a/backend/src/db/tests.rs +++ b/backend/src/db/tests.rs @@ -1471,6 +1471,160 @@ fn reconcile_stale_runs_flips_only_old_running_pending_to_interrupted() { assert_eq!(by_id("fresh").status, RunStatus::Interrupted); } +#[test] +fn set_run_state_key_merges_one_key_under_status_guard() { + use crate::models::RunStatus::*; + let conn = test_db(); + crate::db::workflows::insert_workflow(&conn, &sample_workflow("w1")).unwrap(); + let mut run = sample_run("r-state-1", "w1"); + run.status = Running; + run.state.insert("existing".into(), "kept".into()); + crate::db::workflows::insert_run(&conn, &run).unwrap(); + + // Merge lands while Running, without clobbering sibling keys. + assert!(crate::db::workflows::set_run_state_key(&conn, "r-state-1", "k", "v1", &[Running]).unwrap()); + let row = crate::db::workflows::get_run(&conn, "r-state-1").unwrap().unwrap(); + assert_eq!(row.state.get("k").map(String::as_str), Some("v1")); + assert_eq!(row.state.get("existing").map(String::as_str), Some("kept"), "sibling keys untouched"); + + // Refused once the run changed hands (Interrupted), row unchanged. + run.status = crate::models::RunStatus::Interrupted; + let snap = crate::db::workflows::RunProgressSnapshot::from_run(&row); + let mut snap = snap; snap.status = crate::models::RunStatus::Interrupted; + assert!(crate::db::workflows::update_run_progress(&conn, snap).unwrap()); + assert!(!crate::db::workflows::set_run_state_key(&conn, "r-state-1", "k", "v2", &[Running]).unwrap()); + let row = crate::db::workflows::get_run(&conn, "r-state-1").unwrap().unwrap(); + assert_eq!(row.state.get("k").map(String::as_str), Some("v1"), "guarded write must not land"); + + // Missing run → false. + assert!(!crate::db::workflows::set_run_state_key(&conn, "r-ghost", "k", "v", &[Running]).unwrap()); +} + +#[test] +fn append_foreach_done_builds_versioned_doc_and_freezes_off_running() { + use crate::models::RunStatus::*; + let conn = test_db(); + crate::db::workflows::insert_workflow(&conn, &sample_workflow("w1")).unwrap(); + let mut run = sample_run("r-fd-1", "w1"); + run.status = Running; + crate::db::workflows::insert_run(&conn, &run).unwrap(); + + for (idx, id) in [(0u32, "t-a"), (1u32, "t-b")] { + assert!(crate::db::workflows::append_foreach_done( + &conn, "r-fd-1", "fanout", + serde_json::json!({"idx": idx, "id": id, "status": "Success", "child_run_id": "c"}), + ).unwrap()); + } + let row = crate::db::workflows::get_run(&conn, "r-fd-1").unwrap().unwrap(); + let doc: serde_json::Value = + serde_json::from_str(row.state.get("__kronn.foreach_done.fanout").unwrap()).unwrap(); + assert_eq!(doc["v"], 1); + let items = doc["items"].as_array().unwrap(); + assert_eq!(items.len(), 2); + assert_eq!(items[1]["id"], "t-b"); + + // Frozen once the parent left Running. + let mut snap = crate::db::workflows::RunProgressSnapshot::from_run(&row); + snap.status = Cancelled; + assert!(crate::db::workflows::update_run_progress(&conn, snap).unwrap()); + assert!(!crate::db::workflows::append_foreach_done( + &conn, "r-fd-1", "fanout", + serde_json::json!({"idx": 2, "id": "t-late", "status": "Success", "child_run_id": "c"}), + ).unwrap(), "late child must not extend a frozen done-set"); +} + +#[test] +fn run_status_state_machine_is_locked_for_progress_snapshots() { + // Property test (A1): update_run_progress may move a row ONLY when it is + // Pending/Running, or when it rewrites the SAME status. Exhaustive over + // every (from, to) pair so a future transition can't slip in untested. + use crate::models::RunStatus::*; + let all = [Pending, Running, Success, Failed, Cancelled, WaitingApproval, StoppedByGuard, Interrupted]; + let conn = test_db(); + crate::db::workflows::insert_workflow(&conn, &sample_workflow("w1")).unwrap(); + + for (i, from) in all.iter().enumerate() { + for (j, to) in all.iter().enumerate() { + let id = format!("r-{i}-{j}"); + let mut run = sample_run(&id, "w1"); + run.status = from.clone(); + crate::db::workflows::insert_run(&conn, &run).unwrap(); + + run.status = to.clone(); + let snap = crate::db::workflows::RunProgressSnapshot::from_run(&run); + let allowed = crate::db::workflows::update_run_progress(&conn, snap).unwrap(); + + let expected = from == to || matches!(from, Pending | Running); + assert_eq!(allowed, expected, "snapshot {from:?} -> {to:?}"); + let row = crate::db::workflows::get_run(&conn, &id).unwrap().unwrap(); + let final_expected = if expected { to } else { from }; + assert_eq!(&row.status, final_expected, "row after {from:?} -> {to:?}"); + } + } +} + +#[test] +fn claim_run_status_requires_exact_from_and_single_winner() { + use crate::models::RunStatus::*; + let conn = test_db(); + crate::db::workflows::insert_workflow(&conn, &sample_workflow("w1")).unwrap(); + + // Exact-from required: claiming Interrupted -> Running on a Running row fails. + let mut run = sample_run("r-claim-1", "w1"); + run.status = Running; + crate::db::workflows::insert_run(&conn, &run).unwrap(); + assert!(!crate::db::workflows::claim_run_status(&conn, "r-claim-1", &Interrupted, &Running).unwrap()); + + // Right from → wins once; the second identical claim loses (double-click). + let mut run = sample_run("r-claim-2", "w1"); + run.status = Interrupted; + crate::db::workflows::insert_run(&conn, &run).unwrap(); + assert!(crate::db::workflows::claim_run_status(&conn, "r-claim-2", &Interrupted, &Running).unwrap()); + assert!(!crate::db::workflows::claim_run_status(&conn, "r-claim-2", &Interrupted, &Running).unwrap(), + "second claim must lose"); + let row = crate::db::workflows::get_run(&conn, "r-claim-2").unwrap().unwrap(); + assert_eq!(row.status, Running); + + // Wrapper keeps the gate contract. + let mut run = sample_run("r-claim-3", "w1"); + run.status = WaitingApproval; + crate::db::workflows::insert_run(&conn, &run).unwrap(); + assert!(crate::db::workflows::claim_waiting_run(&conn, "r-claim-3", &Running).unwrap()); +} + +#[test] +fn batch_counters_and_final_status_freeze_off_running() { + use crate::models::RunStatus::*; + let conn = test_db(); + crate::db::workflows::insert_workflow(&conn, &sample_workflow("w1")).unwrap(); + + for frozen in [Cancelled, Interrupted] { + let id = format!("r-batch-{frozen:?}"); + let mut run = sample_run(&id, "w1"); + run.status = frozen.clone(); + run.run_type = "batch".into(); + run.batch_total = 2; + run.batch_completed = 1; + crate::db::workflows::insert_run(&conn, &run).unwrap(); + + // A late child completion must not move counters nor status. + let out = crate::db::workflows::increment_batch_progress(&conn, &id, true).unwrap(); + assert!(out.is_none(), "late child on a {frozen:?} batch is a no-op"); + let row = crate::db::workflows::get_run(&conn, &id).unwrap().unwrap(); + assert_eq!(row.batch_completed, 1, "counter frozen on {frozen:?}"); + assert_eq!(row.status, frozen, "status frozen"); + } + + // Sanity: a Running batch still counts up and finalizes. + let mut run = sample_run("r-batch-live", "w1"); + run.status = Running; + run.run_type = "batch".into(); + run.batch_total = 1; + crate::db::workflows::insert_run(&conn, &run).unwrap(); + let out = crate::db::workflows::increment_batch_progress(&conn, "r-batch-live", true).unwrap().unwrap(); + assert_eq!(out.status, Success); +} + #[test] fn cancelled_status_is_sticky_in_update_run_progress() { // The user's forced cancel races the runner's own writes; the runner diff --git a/backend/src/db/workflows.rs b/backend/src/db/workflows.rs index 3d40cd80..139765b0 100644 --- a/backend/src/db/workflows.rs +++ b/backend/src/db/workflows.rs @@ -1,6 +1,6 @@ use anyhow::Result; use chrono::{DateTime, Utc}; -use rusqlite::{params, Connection}; +use rusqlite::{params, Connection, OptionalExtension}; use uuid::Uuid; use crate::models::*; @@ -820,10 +820,25 @@ pub fn increment_batch_progress( child_succeeded: bool, ) -> Result> { let column = if child_succeeded { "batch_completed" } else { "batch_failed" }; - conn.execute( - &format!("UPDATE workflow_runs SET {0} = {0} + 1 WHERE id = ?1 AND run_type = 'batch'", column), + // Counters are part of the terminal state (waiters/UI read them): a late + // child must not advance a batch already Cancelled/Interrupted. + let bumped = conn.execute( + &format!( + "UPDATE workflow_runs SET {0} = {0} + 1 WHERE id = ?1 AND run_type = 'batch' AND status = 'Running'", + column + ), params![run_id], )?; + if bumped == 0 { + // A′ observability — a frozen counter means a child completed after + // the batch left Running (late finisher post-cancel/terminal). + tracing::warn!( + target: "kronn::invariant", + run_id = %run_id, counter = %column, + "batch progress bump blocked — batch no longer Running" + ); + return Ok(None); + } // Re-read the run to check if we've reached batch_total. let Some(mut run) = get_run(conn, run_id)? else { return Ok(None); }; @@ -837,7 +852,7 @@ pub fn increment_batch_progress( let final_status = if run.batch_completed > 0 { RunStatus::Success } else { RunStatus::Failed }; let finished = chrono::Utc::now(); conn.execute( - "UPDATE workflow_runs SET status = ?2, finished_at = ?3 WHERE id = ?1", + "UPDATE workflow_runs SET status = ?2, finished_at = ?3 WHERE id = ?1 AND status = 'Running'", params![run_id, run_status_str(&final_status), finished.to_rfc3339()], )?; run.status = final_status; @@ -853,14 +868,123 @@ pub fn increment_batch_progress( /// double-click, or a human racing the auto-approve timer) used to both /// pass the read-then-check and spawn two concurrent `resume_run`s on the /// same run; the conditional UPDATE makes exactly one of them win. -pub fn claim_waiting_run(conn: &Connection, run_id: &str, new_status: &RunStatus) -> Result { +/// Atomic status claim: flips `from_status` → `new_status` iff the row still +/// holds `from_status`. Exactly ONE concurrent caller wins (TOCTOU-free). +/// The only sanctioned way OUT of `WaitingApproval` (gate decide) and +/// `Interrupted` (manual resume) — ordinary snapshots can't touch those. +/// Atomically merge ONE key into a run's durable `state` map, guarded by +/// status (A2). Unlike a full `update_run_progress` snapshot, this can't +/// clobber concurrent fields and can't move the run's status — it only lands +/// while the row still holds one of `allowed` (read+merge+write under the +/// single shared connection; the UPDATE re-checks the observed status). +/// Returns `false` when the run is gone or its status changed hands. +pub fn set_run_state_key( + conn: &Connection, + run_id: &str, + key: &str, + value: &str, + allowed: &[RunStatus], +) -> Result { + let row: Option<(Option, String)> = conn + .query_row( + "SELECT state, status FROM workflow_runs WHERE id = ?1", + params![run_id], + |r| Ok((r.get(0)?, r.get(1)?)), + ) + .optional()?; + let Some((state_json, status_s)) = row else { return Ok(false) }; + let status = parse_run_status(&status_s); + if !allowed.contains(&status) { + return Ok(false); + } + let mut map: std::collections::HashMap = state_json + .as_deref() + .and_then(|s| serde_json::from_str(s).ok()) + .unwrap_or_default(); + map.insert(key.to_string(), value.to_string()); let n = conn.execute( - "UPDATE workflow_runs SET status = ?2 WHERE id = ?1 AND status = 'WaitingApproval'", - params![run_id, run_status_str(new_status)], + "UPDATE workflow_runs SET state = ?2 WHERE id = ?1 AND status = ?3", + params![run_id, serde_json::to_string(&map)?, status_s], )?; Ok(n == 1) } +/// A2 — append one entry to the versioned foreach done-set +/// (`__kronn.foreach_done.` → `{"v":1,"items":[…]}`), only while the +/// parent is still `Running`. One connection borrow = atomic read+append+write. +pub fn append_foreach_done( + conn: &Connection, + run_id: &str, + step_name: &str, + entry: serde_json::Value, +) -> Result { + let key = format!("__kronn.foreach_done.{step_name}"); + let cur: Option> = conn + .query_row( + "SELECT state FROM workflow_runs WHERE id = ?1", + params![run_id], + |r| r.get(0), + ) + .optional()?; + let Some(state_json) = cur else { return Ok(false) }; + let map: std::collections::HashMap = state_json + .as_deref() + .and_then(|s| serde_json::from_str(s).ok()) + .unwrap_or_default(); + let mut doc: serde_json::Value = map + .get(&key) + .and_then(|v| serde_json::from_str(v).ok()) + .unwrap_or_else(|| serde_json::json!({"v": 1, "items": []})); + if let Some(items) = doc.get_mut("items").and_then(|i| i.as_array_mut()) { + items.push(entry); + } + set_run_state_key(conn, run_id, &key, &doc.to_string(), &[RunStatus::Running]) +} + +/// A2 — item ids of this parent's `Success` sub-workflow children (read off +/// `trigger_context.__subwf_item_id__`). Resume reconciliation source #2: +/// a child that finished right before the crash counts as done even when the +/// parent died before writing its done-set entry. +pub fn successful_child_item_ids( + conn: &Connection, + parent_run_id: &str, +) -> Result> { + let mut stmt = conn.prepare( + "SELECT trigger_context FROM workflow_runs + WHERE parent_run_id = ?1 AND run_type = 'subworkflow' AND status = 'Success'", + )?; + let rows = stmt.query_map(params![parent_run_id], |r| r.get::<_, Option>(0))?; + let mut ids = std::collections::HashSet::new(); + for row in rows { + if let Some(id) = row? + .as_deref() + .and_then(|s| serde_json::from_str::(s).ok()) + .and_then(|v| v.get("__subwf_item_id__").and_then(|i| i.as_str()).map(String::from)) + .filter(|i| !i.is_empty()) + { + ids.insert(id); + } + } + Ok(ids) +} + +pub fn claim_run_status( + conn: &Connection, + run_id: &str, + from_status: &RunStatus, + new_status: &RunStatus, +) -> Result { + let n = conn.execute( + "UPDATE workflow_runs SET status = ?3 WHERE id = ?1 AND status = ?2", + params![run_id, run_status_str(from_status), run_status_str(new_status)], + )?; + Ok(n == 1) +} + +pub fn claim_waiting_run(conn: &Connection, run_id: &str, new_status: &RunStatus) -> Result { + claim_run_status(conn, run_id, &RunStatus::WaitingApproval, new_status) +} + pub fn update_run(conn: &Connection, run: &WorkflowRun) -> Result<()> { // Same Cancelled-stickiness semantics; callers of this convenience // wrapper don't act on the raced-cancel signal. @@ -900,22 +1024,23 @@ impl RunProgressSnapshot { } } -/// Returns `false` when the row was NOT updated — either it no longer exists, -/// or the `Cancelled` stickiness guard blocked the write. +/// Returns `false` when the row was NOT updated — it no longer exists, or the +/// state-machine guard blocked the write. /// -/// `Cancelled` is STICKY: a user's forced cancel (api cancel_run) races the -/// runner's own progress/terminal writes, and the runner used to win — -/// overwriting `Cancelled` with `Running` (gate-resume window) or `Failed` -/// (rollback-chain tail) minutes after the user was told "stopped". Only a -/// `Cancelled` write may touch a `Cancelled` row. Runner lifecycle sites -/// treat `false` as "cancelled beneath us" and stop. +/// Guard (0.8.11 A1): a progress snapshot may only land when the row is +/// `Pending`/`Running`, or when it rewrites the SAME status (idempotence). +/// Every terminal status is sticky (a zombie runner's late snapshot could +/// resurrect `Cancelled`/`Interrupted`/`Failed`/`Success`), and +/// `WaitingApproval` only leaves via the claim family (`claim_run_status`), +/// never via an ordinary snapshot. Runner sites treat `false` as "the run +/// changed hands beneath us" and stop. pub fn update_run_progress(conn: &Connection, snap: RunProgressSnapshot) -> Result { let new_status = run_status_str(&snap.status); let affected = conn.execute( "UPDATE workflow_runs SET status = ?2, step_results_json = ?3, tokens_used = ?4, workspace_path = ?5, finished_at = ?6, state = ?7, produced_branches = ?8 - WHERE id = ?1 AND (status != 'Cancelled' OR ?2 = 'Cancelled')", + WHERE id = ?1 AND (status = ?2 OR status IN ('Pending', 'Running'))", params![ snap.id, new_status, @@ -927,6 +1052,30 @@ pub fn update_run_progress(conn: &Connection, snap: RunProgressSnapshot) -> Resu if snap.produced_branches.is_empty() { None } else { Some(serde_json::to_string(&snap.produced_branches)?) }, ], )?; + if affected == 0 { + // A′ observability — a blocked write is either the stickiness guard + // doing its job (late runner write racing a Cancel) or a genuinely + // impossible transition; both must be visible, not silent. + let held: Option = conn + .query_row( + "SELECT status FROM workflow_runs WHERE id = ?1", + params![snap.id], + |r| r.get(0), + ) + .optional()?; + match held { + Some(held) => tracing::warn!( + target: "kronn::invariant", + run_id = %snap.id, held = %held, attempted = %new_status, + "run progress write blocked by status guard" + ), + None => tracing::warn!( + target: "kronn::invariant", + run_id = %snap.id, attempted = %new_status, + "run progress write targets a missing run row" + ), + } + } Ok(affected > 0) } diff --git a/backend/src/lib.rs b/backend/src/lib.rs index e981983a..04736adc 100644 --- a/backend/src/lib.rs +++ b/backend/src/lib.rs @@ -309,6 +309,7 @@ async fn auth_middleware( }; if auth_allows( + request.method(), request.uri().path(), auth_enabled, expected_token.is_some(), @@ -320,18 +321,32 @@ async fn auth_middleware( Err(StatusCode::UNAUTHORIZED) } -/// Endpoints that irreversibly destroy user data (wipe the DB, clobber config) -/// or change the encryption-key material. Gated even when app-wide auth is -/// disabled — see `auth_allows`. `recovery/set` reads the active key and writes -/// a recovery blob; `recovery/restore` swaps the live key — both must never be -/// callable by an unauthenticated LAN peer. -const DESTRUCTIVE_PATHS: &[&str] = &[ +/// POST endpoints that irreversibly destroy user data, change key material or +/// mutate the host system. Gated even when app-wide auth is disabled — see +/// `auth_allows`. DELETE routes don't need listing: the VERB itself is the +/// criterion (passe D) — a hand-maintained path list diverged from the route +/// table as soon as new deletes were added. +const DESTRUCTIVE_POSTS: &[&str] = &[ "/api/setup/reset", "/api/config/import", "/api/config/recovery/set", "/api/config/recovery/restore", + "/api/audit-runs/cleanup", + "/api/api-call-logs/purge", + "/api/debug/logs/clear", + "/api/agents/uninstall", + "/api/rtk/deactivate", ]; +/// Passe D — the destructive-request criterion: every DELETE (no benign DELETE +/// exists in this API), the listed POSTs, and the parameterized +/// `/api/mcps/custom/{id}/cleanup-orphan-env`. +fn is_destructive(method: &axum::http::Method, path: &str) -> bool { + method == axum::http::Method::DELETE + || DESTRUCTIVE_POSTS.contains(&path) + || path.ends_with("/cleanup-orphan-env") +} + /// Pure auth decision for a request that already cleared the always-open /// exceptions (health, ws, claim-by-token). `true` = allow. /// @@ -341,13 +356,14 @@ const DESTRUCTIVE_PATHS: &[&str] = &[ /// wipe the instance. Everything else keeps the historical behaviour: auth off → /// open; no token configured → open; otherwise local-bypass-or-token. fn auth_allows( + method: &axum::http::Method, path: &str, auth_enabled: bool, token_configured: bool, local_trusted: bool, has_valid_token: bool, ) -> bool { - if DESTRUCTIVE_PATHS.contains(&path) { + if is_destructive(method, path) { return local_trusted || has_valid_token; } if !auth_enabled { @@ -363,9 +379,15 @@ fn auth_allows( /// nginx sets (Docker), or the direct peer address (Tauri desktop / no proxy). /// Does NOT apply the strict-localhost policy; callers combine it as needed. fn request_is_local_ip(headers: &HeaderMap, request: &axum::extract::Request) -> bool { - if let Some(real_ip) = headers.get("x-real-ip").and_then(|v| v.to_str().ok()) { - if is_local_ip(real_ip) { - return true; + // Passe D — `X-Real-IP` is only trustworthy when the bundled nginx sets + // it (Docker). On a native bind axum talks to clients DIRECTLY, so the + // header is attacker-controlled: a LAN peer sending `X-Real-IP: + // 127.0.0.1` used to mint local trust and bypass the destructive gate. + if crate::core::env::is_docker() { + if let Some(real_ip) = headers.get("x-real-ip").and_then(|v| v.to_str().ok()) { + if is_local_ip(real_ip) { + return true; + } } } if let Some(ci) = request @@ -739,6 +761,7 @@ pub fn build_router_with_auth(state: AppState, enable_auth: bool) -> Router { .route("/api/workflows/import", post(api::workflows::import_workflow)) .route("/api/workflow-runs/batch-summaries", get(api::workflows::list_batch_run_summaries)) .route("/api/workflow-runs/{run_id}", delete(api::workflows::delete_batch_run)) + .route("/api/workflow-runs/{run_id}/resume", post(api::workflows::resume_interrupted)) // ── Quick Prompts ── .route("/api/quick-prompts", get(api::quick_prompts::list).post(api::quick_prompts::create)) .route("/api/quick-prompts/{id}", put(api::quick_prompts::update).delete(api::quick_prompts::delete)) @@ -1009,8 +1032,9 @@ mod audit_tracker_tests { #[cfg(test)] mod auth_tests { use super::{auth_allows, is_local_ip}; + use axum::http::Method; - // ── auth_allows decision matrix (I9: destructive-op gating) ────────────── + // ── auth_allows decision matrix (I9 + passe D: destructive-op gating) ──── const RESET: &str = "/api/setup/reset"; const IMPORT: &str = "/api/config/import"; const NORMAL: &str = "/api/projects"; @@ -1019,17 +1043,17 @@ mod auth_tests { fn destructive_from_remote_is_denied_even_when_auth_off() { // THE fix: Docker default (auth off, 0.0.0.0). A LAN peer — not local, // no token — must NOT be able to wipe/overwrite the instance. - assert!(!auth_allows(RESET, false, true, false, false)); - assert!(!auth_allows(IMPORT, false, true, false, false)); + assert!(!auth_allows(&Method::POST, RESET, false, true, false, false)); + assert!(!auth_allows(&Method::POST, IMPORT, false, true, false, false)); // …and still denied even if no token is configured at all. - assert!(!auth_allows(RESET, false, false, false, false)); + assert!(!auth_allows(&Method::POST, RESET, false, false, false, false)); } #[test] fn destructive_allowed_for_local_or_valid_token() { - assert!(auth_allows(RESET, false, true, true, false), "local host may reset"); - assert!(auth_allows(RESET, false, true, false, true), "valid token may reset"); - assert!(auth_allows(IMPORT, true, true, true, false), "local host may import (auth on)"); + assert!(auth_allows(&Method::POST, RESET, false, true, true, false), "local host may reset"); + assert!(auth_allows(&Method::POST, RESET, false, true, false, true), "valid token may reset"); + assert!(auth_allows(&Method::POST, IMPORT, true, true, true, false), "local host may import (auth on)"); } #[test] @@ -1037,24 +1061,47 @@ mod auth_tests { // recovery/set reads the active key, recovery/restore swaps it — both // must be denied to an unauthenticated remote peer even with auth off. for p in ["/api/config/recovery/set", "/api/config/recovery/restore"] { - assert!(!auth_allows(p, false, true, false, false), "{p} must deny remote+no-token"); - assert!(auth_allows(p, false, true, true, false), "{p} allows local"); - assert!(auth_allows(p, false, true, false, true), "{p} allows valid token"); + assert!(!auth_allows(&Method::POST, p, false, true, false, false), "{p} must deny remote+no-token"); + assert!(auth_allows(&Method::POST, p, false, true, true, false), "{p} allows local"); + assert!(auth_allows(&Method::POST, p, false, true, false, true), "{p} allows valid token"); + } + } + + #[test] + fn every_delete_is_gated_even_when_auth_off() { + // Passe D — the VERB is the criterion: a LAN peer on an auth-off bind + // must not delete projects/discussions/workflows/… whatever the path. + for p in ["/api/projects/p1", "/api/discussions/d1", "/api/workflow-runs/r1", "/api/anything/new"] { + assert!(!auth_allows(&Method::DELETE, p, false, true, false, false), "{p} DELETE must deny remote"); + assert!(auth_allows(&Method::DELETE, p, false, true, true, false), "{p} DELETE allows local"); + assert!(auth_allows(&Method::DELETE, p, false, true, false, true), "{p} DELETE allows token"); + } + } + + #[test] + fn destructive_posts_inventory_is_gated() { + // Passe D — system mutations + purges reachable by POST. + for p in ["/api/audit-runs/cleanup", "/api/api-call-logs/purge", + "/api/debug/logs/clear", "/api/agents/uninstall", "/api/rtk/deactivate", + "/api/mcps/custom/srv-1/cleanup-orphan-env"] { + assert!(!auth_allows(&Method::POST, p, false, true, false, false), "{p} must deny remote+no-token"); + assert!(auth_allows(&Method::POST, p, false, true, true, false), "{p} allows local"); } } #[test] fn non_destructive_keeps_historical_behaviour() { - // auth off → open. - assert!(auth_allows(NORMAL, false, true, false, false)); + // auth off → open (GET and ordinary POST alike). + assert!(auth_allows(&Method::GET, NORMAL, false, true, false, false)); + assert!(auth_allows(&Method::POST, NORMAL, false, true, false, false)); // auth on, no token configured → open (first-run/back-compat). - assert!(auth_allows(NORMAL, true, false, false, false)); + assert!(auth_allows(&Method::GET, NORMAL, true, false, false, false)); // auth on, token configured, remote & no token → denied. - assert!(!auth_allows(NORMAL, true, true, false, false)); + assert!(!auth_allows(&Method::GET, NORMAL, true, true, false, false)); // auth on, local trusted → open. - assert!(auth_allows(NORMAL, true, true, true, false)); + assert!(auth_allows(&Method::GET, NORMAL, true, true, true, false)); // auth on, valid token → open. - assert!(auth_allows(NORMAL, true, true, false, true)); + assert!(auth_allows(&Method::GET, NORMAL, true, true, false, true)); } // Localhost auth-bypass relies entirely on `is_local_ip`. A diff --git a/backend/src/main.rs b/backend/src/main.rs index 4b0358a2..f114d006 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -246,6 +246,7 @@ async fn main() -> anyhow::Result<()> { }).await { Ok(0) => tracing::info!("Orphan scan: nothing to clean up (boot reconcile already ran)"), Ok(n) => tracing::warn!( + target: "kronn::invariant", "Orphan scan: {n} workflow_runs still Running/Pending AFTER the boot reconcile — \ this should be impossible, investigate db::open_path ordering" ), diff --git a/backend/src/models/db.rs b/backend/src/models/db.rs index ce1ff716..94e427a5 100644 --- a/backend/src/models/db.rs +++ b/backend/src/models/db.rs @@ -8,7 +8,7 @@ use ts_rs::TS; use super::{ AgentProfile, Contact, Directive, Discussion, Learning, McpConfig, McpServer, Project, - QuickApi, QuickPrompt, Skill, Workflow, + LearningRejection, QuickApi, QuickPrompt, QuickPromptVersion, Skill, Workflow, }; #[derive(Debug, Serialize, TS)] @@ -29,7 +29,7 @@ pub struct DbInfo { /// Current export schema version. Bump when a new table/field is added to /// `DbExport` so import can WARN when restoring an older backup (whose missing /// tables must NOT wipe newer data — see `do_import_db`'s selective clear). -pub const CURRENT_EXPORT_VERSION: u32 = 4; +pub const CURRENT_EXPORT_VERSION: u32 = 5; #[derive(Debug, Serialize, Deserialize, TS)] #[ts(export)] @@ -62,6 +62,13 @@ pub struct DbExport { /// facts/preferences). Same back-compat default as `quick_apis`. #[serde(default)] pub learnings: Vec, + /// v5 (passe D) — QP version history; without it, imports silently lost + /// the version metrics lineage. `default` keeps v4 exports importable. + #[serde(default)] + pub quick_prompt_versions: Vec, + /// v5 (passe D) — anti-repetition rejection counters for learnings. + #[serde(default)] + pub learning_rejections: Vec, } #[derive(Debug, Serialize, Deserialize, TS)] diff --git a/backend/src/models/learnings.rs b/backend/src/models/learnings.rs index 89b279f1..a752c6ff 100644 --- a/backend/src/models/learnings.rs +++ b/backend/src/models/learnings.rs @@ -77,6 +77,18 @@ pub struct Evidence { pub quote: Option, } +/// One row of `learning_rejections` — the anti-repetition counter keyed by +/// claim hash. Exported/imported with the DB (passe D: losing it reset the +/// auto-reject threshold after a migration). +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, TS)] +#[ts(export)] +pub struct LearningRejection { + pub claim_hash: String, + pub reason: String, + pub count: i64, + pub last_at: String, +} + /// A continual-learning candidate (table `learnings`). #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, TS)] #[ts(export)] diff --git a/backend/src/models/workflows.rs b/backend/src/models/workflows.rs index b0d04331..424caf4f 100644 --- a/backend/src/models/workflows.rs +++ b/backend/src/models/workflows.rs @@ -743,6 +743,20 @@ pub enum PaginationSpec { max_pages: Option, }, + /// GitHub-style: the continuation signal is the `Link: <…>; rel="next"` + /// RESPONSE HEADER, not a body field, and the body is typically a bare + /// top-level array. Walks `rel="next"` until absent. `page_size_param` / + /// `page_size` seed page 1 (e.g. `per_page=100`); later pages reuse the + /// server's own next URL verbatim. + LinkHeader { + #[serde(default, skip_serializing_if = "Option::is_none")] + page_size_param: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + page_size: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + max_pages: Option, + }, + /// Page number: increment `page_param` from 1, stop when `has_more_path` /// is false or there are no more results. Page { @@ -1077,6 +1091,22 @@ pub enum RunStatus { Interrupted, } +impl RunStatus { + /// Canonical terminal set — the states no ordinary write may leave. + /// Exhaustive match on purpose: adding a variant breaks compilation here, + /// so a future status can't silently escape the state-machine guards. + pub fn is_terminal(&self) -> bool { + match self { + RunStatus::Success + | RunStatus::Failed + | RunStatus::Cancelled + | RunStatus::StoppedByGuard + | RunStatus::Interrupted => true, + RunStatus::Pending | RunStatus::Running | RunStatus::WaitingApproval => false, + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize, TS)] #[ts(export)] pub struct StepResult { diff --git a/backend/src/workflows/api_call_executor.rs b/backend/src/workflows/api_call_executor.rs index 608e29cb..9676aa6f 100644 --- a/backend/src/workflows/api_call_executor.rs +++ b/backend/src/workflows/api_call_executor.rs @@ -1055,12 +1055,20 @@ async fn walk_pages( current_query.entry(page_param.clone()).or_insert_with(|| "1".to_string()); current_query.entry(page_size_param.clone()).or_insert_with(|| page_size.to_string()); } + PaginationSpec::LinkHeader { page_size_param: Some(p), page_size: Some(n), .. } => { + current_query.entry(p.clone()).or_insert_with(|| n.to_string()); + } _ => {} } let mut first_response: Option = None; let mut items_key: Option = None; + // C2 — GitHub-style bodies are a bare top-level array: no items key, the + // whole body IS the page. Merged result is then a plain array too. + let mut bare_array = false; let mut accumulated_items: Vec = Vec::new(); + let mut next_link: Option = None; + let mut page_url_base = base_url.clone(); let mut truncated = false; for page_idx in 0..max_pages { @@ -1079,17 +1087,24 @@ async fn walk_pages( current_query.insert(page_param.clone(), next_page_num.to_string()); current_query.insert(page_size_param.clone(), page_size.to_string()); } + PaginationSpec::LinkHeader { .. } => { + let Some(next) = next_link.take() else { break }; + // The server's next URL carries its own cursor params — + // adopt them wholesale (rebuild_query clears the query). + current_query = next.query_pairs().into_owned().collect(); + page_url_base = next; + } _ => break, } } - let url = rebuild_query(&base_url, ¤t_query, &auth.query)?; + let url = rebuild_query(&page_url_base, ¤t_query, &auth.query)?; // Rate-limit gate BEFORE every HTTP page — siblings in a batch // fan-out compete for the same bucket. super::api_call_ratelimit::acquire_slot(plugin_slug, config_id).await; - let resp = send_with_retry( + let (resp, link_header) = send_with_retry( method.clone(), &url, auth, @@ -1105,11 +1120,18 @@ async fn walk_pages( if matches!(pagination, PaginationSpec::None | PaginationSpec::Auto { .. }) { return Ok((resp, false)); } - items_key = detect_items_key(&resp); + bare_array = resp.is_array(); + if !bare_array { + items_key = detect_items_key(&resp); + } first_response = Some(resp.clone()); } - if let Some(key) = items_key.as_ref() { + if bare_array { + if let Value::Array(page_items) = resp.clone() { + accumulated_items.extend(page_items); + } + } else if let Some(key) = items_key.as_ref() { let page_items = extract_array_at(&resp, key); accumulated_items.extend(page_items); } @@ -1134,6 +1156,29 @@ async fn walk_pages( if !has_more { break; } next_page_num += 1; } + PaginationSpec::LinkHeader { .. } => { + // Resolve against the CURRENT page URL (relative links are + // RFC-legal), then enforce SAME ORIGIN as the plugin's base: + // the next request re-sends the plugin's auth, so following a + // cross-origin `rel="next"` from a compromised/misbehaving + // server would exfiltrate the credentials to that host. + let next = link_header.as_deref().and_then(parse_link_next) + .and_then(|u| page_url_base.join(&u).ok()); + let Some(next) = next else { break }; + let same_origin = next.scheme() == base_url.scheme() + && next.host() == base_url.host() + && next.port_or_known_default() == base_url.port_or_known_default(); + if !same_origin { + tracing::warn!( + target: "kronn::invariant", + plugin = %plugin_slug, config = %config_id, + next = %redact_url_query(&next), base = %redact_url_query(&base_url), + "Link rel=next points OFF-ORIGIN — refusing to follow (auth would leak to that host)" + ); + break; + } + next_link = Some(next); + } _ => break, } @@ -1154,7 +1199,9 @@ async fn walk_pages( // accumulator. Keeps the rest of the body (counters, metadata) intact // so callers reading `total` / pagination cursors still see them. let mut final_resp = first_response.unwrap_or(Value::Null); - if let (Some(key), true) = (items_key.as_deref(), !accumulated_items.is_empty()) { + if bare_array { + final_resp = Value::Array(accumulated_items); + } else if let (Some(key), true) = (items_key.as_deref(), !accumulated_items.is_empty()) { if let Value::Object(map) = &mut final_resp { map.insert(key.to_string(), Value::Array(accumulated_items)); } @@ -1212,6 +1259,21 @@ fn jsonpath_first_bool(value: &Value, path: &str) -> Option { /// Re-builds a `Url` with a fresh query string. Used by the paginated /// walker to inject updated `startAt` / `cursor` / `page` params on each /// loop iteration without re-parsing the base URL. +/// Extract the `rel="next"` target from a raw `Link` header +/// (`; rel="next", <…>; rel="last"`). +fn parse_link_next(link: &str) -> Option { + for part in link.split(',') { + let Some((target, params)) = part.split_once(';') else { continue }; + if params.split(';').any(|p| { + let p = p.trim(); + p == "rel=\"next\"" || p == "rel=next" + }) { + return Some(target.trim().trim_start_matches('<').trim_end_matches('>').to_string()); + } + } + None +} + fn rebuild_query( base: &Url, step_query: &HashMap, @@ -1239,6 +1301,8 @@ fn rebuild_query( /// Send with exponential backoff on 5xx + 429. 4xx is a *client* error /// and retrying never helps — we fail fast and surface the status + body /// excerpt so the user can fix their params. +/// Returns the parsed body plus the raw `Link` response header (C2 — the +/// only continuation signal GitHub-style list APIs give is in the headers). async fn send_with_retry( method: Method, url: &Url, @@ -1247,7 +1311,7 @@ async fn send_with_retry( body: Option<&Value>, timeout: Duration, max_retries: u8, -) -> Result { +) -> Result<(Value, Option), String> { // 0.8.2 — Explicit User-Agent. GitHub REQUIRES one (returns 403 // "Request forbidden by administrative rules" without it — see // https://docs.github.com/en/rest/overview/resources-in-the-rest-api#user-agent-required). @@ -1305,9 +1369,16 @@ async fn send_with_retry( let status = response.status(); if status.is_success() { - return response.json::().await.map_err(|e| { - format!("Response JSON parse failed ({}): {e}", status.as_u16()) - }); + let link = response + .headers() + .get(reqwest::header::LINK) + .and_then(|v| v.to_str().ok()) + .map(String::from); + return response + .json::() + .await + .map(|v| (v, link)) + .map_err(|e| format!("Response JSON parse failed ({}): {e}", status.as_u16())); } // Non-success. Retry only on 5xx + 429, never 4xx. @@ -2337,6 +2408,161 @@ mod tests { assert_eq!(envelope["data"], json!(["K-1", "K-2", "K-3", "K-4", "K-5"])); } + #[test] + fn parse_link_next_extracts_the_next_rel() { + // GitHub's real shape: several rels, next not necessarily first. + let link = r#"; rel="prev", ; rel="next", ; rel="last""#; + assert_eq!( + parse_link_next(link).as_deref(), + Some("https://api.github.com/repos/o/r/pulls/1/reviews?page=5"), + ); + // Last page: no rel="next" at all. + assert_eq!(parse_link_next(r#"; rel="first""#), None); + // Unquoted rel param (RFC-legal) + garbage segments are tolerated. + assert_eq!(parse_link_next("garbage, ; rel=next").as_deref(), Some("https://x/?page=2")); + assert_eq!(parse_link_next(""), None); + } + + #[tokio::test] + async fn walk_pages_link_header_merges_bare_array_pages() { + // GitHub-style: bare top-level array + Link header continuation. + // The exact TD-20260630 case — reviews past page 1 were invisible. + let server = MockServer::start().await; + let next1 = format!("<{}/reviews?page=2&per_page=2>; rel=\"next\", <{0}/reviews?page=3&per_page=2>; rel=\"last\"", server.uri()); + Mock::given(method("GET")) + .and(path("/reviews")) + .and(query_param("per_page", "2")) + .and(wiremock::matchers::query_param_is_missing("page")) + .respond_with(ResponseTemplate::new(200) + .insert_header("Link", next1.as_str()) + .set_body_json(json!([{ "id": 1 }, { "id": 2 }]))) + .mount(&server) + .await; + let next2 = format!("<{}/reviews?page=3&per_page=2>; rel=\"next\"", server.uri()); + Mock::given(method("GET")) + .and(path("/reviews")) + .and(query_param("page", "2")) + .respond_with(ResponseTemplate::new(200) + .insert_header("Link", next2.as_str()) + .set_body_json(json!([{ "id": 3 }, { "id": 4 }]))) + .mount(&server) + .await; + // Terminal page: NO Link header at all. + Mock::given(method("GET")) + .and(path("/reviews")) + .and(query_param("page", "3")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!([{ "id": 5 }]))) + .mount(&server) + .await; + + let plugin = mk_plugin(&server.uri(), ApiAuthKind::None, vec![mk_endpoint("GET", "/reviews")]); + let mut step = mk_step("/reviews"); + step.api_pagination = Some(PaginationSpec::LinkHeader { + page_size_param: Some("per_page".into()), + page_size: Some(2), + max_pages: Some(10), + }); + step.api_extract = Some(ExtractSpec { + path: "$[*].id".into(), + fallback: None, + fail_on_empty: false, + }); + + let outcome = execute_api_call_step_core( + &step, &plugin, &HashMap::new(), &TemplateContext::new(), + SecurityPolicy::allow_loopback_for_tests(), + ).await; + assert_eq!(outcome.result.status, RunStatus::Success, + "link-header walk failed: {}", outcome.result.output); + let envelope = extract_envelope(&outcome.result.output); + assert_eq!(envelope["data"], json!([1, 2, 3, 4, 5]), + "all three bare-array pages merged in order"); + assert!(!outcome.result.output.contains("PAGINATION_TRUNCATED")); + } + + #[tokio::test] + async fn walk_pages_link_header_refuses_cross_origin_next() { + // Codex review (C2): a compromised server advertising an off-origin + // rel="next" must NOT receive a follow-up request — the walker + // re-sends the plugin's auth, so following would leak credentials. + let api = MockServer::start().await; + let attacker = MockServer::start().await; + Mock::given(method("GET")).and(path("/steal")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!([]))) + .expect(0) // the whole point: zero requests reach the attacker + .mount(&attacker).await; + let evil_next = format!("<{}/steal?page=2>; rel=\"next\"", attacker.uri()); + Mock::given(method("GET")).and(path("/items")) + .respond_with(ResponseTemplate::new(200) + .insert_header("Link", evil_next.as_str()) + .set_body_json(json!([{ "id": 1 }]))) + .expect(1) + .mount(&api).await; + + let plugin = mk_plugin(&api.uri(), ApiAuthKind::None, vec![mk_endpoint("GET", "/items")]); + let mut step = mk_step("/items"); + step.api_pagination = Some(PaginationSpec::LinkHeader { + page_size_param: None, page_size: None, max_pages: Some(10), + }); + let outcome = execute_api_call_step_core( + &step, &plugin, &HashMap::new(), &TemplateContext::new(), + SecurityPolicy::allow_loopback_for_tests(), + ).await; + assert_eq!(outcome.result.status, RunStatus::Success, "{}", outcome.result.output); + let envelope = extract_envelope(&outcome.result.output); + assert_eq!(envelope["data"], json!([{ "id": 1 }]), "page 1 kept, walk stopped at the boundary"); + // MockServer::expect(0) on the attacker asserts on drop. + } + + #[tokio::test] + async fn walk_pages_link_header_single_page_and_cap() { + let server = MockServer::start().await; + // /single: no Link header → exactly one page, no walking. + Mock::given(method("GET")) + .and(path("/single")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!([{ "id": 1 }]))) + .expect(1) + .mount(&server) + .await; + // /loop: every page advertises a next → the cap must stop the walk + // and surface PAGINATION_TRUNCATED. + let next = format!("<{}/loop?page=2>; rel=\"next\"", server.uri()); + Mock::given(method("GET")) + .and(path("/loop")) + .respond_with(ResponseTemplate::new(200) + .insert_header("Link", next.as_str()) + .set_body_json(json!([{ "id": 9 }]))) + .mount(&server) + .await; + + let plugin = mk_plugin(&server.uri(), ApiAuthKind::None, + vec![mk_endpoint("GET", "/single"), mk_endpoint("GET", "/loop")]); + + let mut step = mk_step("/single"); + step.api_pagination = Some(PaginationSpec::LinkHeader { + page_size_param: None, page_size: None, max_pages: Some(10), + }); + let outcome = execute_api_call_step_core( + &step, &plugin, &HashMap::new(), &TemplateContext::new(), + SecurityPolicy::allow_loopback_for_tests(), + ).await; + assert_eq!(outcome.result.status, RunStatus::Success, "{}", outcome.result.output); + let envelope = extract_envelope(&outcome.result.output); + assert_eq!(envelope["data"], json!([{ "id": 1 }]), "bare array preserved as-is"); + + let mut step = mk_step("/loop"); + step.api_pagination = Some(PaginationSpec::LinkHeader { + page_size_param: None, page_size: None, max_pages: Some(3), + }); + let outcome = execute_api_call_step_core( + &step, &plugin, &HashMap::new(), &TemplateContext::new(), + SecurityPolicy::allow_loopback_for_tests(), + ).await; + assert_eq!(outcome.result.status, RunStatus::Success, "{}", outcome.result.output); + assert!(outcome.result.output.contains("PAGINATION_TRUNCATED"), + "cap hit with a next still advertised must surface the truncation signal"); + } + #[tokio::test] async fn walk_pages_cursor_stops_when_next_path_resolves_null() { // Cloudflare-ish GraphQL pattern with cursor=endCursor. diff --git a/backend/src/workflows/api_call_step.rs b/backend/src/workflows/api_call_step.rs index 04f100e6..37698215 100644 --- a/backend/src/workflows/api_call_step.rs +++ b/backend/src/workflows/api_call_step.rs @@ -153,7 +153,8 @@ pub fn pagination_max_pages(spec: &PaginationSpec) -> u32 { PaginationSpec::Auto { max_pages } | PaginationSpec::Offset { max_pages, .. } | PaginationSpec::Cursor { max_pages, .. } - | PaginationSpec::Page { max_pages, .. } => *max_pages, + | PaginationSpec::Page { max_pages, .. } + | PaginationSpec::LinkHeader { max_pages, .. } => *max_pages, PaginationSpec::None => return 1, }; override_value.unwrap_or(DEFAULT_MAX_PAGES) diff --git a/backend/src/workflows/runner.rs b/backend/src/workflows/runner.rs index 0d407487..ab463b44 100644 --- a/backend/src/workflows/runner.rs +++ b/backend/src/workflows/runner.rs @@ -251,7 +251,12 @@ pub async fn execute_run( // instead of creating a new one — both because it already holds // the artifacts the operator just inspected, and because creating // a second worktree on the same branch would fail. - let is_resume = !run.step_results.is_empty(); + // `workspace_path` is only ever set by a PRIOR execute_run (fresh runs + // arrive with None), so its presence also marks a resume — covers a run + // interrupted during its FIRST step (no result persisted yet) and a gate + // RequestChanges that truncates back to step 0: both must re-attach the + // preserved worktree, not create a second one. + let is_resume = !run.step_results.is_empty() || run.workspace_path.is_some(); // Create or attach workspace (if we have a project path) let workspace = if !project_path.is_empty() { @@ -2114,6 +2119,83 @@ pub async fn resume_run( execute_run(state, workflow, run, tokens_config, agents_config, events_tx, None, None).await } +/// A2 — validate + atomically claim an `Interrupted` run for manual resume. +/// +/// Fast (no execution) so the API endpoint can await it BEFORE answering: +/// exactly one of two racing "Reprendre" clicks wins the +/// `Interrupted → Running` claim, the loser gets an error. Refusals: +/// - not `Interrupted` (Cancelled/Failed stay sticky — restart those fresh) +/// - a child (`subworkflow`) or `batch` run — resume the parent instead; +/// batch fan-out has its own machinery +/// - the preserved worktree is gone: resuming would silently fall back to +/// the MAIN checkout (execute_run's legacy resume fallback), which is +/// exactly the require_isolation hazard — refuse instead +pub async fn claim_interrupted_run(state: &AppState, run: &WorkflowRun) -> Result<()> { + use anyhow::anyhow; + if run.status != RunStatus::Interrupted { + return Err(anyhow!( + "Run {} is {:?} — only Interrupted runs can be resumed", + run.id, run.status + )); + } + if run.run_type == "subworkflow" || run.parent_run_id.is_some() { + return Err(anyhow!( + "Run {} is a sub-workflow child — resume its parent run instead (the foreach re-runs only the missing items)", + run.id + )); + } + if run.run_type == "batch" { + return Err(anyhow!( + "Run {} is a batch fan-out — batch runs cannot be resumed, re-trigger the workflow", + run.id + )); + } + if let Some(ws) = run.workspace_path.as_deref() { + if !std::path::Path::new(ws).exists() { + return Err(anyhow!( + "Worktree `{}` no longer exists — refusing to resume in the main checkout. Re-trigger the workflow for a fresh run.", + ws + )); + } + } + let run_id = run.id.clone(); + let claimed = state + .db + .with_conn(move |conn| { + crate::db::workflows::claim_run_status( + conn, &run_id, &RunStatus::Interrupted, &RunStatus::Running, + ) + }) + .await?; + if !claimed { + return Err(anyhow!( + "Run {} was just claimed by another caller — resume ignored (no double-resume)", + run.id + )); + } + Ok(()) +} + +/// A2 — continue an `Interrupted` run the caller just claimed via +/// [`claim_interrupted_run`]. Everything resume-specific already lives in +/// [`execute_run`]: `is_resume` detection re-attaches the preserved worktree, +/// the cursor restarts at the step AFTER the last completed result (the +/// interrupted step re-runs from its start — its foreach items dedupe via the +/// done-set/git-ledger reconciliation), and prior step outputs + `run.state` +/// are replayed into the template ctx. +pub async fn resume_interrupted_run( + state: AppState, + workflow: &Workflow, + run: &mut WorkflowRun, + tokens_config: &TokensConfig, + agents_config: &AgentsConfig, + events_tx: Option, +) -> Result<()> { + run.status = RunStatus::Running; + run.finished_at = None; + execute_run(state, workflow, run, tokens_config, agents_config, events_tx, None, None).await +} + /// Append a `> Décision: ` (and optional comment) footer to a /// gate StepResult's output. Keeps the rendered gate message intact so /// the operator's decision is visible alongside the original prompt @@ -3242,6 +3324,187 @@ mod tests { .expect("run row exists"); assert_eq!(persisted.status, RunStatus::Failed, "failure persisted to DB"); } + + // ─── A2 — manual resume of Interrupted runs (acceptance tests) ────── + + async fn insert_wf_and_run( + state: &crate::AppState, + wf: &Workflow, + run: &WorkflowRun, + ) { + let wf_db = wf.clone(); + let run_db = run.clone(); + state.db.with_conn(move |conn| { + crate::db::workflows::insert_workflow(conn, &wf_db)?; + crate::db::workflows::insert_run(conn, &run_db)?; + Ok(()) + }).await.unwrap(); + } + + #[tokio::test] + async fn double_resume_wins_exactly_one_claim() { + let (state, _tokens, _agents) = test_state_and_configs(); + let mut wf = make_workflow_with_artifacts(Default::default()); + wf.id = "wf-claim".into(); + let mut run = pending_run("run-claim", "wf-claim"); + run.status = RunStatus::Interrupted; + insert_wf_and_run(&state, &wf, &run).await; + + // Both callers hold the same (stale) Interrupted snapshot — the + // double-click case. Validation passes for both; the atomic claim + // decides. + let first = claim_interrupted_run(&state, &run).await; + let second = claim_interrupted_run(&state, &run).await; + assert!(first.is_ok(), "first caller wins the claim: {first:?}"); + let err = second.expect_err("second caller must lose").to_string(); + assert!(err.contains("another caller"), "loser gets the no-double-resume error, got: {err}"); + } + + #[tokio::test] + async fn cancelled_and_failed_stay_sticky_on_resume() { + let (state, _tokens, _agents) = test_state_and_configs(); + let mut wf = make_workflow_with_artifacts(Default::default()); + wf.id = "wf-sticky".into(); + for (id, status) in [("run-cancelled", RunStatus::Cancelled), ("run-failed", RunStatus::Failed)] { + let mut run = pending_run(id, "wf-sticky"); + run.status = status; + let run_db = run.clone(); + let wf_db = wf.clone(); + state.db.with_conn(move |conn| { + let _ = crate::db::workflows::insert_workflow(conn, &wf_db); + crate::db::workflows::insert_run(conn, &run_db) + }).await.unwrap(); + + let err = claim_interrupted_run(&state, &run).await + .expect_err("non-Interrupted runs are not resumable").to_string(); + assert!(err.contains("only Interrupted"), "{err}"); + + // Belt-and-braces at the DB layer too: even a caller that skips + // validation can't flip a terminal row. + let rid = id.to_string(); + let claimed = state.db.with_conn(move |conn| { + crate::db::workflows::claim_run_status(conn, &rid, &RunStatus::Interrupted, &RunStatus::Running) + }).await.unwrap(); + assert!(!claimed, "{id} must not be claimable Interrupted→Running"); + } + } + + #[tokio::test] + async fn resume_refuses_children_batches_and_gone_worktrees() { + let (state, _tokens, _agents) = test_state_and_configs(); + + let mut child = pending_run("run-child", "wf-x"); + child.status = RunStatus::Interrupted; + child.run_type = "subworkflow".into(); + child.parent_run_id = Some("run-parent".into()); + let err = claim_interrupted_run(&state, &child).await.unwrap_err().to_string(); + assert!(err.contains("parent"), "children resume through the parent: {err}"); + + let mut batch = pending_run("run-batch", "wf-x"); + batch.status = RunStatus::Interrupted; + batch.run_type = "batch".into(); + let err = claim_interrupted_run(&state, &batch).await.unwrap_err().to_string(); + assert!(err.contains("batch"), "{err}"); + + let mut gone = pending_run("run-gone-ws", "wf-x"); + gone.status = RunStatus::Interrupted; + gone.workspace_path = Some("/nonexistent/kronn-worktree-a2".into()); + let err = claim_interrupted_run(&state, &gone).await.unwrap_err().to_string(); + assert!(err.contains("main checkout"), "a gone worktree must refuse, not fall back: {err}"); + } + + #[tokio::test] + async fn resume_during_first_step_reattaches_the_preserved_worktree() { + // Codex A2 finding: a run interrupted DURING its first step has + // step_results=[] — is_resume keyed on results alone took the + // Workspace::create path and abandoned the preserved worktree. + let (state, tokens, agents) = test_state_and_configs(); + + let repo = tempfile::TempDir::new().unwrap(); + let git = |args: &[&str]| { + let out = std::process::Command::new("git") + .args(["-c", "user.email=t@t", "-c", "user.name=t"]) + .args(args).current_dir(repo.path()).output().unwrap(); + assert!(out.status.success(), "git {args:?}: {}", String::from_utf8_lossy(&out.stderr)); + }; + git(&["init", "-q"]); + std::fs::write(repo.path().join("README.md"), "x").unwrap(); + git(&["add", "-A"]); + git(&["commit", "-q", "-m", "init"]); + + let project: crate::models::Project = serde_json::from_value(serde_json::json!({ + "id": "proj-resume", "name": "p", + "path": repo.path().to_string_lossy(), + "repo_url": null, "token_override": null, "ai_config": {"detected": false, "configs": []}, + "created_at": chrono::Utc::now().to_rfc3339(), + "updated_at": chrono::Utc::now().to_rfc3339(), + })).unwrap(); + state.db.with_conn(move |c| crate::db::projects::insert_project(c, &project)) + .await.unwrap(); + + let mut wf = make_workflow_with_artifacts(Default::default()); + wf.id = "wf-first-step".into(); + wf.project_id = Some("proj-resume".into()); + wf.steps = vec![json_data_step("only", serde_json::json!({ "n": 1 }))]; + + // The preserved worktree: exists on disk, holds pre-crash work. + let preserved = tempfile::TempDir::new().unwrap(); + let preserved_path = preserved.path().to_string_lossy().to_string(); + + let mut run = pending_run("run-first-step", "wf-first-step"); + run.status = RunStatus::Interrupted; + run.workspace_path = Some(preserved_path.clone()); + insert_wf_and_run(&state, &wf, &run).await; + + claim_interrupted_run(&state, &run).await.expect("claim"); + resume_interrupted_run(state.clone(), &wf, &mut run, &tokens, &agents, None) + .await + .expect("resume must not error"); + + assert_eq!(run.status, RunStatus::Success); + assert_eq!(run.step_results.len(), 1, "the first step re-ran"); + assert_eq!( + run.workspace_path.as_deref(), Some(preserved_path.as_str()), + "resume must ATTACH the preserved worktree, not create a second one" + ); + } + + #[tokio::test] + async fn resume_continues_from_the_step_after_the_last_completed_one() { + let (state, tokens, agents) = test_state_and_configs(); + + let mut wf = make_workflow_with_artifacts(Default::default()); + wf.id = "wf-resume".into(); + wf.steps = vec![ + json_data_step("first", serde_json::json!({ "phase": "one" })), + json_data_step("second", serde_json::json!({ "phase": "two" })), + ]; + + // The crash snapshot: step `first` completed + persisted, the process + // died during `second`, the boot reconcile flipped the row. + let mut run = pending_run("run-resume", "wf-resume"); + run.status = RunStatus::Interrupted; + run.step_results.push(fake_result("first")); + run.state.insert("survivor".into(), "kept".into()); + insert_wf_and_run(&state, &wf, &run).await; + + claim_interrupted_run(&state, &run).await.expect("claim"); + resume_interrupted_run(state.clone(), &wf, &mut run, &tokens, &agents, None) + .await + .expect("resume must not error"); + + assert_eq!(run.status, RunStatus::Success); + assert_eq!(run.step_results.len(), 2, "only `second` re-ran — `first` was NOT re-executed"); + assert_eq!(run.step_results[0].step_name, "first"); + assert_eq!(run.step_results[1].step_name, "second"); + assert_eq!(run.state.get("survivor").map(String::as_str), Some("kept"), "durable state survives the resume"); + + let persisted = state.db + .with_conn(|conn| crate::db::workflows::get_run(conn, "run-resume")) + .await.unwrap().expect("row"); + assert_eq!(persisted.status, RunStatus::Success); + assert!(persisted.finished_at.is_some(), "terminal write re-stamps finished_at"); + } } #[cfg(test)] diff --git a/backend/src/workflows/steps.rs b/backend/src/workflows/steps.rs index 3abf63cc..82fb0014 100644 --- a/backend/src/workflows/steps.rs +++ b/backend/src/workflows/steps.rs @@ -237,7 +237,11 @@ pub async fn execute_step( } else { "missing envelope".into() }; - tracing::info!("Step '{}': output {}, attempting repair", step.name, reason); + tracing::warn!( + target: "kronn::invariant", + step = %step.name, reason = %reason, + "step output broke the envelope contract — repair attempt" + ); // Truncate by char count — `&s[..2000]` panics if // byte 2000 falls inside a UTF-8 sequence (emoji, // accented chars in the LLM output). diff --git a/backend/src/workflows/sub_workflow_step.rs b/backend/src/workflows/sub_workflow_step.rs index f9bdf966..f0f5579f 100644 --- a/backend/src/workflows/sub_workflow_step.rs +++ b/backend/src/workflows/sub_workflow_step.rs @@ -335,6 +335,23 @@ pub(crate) fn current_task_template_vars(item: &serde_json::Value) -> Vec<(Strin /// child's prompts read ONLY their slice, then re-enters the runner. A failed /// item doesn't stop the loop (PARTIAL surfaces it); parent cancel is honoured /// between items. +/// A2 — durable per-item done marker on the PARENT run, written only AFTER a +/// durable effect (commit landed / child Success). Best-effort: child rows and +/// the git ledger remain the reconciliation sources at resume. +async fn record_foreach_done( + state: &crate::AppState, + parent_run_id: &str, + step_name: &str, + entry: serde_json::Value, +) { + let (rid, sname) = (parent_run_id.to_string(), step_name.to_string()); + if let Err(e) = state.db.with_conn(move |conn| { + crate::db::workflows::append_foreach_done(conn, &rid, &sname, entry) + }).await { + tracing::warn!(target: "kronn::sub_workflow", "foreach done-set write failed: {e}"); + } +} + #[allow(clippy::too_many_arguments)] async fn execute_foreach( state: &crate::AppState, @@ -373,6 +390,39 @@ async fn execute_foreach( Err(e) => return fail(step, start, format!("DB error loading sub-workflow `{target}`: {e}")), }; + // A2 resume reconciliation — three sources, trusted in this order: + // 1. git ledger (`[item_id]` commit subjects, checked per-item below) + // 2. Success child rows (a child that finished right before a crash, + // before its done-set entry landed) + // 3. the done-set in run.state — NEVER trusted alone: an entry that + // neither the ledger nor a child row confirms is stale and the + // item re-runs (warn below). + let child_done: std::collections::HashSet = { + let pid = parent_run_id.to_string(); + state.db.with_conn(move |c| crate::db::workflows::successful_child_item_ids(c, &pid)) + .await + .unwrap_or_else(|e| { + tracing::warn!(target: "kronn::sub_workflow", parent_run=%parent_run_id, error=%e, + "cannot load Success child rows — reconciliation falls back to the git ledger only"); + Default::default() + }) + }; + let state_done: std::collections::HashSet = { + let pid = parent_run_id.to_string(); + let key = format!("__kronn.foreach_done.{}", step.name); + state.db.with_conn(move |c| crate::db::workflows::get_run(c, &pid)).await + .ok().flatten() + .and_then(|r| r.state.get(&key).cloned()) + .and_then(|doc| serde_json::from_str::(&doc).ok()) + .and_then(|d| d.get("items").and_then(|i| i.as_array()).map(|items| { + items.iter() + .filter_map(|e| e.get("id").and_then(|i| i.as_str()).map(String::from)) + .filter(|i| !i.is_empty()) + .collect() + })) + .unwrap_or_default() + }; + let task_file = std::path::Path::new(&ws).join(".kronn/current_task.json"); let mut results: Vec = Vec::with_capacity(items.len()); let mut succeeded = 0usize; @@ -400,18 +450,26 @@ async fn execute_foreach( // everything. Deterministic, 0 token. Cross-RUN resume is out of // scope (a new run gets a fresh worktree/branch). if !item_id.is_empty() { - let done = crate::core::cmd::async_cmd("git") + let git_done = crate::core::cmd::async_cmd("git") .args(["log", "--oneline", "--fixed-strings", "--grep", &format!("[{item_id}]")]) .current_dir(&ws) .output().await .map(|o| o.status.success() && !o.stdout.is_empty()) .unwrap_or(false); - if done { - tracing::info!(target: "kronn::sub_workflow", parent_run=%parent_run_id, item=%idx, item_id=%item_id, "foreach: item already committed — skipping"); + if git_done || child_done.contains(&item_id) { + tracing::info!(target: "kronn::sub_workflow", parent_run=%parent_run_id, item=%idx, item_id=%item_id, + source = if git_done { "git-ledger" } else { "child-row" }, + "foreach: item already done — skipping"); succeeded += 1; results.push(json!({ "item": idx, "id": item_id, "child_run_id": null, "status": "SkippedAlreadyDone" })); + record_foreach_done(state, parent_run_id, &step.name, + json!({"idx": idx, "id": item_id, "status": "SkippedAlreadyDone", "child_run_id": null})).await; continue; } + if state_done.contains(&item_id) { + tracing::warn!(target: "kronn::sub_workflow", parent_run=%parent_run_id, item=%idx, item_id=%item_id, + "foreach: done-set entry not confirmed by git ledger or a Success child row — stale marker ignored, re-running the item"); + } } // Flip économique (2026-06-12) — `mechanical:true` items whose content @@ -449,6 +507,8 @@ async fn execute_foreach( tracing::info!(target: "kronn::sub_workflow", item_id=%item_id, files=files.len(), "mechanical item engine-applied (0 tokens)"); succeeded += 1; results.push(json!({ "item": idx, "id": item_id, "child_run_id": null, "status": "MechanicalApplied", "files": files.len() })); + record_foreach_done(state, parent_run_id, &step.name, + json!({"idx": idx, "id": item_id, "status": "MechanicalApplied", "child_run_id": null})).await; continue; } tracing::warn!(target: "kronn::sub_workflow", item_id=%item_id, "mechanical commit failed — falling back to agent"); @@ -554,6 +614,12 @@ async fn execute_foreach( "child_run_id": child_run.id, "status": format!("{:?}", child_run.status), })); + // Done ONLY on child Success — a Failed child must be re-attempted + // by a resume, never skipped on the strength of a stale marker. + if ok { + record_foreach_done(state, parent_run_id, &step.name, + json!({"idx": idx, "id": item_id, "status": "Success", "child_run_id": child_run.id})).await; + } } let _ = std::fs::remove_file(&task_file); // best-effort cleanup @@ -654,6 +720,200 @@ mod tests { use super::child_depth_exceeds; use super::{aggregate_foreach, parse_foreach_items, MAX_SUBWORKFLOW_DEPTH}; + // ── A2 — foreach resume reconciliation (3 sources) ────────────────── + + fn step_json(v: serde_json::Value) -> crate::models::WorkflowStep { + serde_json::from_value(v).expect("minimal step JSON") + } + + /// Parent Running run + trivial JsonData child workflow + a git-init'd + /// worktree holding `tasks.json` with items T1/T2. Returns everything a + /// reconciliation test needs to call `execute_sub_workflow_step`. + async fn foreach_fixture() -> ( + crate::AppState, + crate::models::TokensConfig, + crate::models::AgentsConfig, + tempfile::TempDir, + crate::models::WorkflowStep, + ) { + let db = std::sync::Arc::new(crate::db::Database::open_in_memory().expect("in-memory DB")); + let cfg = crate::core::config::default_config(); + let tokens = cfg.tokens.clone(); + let agents = cfg.agents.clone(); + let config = std::sync::Arc::new(tokio::sync::RwLock::new(cfg)); + let state = crate::AppState::new_defaults(config, db, crate::DEFAULT_MAX_CONCURRENT_AGENTS); + + // Child workflow: one deterministic JsonData step — no LLM, no project. + let child_wf = crate::models::Workflow { + id: "child-wf".into(), + name: "child".into(), + project_id: None, + trigger: crate::models::WorkflowTrigger::Manual, + steps: vec![step_json(serde_json::json!({ + "name": "emit", "step_type": {"type": "JsonData"}, + "json_data_payload": { "done": true }, + }))], + actions: vec![], + safety: crate::models::WorkflowSafety { sandbox: false, max_files: None, max_lines: None, require_approval: false }, + workspace_config: None, + concurrency_limit: None, + guards: None, + artifacts: Default::default(), + on_failure: vec![], + exec_allowlist: vec![], + variables: vec![], + enabled: true, + created_at: chrono::Utc::now(), + updated_at: chrono::Utc::now(), + }; + let parent_run = crate::models::WorkflowRun { + id: "parent-run".into(), + workflow_id: "parent-wf".into(), + status: crate::models::RunStatus::Running, + trigger_context: None, + step_results: vec![], + tokens_used: 0, + workspace_path: None, + started_at: chrono::Utc::now(), + finished_at: None, + run_type: "linear".into(), + batch_total: 0, + batch_completed: 0, + batch_failed: 0, + batch_name: None, + parent_run_id: None, + state: Default::default(), + produced_branches: vec![], + parent_workflow_id: None, + parent_workflow_name: None, + parent_run_started_at: None, + }; + // FK: the parent run's workflow row must exist too. + let mut parent_wf = child_wf.clone(); + parent_wf.id = "parent-wf".into(); + state.db.with_conn(move |c| { + crate::db::workflows::insert_workflow(c, &child_wf)?; + crate::db::workflows::insert_workflow(c, &parent_wf)?; + crate::db::workflows::insert_run(c, &parent_run) + }).await.unwrap(); + + // Worktree stand-in: a real git repo with the items file. + let ws = tempfile::TempDir::new().unwrap(); + let git = |args: &[&str]| { + let out = std::process::Command::new("git") + .args(["-c", "user.email=t@t", "-c", "user.name=t"]) + .args(args).current_dir(ws.path()).output().unwrap(); + assert!(out.status.success(), "git {args:?}: {}", String::from_utf8_lossy(&out.stderr)); + }; + git(&["init", "-q"]); + std::fs::create_dir_all(ws.path().join(".kronn")).unwrap(); + std::fs::write( + ws.path().join("tasks.json"), + r#"[{"id":"T1","what":"a"},{"id":"T2","what":"b"}]"#, + ).unwrap(); + git(&["add", "-A"]); + git(&["commit", "-q", "-m", "init"]); + + let step = step_json(serde_json::json!({ + "name": "fanout", "step_type": {"type": "SubWorkflow"}, + "sub_workflow_id": "child-wf", + "sub_workflow_foreach_file": "tasks.json", + })); + (state, tokens, agents, ws, step) + } + + async fn run_foreach( + state: &crate::AppState, + tokens: &crate::models::TokensConfig, + agents: &crate::models::AgentsConfig, + ws: &tempfile::TempDir, + step: &crate::models::WorkflowStep, + ) -> serde_json::Value { + let outcome = super::execute_sub_workflow_step( + state, "parent-run", 0, step, tokens, agents, + crate::workflows::runner::SharedBudget::root(50), + Some(ws.path().to_string_lossy().to_string()), + ).await; + assert_eq!(outcome.result.status, crate::models::RunStatus::Success, + "foreach must succeed: {}", outcome.result.output); + crate::workflows::step_output_format::parse_envelope_for_test(&outcome.result.output)["data"].clone() + } + + async fn child_rows_for(state: &crate::AppState, item_id: &str) -> usize { + let iid = item_id.to_string(); + state.db.with_conn(move |c| { + let mut stmt = c.prepare( + "SELECT trigger_context FROM workflow_runs WHERE parent_run_id = 'parent-run'", + )?; + let rows = stmt.query_map([], |r| r.get::<_, Option>(0))?; + let mut n = 0; + for row in rows { + if row?.as_deref().map(|t| t.contains(&format!("\"{iid}\""))).unwrap_or(false) { + n += 1; + } + } + Ok(n) + }).await.unwrap() + } + + #[tokio::test] + async fn foreach_skips_items_already_in_the_git_ledger() { + let (state, tokens, agents, ws, step) = foreach_fixture().await; + // The crash happened after T1's commit landed. + let out = std::process::Command::new("git") + .args(["-c", "user.email=t@t", "-c", "user.name=t", + "commit", "-q", "--allow-empty", "-m", "Kronn AutoPilot [T1] done"]) + .current_dir(ws.path()).output().unwrap(); + assert!(out.status.success()); + + let data = run_foreach(&state, &tokens, &agents, &ws, &step).await; + let items = data["items"].as_array().unwrap(); + assert_eq!(items[0]["status"], "SkippedAlreadyDone", "T1 confirmed by the ledger"); + assert_eq!(items[1]["status"], "Success", "T2 ran its child"); + assert_eq!(child_rows_for(&state, "T1").await, 0, "no duplicate child for T1"); + assert_eq!(child_rows_for(&state, "T2").await, 1); + } + + #[tokio::test] + async fn foreach_rebuilds_from_a_success_child_row_without_duplicating() { + let (state, tokens, agents, ws, step) = foreach_fixture().await; + // Crash window: T1's child finished (row Success) but the parent died + // BEFORE writing the done-set entry — and its child made no commit. + state.db.with_conn(|c| { + c.execute( + "INSERT INTO workflow_runs (id, workflow_id, status, run_type, parent_run_id, trigger_context, started_at) + VALUES ('pre-child', 'child-wf', 'Success', 'subworkflow', 'parent-run', + '{\"__subwf_item_id__\":\"T1\"}', datetime('now'))", + [], + ).map_err(Into::into) + }).await.unwrap(); + + let data = run_foreach(&state, &tokens, &agents, &ws, &step).await; + let items = data["items"].as_array().unwrap(); + assert_eq!(items[0]["status"], "SkippedAlreadyDone", "T1 rebuilt from the child row"); + assert_eq!(items[1]["status"], "Success"); + assert_eq!(child_rows_for(&state, "T1").await, 1, "only the pre-crash child — no duplicate"); + } + + #[tokio::test] + async fn foreach_ignores_a_stale_done_set_entry_and_reruns_the_item() { + let (state, tokens, agents, ws, step) = foreach_fixture().await; + // A done-set entry NOBODY confirms (no commit, no child row) — e.g. a + // hand-edited or corrupted state. Trusting it would silently drop T1. + state.db.with_conn(|c| { + crate::db::workflows::set_run_state_key( + c, "parent-run", "__kronn.foreach_done.fanout", + r#"{"v":1,"items":[{"idx":0,"id":"T1","status":"Success","child_run_id":"ghost"}]}"#, + &[crate::models::RunStatus::Running], + ).map(|_| ()) + }).await.unwrap(); + + let data = run_foreach(&state, &tokens, &agents, &ws, &step).await; + let items = data["items"].as_array().unwrap(); + assert_eq!(items[0]["status"], "Success", "stale marker ignored — T1 re-ran"); + assert_eq!(child_rows_for(&state, "T1").await, 1, "T1 got a real child this time"); + } + #[test] fn depth_guard_allows_up_to_cap_and_refuses_beyond() { // From depth 0 we can descend until the child reaches the cap. diff --git a/backend/src/workflows/template.rs b/backend/src/workflows/template.rs index bce3819b..5d29307f 100644 --- a/backend/src/workflows/template.rs +++ b/backend/src/workflows/template.rs @@ -809,7 +809,22 @@ pub fn extract_step_envelope(text: &str) -> Option { // from the agent's own reasoning. if let Some(start) = text.find("---STEP_OUTPUT---") { let after_delim = &text[start + "---STEP_OUTPUT---".len()..]; - if let Some(end) = after_delim.find("---END_STEP_OUTPUT---") { + // The payload may itself CONTAIN the marker text inside a JSON string + // (a foreach envelope carries the child's full output — markers + // included — in `last_output`), so the FIRST end-marker match can cut + // the JSON mid-string. Try candidates outermost-first: an embedded + // marker never yields valid JSON, the real closer does. + // The real closer is the LAST marker (format_step_output ends the + // block there) or just before trailing prose that quotes one — so + // bound the parse attempts by keeping the last 8 candidates, not the + // first 8 (with ≥8 embedded markers the first-8 window missed the + // real closer entirely). + let all_ends: Vec = after_delim + .match_indices("---END_STEP_OUTPUT---") + .map(|(i, _)| i) + .collect(); + let start_at = all_ends.len().saturating_sub(8); + for end in all_ends[start_at..].iter().rev().copied() { let json_str = after_delim[..end].trim(); if let Ok(parsed) = serde_json::from_str::(json_str) { return envelope_from_json(&parsed); @@ -820,9 +835,19 @@ pub fn extract_step_envelope(text: &str) -> Option { .trim_end_matches("```") .trim(); if let Ok(parsed) = serde_json::from_str::(stripped) { + tracing::info!( + target: "kronn::invariant", + "envelope salvaged by stripping markdown fences — agent wrapped the block in ```" + ); return envelope_from_json(&parsed); } } + // A′ observability — the agent TRIED to emit an envelope (markers + // present) and botched it; distinct from marker-less free text. + tracing::warn!( + target: "kronn::invariant", + "STEP_OUTPUT markers present but the block is unparsable" + ); return None; } @@ -1426,6 +1451,110 @@ mod tests { } } + #[test] + fn envelope_with_embedded_child_markers_parses_via_outermost_end() { + // REAL-BUG (passe B) — a foreach envelope carries the child's FULL + // output (markers included) in `data.last_output`; the first + // end-marker match used to cut the outer JSON mid-string and reject a + // perfectly valid envelope into the repair loop. + let child_output = "---STEP_OUTPUT---\n{\"data\":{\"done\":true},\"status\":\"OK\",\"summary\":\"child\"}\n---END_STEP_OUTPUT---\nOK"; + let outer = serde_json::json!({ + "data": { "succeeded": 2, "failed": 0, "last_output": child_output }, + "status": "OK", + "summary": "Sous-workflow x 2", + }); + let text = format!("---STEP_OUTPUT---\n{}\n---END_STEP_OUTPUT---\nOK", outer); + let env = extract_step_envelope(&text).expect("outer envelope must parse"); + assert_eq!(env.status, "OK"); + let data: serde_json::Value = serde_json::from_str(&env.data_json).unwrap(); + assert_eq!(data["succeeded"], 2); + assert!(data["last_output"].as_str().unwrap().contains("---END_STEP_OUTPUT---"), + "the child's markers survive intact inside the string"); + } + + #[test] + fn envelope_with_many_embedded_markers_still_finds_the_real_closer() { + // Codex review (lot B): with >=8 embedded end-markers, a first-8 + // candidate window never reached the real (last) closer and a valid + // envelope fell into the repair loop. + let embedded: Vec = (0..9) + .map(|i| format!("---STEP_OUTPUT---\n{{\"n\":{i}}}\n---END_STEP_OUTPUT---")) + .collect(); + let outer = serde_json::json!({ + "data": { "children": embedded }, + "status": "OK", + "summary": "fan-out" + }); + let text = format!("---STEP_OUTPUT---\n{}\n---END_STEP_OUTPUT---\nOK", outer); + let env = extract_step_envelope(&text).expect("nine embedded markers must not defeat the extractor"); + assert_eq!(env.status, "OK"); + } + + #[test] + fn envelope_corpus_of_degenerate_agent_outputs() { + // Table of real failure classes seen in -lab runs / hardening passes. + // (name, input, expected status: Some(status) or None → repair path) + let valid = "{\"data\":{\"n\":1},\"status\":\"OK\",\"summary\":\"s\"}"; + let cases: Vec<(&str, String, Option<&str>)> = vec![ + ("canonical", format!("---STEP_OUTPUT---\n{valid}\n---END_STEP_OUTPUT---\nOK"), Some("OK")), + ("crlf line endings", format!("---STEP_OUTPUT---\r\n{valid}\r\n---END_STEP_OUTPUT---\r\nOK"), Some("OK")), + ("fenced json inside markers", format!("---STEP_OUTPUT---\n```json\n{valid}\n```\n---END_STEP_OUTPUT---"), Some("OK")), + ("prose before and after", format!("Voilà le résultat :\n---STEP_OUTPUT---\n{valid}\n---END_STEP_OUTPUT---\nEt ensuite je vais…"), Some("OK")), + ("truncated mid-string", "---STEP_OUTPUT---\n{\"data\":{\"n\":1},\"status\":\"OK\",\"su".to_string(), None), + ("open marker only, no close", format!("---STEP_OUTPUT---\n{valid}"), None), + ("close marker only", format!("{valid}\n---END_STEP_OUTPUT---"), Some("OK")), // no open marker → strategy 2 on the bare JSON + ("empty block", "---STEP_OUTPUT---\n\n---END_STEP_OUTPUT---".to_string(), None), + ("emoji + accents in strings", "---STEP_OUTPUT---\n{\"data\":{\"msg\":\"réussi ✅ à 100%\"},\"status\":\"OK\",\"summary\":\"été\"}\n---END_STEP_OUTPUT---".to_string(), Some("OK")), + ("brace inside string (strategy 2)", "{\"data\":\"brace } inside\",\"status\":\"OK\",\"summary\":\"s\"}".to_string(), Some("OK")), + ("malformed block does NOT fall through to a quoted example", + "---STEP_OUTPUT---\n{broken\n---END_STEP_OUTPUT---\nexample: {\"data\":{},\"status\":\"OK\",\"summary\":\"quoted\"}".to_string(), None), + // 0.8.11 semantics: a present-but-non-string status must not read + // as success — it maps to ERROR (failure direction preserved). + ("null status maps to ERROR", "---STEP_OUTPUT---\n{\"data\":{},\"status\":null,\"summary\":\"s\"}\n---END_STEP_OUTPUT---".to_string(), Some("ERROR")), + ]; + for (name, input, expected) in cases { + let got = extract_step_envelope(&input); + match expected { + Some(status) => { + let env = got.unwrap_or_else(|| panic!("case `{name}` must parse")); + assert_eq!(env.status, status, "case `{name}`"); + } + None => assert!(got.is_none(), "case `{name}` must be rejected (repair path)"), + } + } + } + + #[test] + fn envelope_extraction_never_panics_on_mutated_input() { + // Fuzz-lite, deterministic (no rand dep, no Date): every prefix + // truncation + single-byte corruptions at a stride, over both + // strategy shapes. The property is total absence of panics — + // Option::None is always acceptable. + let bases = [ + format!("---STEP_OUTPUT---\n{}\n---END_STEP_OUTPUT---\nPARTIAL", + "{\"data\":{\"items\":[{\"id\":\"T1\"}],\"note\":\"brace } et « accents » ✅\"},\"status\":\"PARTIAL\",\"summary\":\"s\"}"), + "prose {\"data\":{\"a\":1},\"status\":\"OK\",\"summary\":\"s\"} tail".to_string(), + ]; + for base in &bases { + for cut in 0..base.len() { + if !base.is_char_boundary(cut) { continue; } + let _ = extract_step_envelope(&base[..cut]); + } + let bytes = base.as_bytes(); + for pos in (0..bytes.len()).step_by(7) { + for repl in [b'{', b'}', b'"', b'\\', 0xC3u8] { + let mut mutated = bytes.to_vec(); + mutated[pos] = repl; + // Invalid UTF-8 mutants are legitimately unrepresentable + // as &str — the parser only ever sees valid UTF-8. + if let Ok(s) = std::str::from_utf8(&mutated) { + let _ = extract_step_envelope(s); + } + } + } + } + } + #[test] fn strategy2_only_accepts_the_last_toplevel_json_as_envelope() { // Codex acceptance matrix for the bounded strategy-2 fallback. diff --git a/backend/tests/api_tests.rs b/backend/tests/api_tests.rs index fc5a96e8..6110ae66 100644 --- a/backend/tests/api_tests.rs +++ b/backend/tests/api_tests.rs @@ -24,7 +24,21 @@ use futures::{SinkExt, StreamExt}; // ═══════════════════════════════════════════════════════════════════════════════ /// Create a test AppState with an in-memory database and default config. +/// Every config::save reached through a handler under test would otherwise +/// write the DEVELOPER'S REAL config.toml (config_dir() falls back to the +/// platform dir when KRONN_DATA_DIR is unset) — a full `cargo test` used to +/// wipe pseudo/avatar/model-tiers on the host (2026-07-13 incident). +fn isolate_config_dir() { + static INIT: std::sync::Once = std::sync::Once::new(); + INIT.call_once(|| { + let dir = std::env::temp_dir().join(format!("kronn-inttest-cfg-{}", std::process::id())); + std::fs::create_dir_all(&dir).ok(); + std::env::set_var("KRONN_DATA_DIR", &dir); + }); +} + fn test_state() -> AppState { + isolate_config_dir(); let db = Arc::new( kronn::db::Database::open_in_memory().expect("Failed to open in-memory DB"), ); @@ -47,6 +61,10 @@ async fn get_json(app: Router, uri: &str) -> (StatusCode, Value) { .body(Body::empty()) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let status = resp.status(); let body = resp.into_body().collect().await.unwrap().to_bytes(); @@ -63,6 +81,10 @@ async fn post_json(app: Router, uri: &str, body: Value) -> (StatusCode, Value) { .body(Body::from(serde_json::to_vec(&body).unwrap())) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let status = resp.status(); let body = resp.into_body().collect().await.unwrap().to_bytes(); @@ -78,6 +100,10 @@ async fn delete_json(app: Router, uri: &str) -> (StatusCode, Value) { .body(Body::empty()) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let status = resp.status(); let body = resp.into_body().collect().await.unwrap().to_bytes(); @@ -94,6 +120,10 @@ async fn patch_json(app: Router, uri: &str, body: Value) -> (StatusCode, Value) .body(Body::from(serde_json::to_vec(&body).unwrap())) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let status = resp.status(); let body = resp.into_body().collect().await.unwrap().to_bytes(); @@ -211,6 +241,10 @@ async fn context_files_upload_text_file() { .body(Body::from(body)) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let status = resp.status(); let body = resp.into_body().collect().await.unwrap().to_bytes(); @@ -254,6 +288,10 @@ async fn context_files_upload_image_lands_in_persistent_dir_not_temp() { .header("content-type", format!("multipart/form-data; boundary={}", boundary)) .body(Body::from(body)) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let json: Value = serde_json::from_slice(&resp.into_body().collect().await.unwrap().to_bytes()).unwrap(); @@ -288,6 +326,10 @@ async fn context_files_upload_arbitrary_file_lands_on_disk() { .body(Body::from(body)) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let body = resp.into_body().collect().await.unwrap().to_bytes(); let json: Value = serde_json::from_slice(&body).unwrap(); @@ -335,6 +377,10 @@ async fn context_files_delete_nonexistent_returns_error() { /// GET the raw bytes of an attachment; returns (status, content-type, body). async fn get_raw(app: Router, uri: &str) -> (StatusCode, Option, Vec) { let req = Request::builder().method("GET").uri(uri).body(Body::empty()).unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let status = resp.status(); let ct = resp.headers().get("content-type").and_then(|v| v.to_str().ok()).map(String::from); @@ -471,6 +517,10 @@ async fn context_file_content_sanitizes_filename_in_content_disposition() { let req = Request::builder().method("GET") .uri(format!("/api/discussions/{}/context-files/cf-evil/content", disc_id)) .body(Body::empty()).unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); assert_eq!(resp.status(), StatusCode::OK); let cd = resp.headers().get("content-disposition").unwrap().to_str().unwrap(); @@ -1325,6 +1375,10 @@ async fn discussions_send_message_blocks_while_partial_pending() { "target_agent": null, })).unwrap())) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); assert_eq!(resp.status(), StatusCode::OK); let body = resp.into_body().collect().await.unwrap().to_bytes(); @@ -1693,6 +1747,10 @@ async fn config_export_empty_db() { .body(Body::empty()) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( @@ -1710,7 +1768,7 @@ async fn config_export_empty_db() { let mut contents = String::new(); std::io::Read::read_to_string(&mut data_file, &mut contents).unwrap(); let data: Value = serde_json::from_str(&contents).unwrap(); - assert_eq!(data["version"], 4); + assert_eq!(data["version"], kronn::models::db::CURRENT_EXPORT_VERSION); assert!(data["projects"].as_array().unwrap().is_empty()); assert!(data["discussions"].as_array().unwrap().is_empty()); assert!(data["workflows"].as_array().unwrap().is_empty()); @@ -2346,6 +2404,10 @@ async fn bootstrap_invalid_payload_returns_error() { .body(Body::from(serde_json::to_vec(&body).unwrap())) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let status = resp.status(); // Axum rejects malformed JSON payloads with 422 (Unprocessable Entity) @@ -2907,6 +2969,10 @@ async fn disc_git_diff_route_exists() { // git-diff may return non-JSON (raw diff text), so just verify the route exists (not 404) let app = test_app(); let req = Request::builder().method("GET").uri("/api/discussions/nonexistent/git-diff").body(Body::empty()).unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); assert_ne!(resp.status(), StatusCode::NOT_FOUND, "Route must exist"); } @@ -4141,6 +4207,10 @@ async fn send_message_to_no_agent_disc_skips_the_runner() { "target_agent": null, })).unwrap())) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); assert_eq!(resp.status(), StatusCode::OK); let body = resp.into_body().collect().await.unwrap().to_bytes(); @@ -4435,6 +4505,10 @@ async fn export_returns_zip() { .body(Body::empty()) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); assert_eq!(resp.status(), StatusCode::OK); assert_eq!( @@ -4461,7 +4535,7 @@ async fn export_returns_zip() { let mut contents = String::new(); std::io::Read::read_to_string(&mut data_file, &mut contents).unwrap(); let data: Value = serde_json::from_str(&contents).unwrap(); - assert_eq!(data["version"], 4); + assert_eq!(data["version"], kronn::models::db::CURRENT_EXPORT_VERSION); } #[tokio::test] @@ -4491,7 +4565,11 @@ async fn import_zip_roundtrip() { .uri("/api/config/export") .body(Body::empty()) .unwrap(); - let resp = app.oneshot(req).await.unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); + let resp = app.oneshot(req).await.unwrap(); resp.into_body().collect().await.unwrap().to_bytes() }; @@ -4513,6 +4591,10 @@ async fn import_zip_roundtrip() { .body(Body::from(multipart_body)) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let status = resp.status(); let body = resp.into_body().collect().await.unwrap().to_bytes(); @@ -4559,6 +4641,10 @@ async fn import_accepts_payload_over_2mb_default_body_limit() { .header("content-type", format!("multipart/form-data; boundary={}", boundary)) .body(Body::from(body)) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let status = resp.status(); let json: Value = serde_json::from_slice( @@ -4648,6 +4734,10 @@ async fn import_legacy_json_via_multipart() { .body(Body::from(multipart_body)) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let status = resp.status(); let body = resp.into_body().collect().await.unwrap().to_bytes(); @@ -4760,6 +4850,10 @@ async fn workflow_update_project_id_persists() { "project_id": project_id })).unwrap())) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let body = resp.into_body().collect().await.unwrap().to_bytes(); let update_json: Value = serde_json::from_slice(&body).unwrap(); @@ -4782,6 +4876,10 @@ async fn workflow_update_project_id_persists() { "project_id": null })).unwrap())) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); let body = resp.into_body().collect().await.unwrap().to_bytes(); let detach_json: Value = serde_json::from_slice(&body).unwrap(); @@ -4852,6 +4950,10 @@ async fn test_step_returns_sse_stream() { })).unwrap())) .unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); let resp = app.oneshot(req).await.unwrap(); // Should return 200 with SSE content-type (stream starts immediately) assert_eq!(resp.status(), StatusCode::OK); @@ -5709,17 +5811,26 @@ mod auth_middleware_tests { // ── Localhost bypass via X-Real-IP (nginx setup) ─────────────────────── #[tokio::test] - async fn x_real_ip_localhost_bypasses_auth_when_strict_off() { - // The Docker setup ships nginx that sets X-Real-IP to the real - // client IP. A loopback X-Real-IP means the request originated - // from the host machine and should bypass auth (documented - // self-hosted default). + #[serial_test::serial] + async fn x_real_ip_localhost_bypasses_auth_only_in_docker() { + // Docker: the bundled nginx OVERWRITES X-Real-IP, so a loopback value + // really means the host machine — trusted (self-hosted default). + std::env::set_var("KRONN_IN_DOCKER", "1"); let app = app_with_auth("expected-secret", false); let r = req_with_header("/api/setup/status", "x-real-ip", "127.0.0.1"); assert_eq!(status_of(app, r).await, StatusCode::OK); + + // Native: axum faces clients directly, the header is CLIENT-supplied — + // a LAN peer minting `X-Real-IP: 127.0.0.1` must not gain local trust + // (passe D: it bypassed the whole destructive gate). + std::env::remove_var("KRONN_IN_DOCKER"); + let app = app_with_auth("expected-secret", false); + let r = req_with_header("/api/setup/status", "x-real-ip", "127.0.0.1"); + assert_eq!(status_of(app, r).await, StatusCode::UNAUTHORIZED); } #[tokio::test] + #[serial_test::serial] async fn x_real_ip_public_does_not_bypass_auth() { // A public IP forwarded by nginx must require Bearer auth ; // otherwise any internet client trivially bypasses by relying @@ -5730,6 +5841,7 @@ mod auth_middleware_tests { } #[tokio::test] + #[serial_test::serial] async fn x_real_ip_forged_localhost_string_does_not_bypass() { // Hardened on 2026-05-10 — `is_local_ip("localhost")` returns // false. A misconfigured upstream forwarding the literal @@ -6038,7 +6150,11 @@ mod cold_api_handlers_tests { .body(Body::from(serde_json::to_vec(b).unwrap())).unwrap(), _ => panic!("unsupported method/body combo"), }; - let resp = app.oneshot(req).await.unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); + let resp = app.oneshot(req).await.unwrap(); let st = resp.status(); // We want : either 2xx (envelope or pass-through OK) or 4xx // (NotFound / BadRequest), NEVER a 5xx panic. @@ -6126,7 +6242,11 @@ mod cold_api_handlers_tests { let req = Request::builder() .method("PUT").uri(uri).header("content-type", "application/json") .body(Body::from(serde_json::to_vec(&body).unwrap())).unwrap(); - let resp = app.oneshot(req).await.unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); + let resp = app.oneshot(req).await.unwrap(); let st = resp.status(); let bytes = resp.into_body().collect().await.unwrap().to_bytes(); let json: Value = serde_json::from_slice(&bytes).unwrap_or(Value::Null); @@ -6507,7 +6627,11 @@ mod cold_api_handlers_tests { .header("content-type", "application/json") .body(Body::from(serde_json::to_vec(&serde_json::json!({ "label": "x" })).unwrap())) .unwrap(); - let resp = app.oneshot(req).await.unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); + let resp = app.oneshot(req).await.unwrap(); assert!(resp.status().is_success() || resp.status().is_client_error()); } envelope_delete!(mcps_delete_config_unknown_id, "/api/mcps/configs/nope"); @@ -6525,7 +6649,11 @@ mod cold_api_handlers_tests { .header("content-type", "application/json") .body(Body::from(serde_json::to_vec(&serde_json::json!({ "name": "x" })).unwrap())) .unwrap(); - let resp = app.oneshot(req).await.unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); + let resp = app.oneshot(req).await.unwrap(); assert!(resp.status().is_success() || resp.status().is_client_error()); } envelope_delete!(profiles_delete_unknown_id, "/api/profiles/nope"); @@ -6537,7 +6665,11 @@ mod cold_api_handlers_tests { .header("content-type", "application/json") .body(Body::from(serde_json::to_vec(&serde_json::json!({ "name": "x" })).unwrap())) .unwrap(); - let resp = app.oneshot(req).await.unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); + let resp = app.oneshot(req).await.unwrap(); assert!(resp.status().is_success() || resp.status().is_client_error()); } envelope_delete!(directives_delete_unknown_id, "/api/directives/nope"); @@ -6551,7 +6683,11 @@ mod cold_api_handlers_tests { .header("content-type", "application/json") .body(Body::from(serde_json::to_vec(&serde_json::json!({ "name": "x" })).unwrap())) .unwrap(); - let resp = app.oneshot(req).await.unwrap(); + let mut req = req; + req.extensions_mut().insert(axum::extract::ConnectInfo( + std::net::SocketAddr::from(([127, 0, 0, 1], 45678)), + )); + let resp = app.oneshot(req).await.unwrap(); assert!(resp.status().is_success() || resp.status().is_client_error()); } diff --git a/backend/tests/config_write_guard.rs b/backend/tests/config_write_guard.rs new file mode 100644 index 00000000..a28437e9 --- /dev/null +++ b/backend/tests/config_write_guard.rs @@ -0,0 +1,11 @@ +// Proof test for the runtime write guard: an integration test that FORGOT +// isolate_config_dir() must be refused, not silently clobber the real config. +#[tokio::test] +async fn unisolated_config_write_is_refused() { + // Deliberately NO isolate_config_dir() and KRONN_DATA_DIR cleared. + std::env::remove_var("KRONN_DATA_DIR"); + let cfg = kronn::core::config::default_config(); + let err = kronn::core::config::save(&cfg).await + .expect_err("write from an unisolated test binary must be refused"); + assert!(err.to_string().contains("isolate_config_dir"), "{err}"); +} diff --git a/backend/tests/learnings_api.rs b/backend/tests/learnings_api.rs index e4368093..c2d6a5bd 100644 --- a/backend/tests/learnings_api.rs +++ b/backend/tests/learnings_api.rs @@ -19,7 +19,19 @@ use kronn::{build_router_with_auth, AppState, DEFAULT_MAX_CONCURRENT_AGENTS}; // concurrent set_var/remove_var can't cross-contaminate. static ENV_LOCK: Mutex<()> = Mutex::const_new(()); +/// See api_tests.rs — without this, handler-level config saves during tests +/// write the developer's REAL config.toml (2026-07-13 incident). +fn isolate_config_dir() { + static INIT: std::sync::Once = std::sync::Once::new(); + INIT.call_once(|| { + let dir = std::env::temp_dir().join(format!("kronn-inttest-cfg-{}", std::process::id())); + std::fs::create_dir_all(&dir).ok(); + std::env::set_var("KRONN_DATA_DIR", &dir); + }); +} + fn app_with(enabled: bool) -> Router { + isolate_config_dir(); let db = Arc::new(kronn::db::Database::open_in_memory().expect("in-memory DB")); let mut cfg = kronn::core::config::default_config(); cfg.server.auth_token = None; diff --git a/docs/design/adr-001-db-connection-model.md b/docs/design/adr-001-db-connection-model.md new file mode 100644 index 00000000..48042966 --- /dev/null +++ b/docs/design/adr-001-db-connection-model.md @@ -0,0 +1,107 @@ +# ADR-001 — SQLite connection model: keep the single connection, defer a separate read connection + +- **Status**: PROPOSED (pass 7 of the 0.9 plan — Codex review requested) +- **Date**: 2026-07-13 +- **Upstream context**: 2026-07 audit ("structural amplifier" finding), the + pass-A1 writer inventory, 0.8.11 incidents (API freeze during backup, + poisoned mutex). + +## Context + +All DB access goes through ONE shared connection: +`Arc>`, exposed by `Database::with_conn`, which +runs every closure inside `tokio::task::spawn_blocking` while holding the lock +(`backend/src/db/mod.rs`). WAL is on by default (`KRONN_DB_WAL=0` for network +mounts), `busy_timeout=5000`. + +Observed structural consequences: + +1. **Total serialization** — reads AND writes compete for the same mutex; WAL + (which would allow N readers concurrent with 1 writer) is neutralized by + our own application-level lock. +2. **Incident amplification** — any slow code under the lock freezes the WHOLE + API. Lived through in 0.8.11: the backup copy ran in 5-page steps with a + 50 ms pause while holding the lock (~2.5 s of global freeze per MB); fixed + with a single-step copy, but the bug CLASS stays open as long as the lock + is unique. +3. **Total outage on panic** — a panic inside a closure poisoned the mutex + forever (lived through, fixed in 0.8.11: poison recovery + catch_unwind). + Same remark: symptom treated, structure unchanged. + +## What the A1 inventory contributes to the decision + +Pass A1 inventoried the **9 writers** of `workflow_runs.status` and converged +them onto SQL-predicate primitives (`update_run_progress` guarded by status, +atomic `claim_run_status`, batch counters frozen outside Running). Two +properties follow: + +- **Transition integrity does NOT depend on the single connection.** Every + critical write is an atomic `UPDATE … WHERE status = ?` at the SQL level; + two concurrent connections would produce the same outcome (exactly one + winner), because the arbitration lives in the predicate, not in the Rust + mutex. The single connection is therefore NOT a correctness ingredient — + it is a simplicity choice. +- **Writers are few and short.** No long multi-step transactions on the runs + path; large payloads (step_results_json) are single writes. The real + contention comes from HEAVY READS (run lists with full step_results, + exports) sharing the queue with these short writes. + +## Options + +### O1 — Strict status quo (single connection, nothing else) +- ✅ Zero risk, zero work. +- ❌ The "slow code under the lock = global freeze" class remains; every new + read-heavy feature (dashboards, exports, learning) pays the tax again. + +### O2 — Separate READ connection(s) (WAL readers) +A second connection (or a small pool, N=2-3) opened for reads, used by +consultation endpoints (`with_read_conn`); all writes stay on the current +single connection. +- ✅ Finally exploits WAL (readers never blocked by the writer); removes the + global-freeze class for reads; incremental migration (endpoint by + endpoint); NO change to the writers — the A1 inventory stays fully valid. +- ✅ Bounded regression risk: a WAL read sees a consistent snapshot; the worst + case is a read slightly behind an in-flight write — already true today at + the granularity of frontend polling. +- ❌ Two disciplines to maintain ("which connection for this handler?"); risk + of drift if a writer sneaks onto the read connection (simple guard: + `PRAGMA query_only=1` on read connections — a violation is an immediate + SQLite error). + +### O3 — Generalized pool (r2d2 / deadpool, writes included) +- ✅ The "normal" model for SQL applications. +- ❌ SQLite has only ONE writer at a time: a pool of writers parallelizes + nothing, it moves the serialization to `SQLITE_BUSY`/busy_timeout with a + WORSE failure mode (scattered non-deterministic timeouts instead of a + wait queue). All writes would need routing to a dedicated connection + anyway — that is O2 with more dependencies. +- ❌ Invalidates the serialization assumption some read-modify-write closures + still rely on outside the runs path (config, contacts): a full audit would + be required first, for no benefit beyond O2. + +## Decision (proposed) + +**Adopt O2 as the target; defer its execution until after 0.9.** + +1. **Now (0.9)**: nothing blocks the release on this front. Both known + structural incidents have point fixes, transition correctness is + guaranteed by the SQL predicates (A1), and the `kronn::invariant` + observability will surface blocked writes. +2. **Post-0.9, first O2 iteration**: add `with_read_conn` (a `query_only` + read connection, same WAL), migrate the 3 heaviest read endpoints (run + list, exports, dashboards). Measure before/after on P99 handler freeze + during a heavy write. +3. **Non-regression guard**: a test that opens both connections, writes on + one, and verifies visibility plus the `query_only` enforcement on the + other. +4. **O3 is explicitly rejected** as long as SQLite remains the engine — the + day a real multi-writer becomes necessary, the question is "Postgres?", + not "SQLite pool?". + +## Consequences + +- No code change in 0.9 (this document only). +- The `with_conn` API remains the write path; every new heavy-read endpoint + must target `with_read_conn` from its creation post-0.9. +- The project rule "no writes outside `with_conn`" extends to: "no heavy + SELECT on the write connection once `with_read_conn` exists". diff --git a/docs/tech-debt/TD-20260630-apicall-link-header-pagination.md b/docs/tech-debt/TD-20260630-apicall-link-header-pagination.md index dcd1bba5..8488fa4f 100644 --- a/docs/tech-debt/TD-20260630-apicall-link-header-pagination.md +++ b/docs/tech-debt/TD-20260630-apicall-link-header-pagination.md @@ -63,6 +63,12 @@ any other `Link`-paginated bare-array API — so workflows stop silently truncating long lists. - **Next step**: create ticket. +- **Status (plan 0.9, C2 — 2026-07-13)**: ✅ RESOLVED. + - `send_with_retry` now returns the raw `Link` response header alongside the parsed body; `parse_link_next` extracts `rel="next"` (quoted and unquoted forms). + - New `PaginationSpec::LinkHeader { page_size_param, page_size, max_pages }` variant: seeds page 1 (e.g. `per_page=100`), then follows the server's own `rel="next"` URL verbatim (its query params adopted wholesale) until absent — reusing the existing `max_pages` cap + `[SIGNAL: PAGINATION_TRUNCATED]`. + - `walk_pages` accumulates **bare top-level arrays** (the merged result is then a plain array, so `$[*].id` extracts work); object-envelope bodies with a `Link` header (GitHub search) still merge via `detect_items_key`. + - Locked by wiremock tests: 3-page bare-array merge in order, single page without `Link`, cap → truncated signal, and `parse_link_next` unit cases. + - The PR-Review workflow's `per_page=100` mitigation can now be replaced by `{"type":"LinkHeader","page_size_param":"per_page","page_size":100}` on `fetch_reviews`/`fetch_comments`/`fetch_files` to cover the >100 case. ## Notes diff --git a/docs/tech-debt/TD-20260701-typegen-generated-aggregate.md b/docs/tech-debt/TD-20260701-typegen-generated-aggregate.md index 7ddfcdac..011c11c9 100644 --- a/docs/tech-debt/TD-20260701-typegen-generated-aggregate.md +++ b/docs/tech-debt/TD-20260701-typegen-generated-aggregate.md @@ -12,3 +12,9 @@ - `frontend/scripts/assemble-generated-types.mjs` — a full-regen assembler (strips headers/imports, `bigint`→`number`, keeps ts-rs `export type`). NOT wired to overwrite: a faithful wholesale regen still needs the serde-default→optional transform (the ~189-call-site churn described above) — that remains the dedicated migration. The guard makes it safe to defer. - Misleading "AUTO-GENERATED by ts-rs" header replaced with an accurate "SEMI-GENERATED + how to sync" banner. - REMAINING: the full auto-regen (retire the hand-maintained aggregate) — do the serde-default→optional transform + adapt the frontend. No longer urgent now that drift is guarded. +- **Status (plan 0.9, C1)**: ✅ RESOLVED — `generated.ts` is fully auto-regenerated. + - The original constraint was CONFIRMED, not obsolete: ts-rs 12 emits `?` only for `skip_serializing_if`/`#[ts(optional)]`, NOT for a bare `#[serde(default)]` (verified: `CreateDiscussionRequest.skill_ids` required vs `WorkflowStep.skill_ids?`). The faithful transform is implemented as a Rust-source scan inside the assembler: on REQUEST-direction types (Deserialize-only, or `…Request`-named), fields with `#[serde(default)]` (per-field or container) or plain `Option` (serde reads missing as None) become optional. Response/bidirectional types are left strict so read sites keep non-null guarantees. + - `make typegen` = `cargo test export_bindings` + `assemble-generated-types.mjs` OVERWRITING `generated.ts` (281 types, was 194 hand-maintained; deterministic output, no trailing whitespace, hygiene locked by a vitest test). + - CI guard = regenerate + `git diff --exit-code` on `generated.ts` (a Rust model change without `make typegen` fails the build). `check-types-drift.mjs` is superseded and removed. + - 3 types the aggregate had but Rust never exported now carry `#[ts(export)]` (`LearningProposeRequest` via rename, `ProposeResult`, `AdoptHostMcpRequest`); 2 dead types dropped (`RevealSecretRequest`, `SaveTokensRequest` — no usage). + - Call-site churn absorbed: ~190 tsc errors on the first honest run (ANSI colors had defeated the earlier "0 errors" readings — always check `tsc`'s EXIT CODE) → 8 production sites fixed (typed `BodyEditor.onChange`, JSON.parse boundaries cast to `JsonValue`, two step-builders now set `step_type`/`output_format`, Dashboard `McpOverview` fallback completed) + test mocks completed across ~20 files. Final: `tsc` exit 0, vitest 2550/2550. diff --git a/docs/tech-debt/TD-20260713-typescript-7-native.md b/docs/tech-debt/TD-20260713-typescript-7-native.md new file mode 100644 index 00000000..816e33a4 --- /dev/null +++ b/docs/tech-debt/TD-20260713-typescript-7-native.md @@ -0,0 +1,10 @@ +- **ID**: TD-20260713-typescript-7-native +- **Area**: Frontend / Tooling / CI +- **Problem (fact)**: Kronn uses TypeScript 6.0.3 and runs `tsc -b && vite build` for its frontend build. TypeScript 7.0 is a native Go implementation whose compiler is expected to reduce full-build time substantially, but it does not yet expose the programmatic API relied on by tools such as typescript-eslint. [src: file: frontend/package.json:11] [src: file: frontend/package.json:56] [src: url: https://devblogs.microsoft.com/typescript/announcing-typescript-7-0/] +- **Why we can't fix now (constraint)**: Replacing the `typescript` dependency directly would risk breaking lint and any other dependency that imports the TypeScript API. The upstream-supported transition is a side-by-side install: TypeScript 7 provides `tsc` for build/type-checking, while an npm alias to `@typescript/typescript6` remains available to API-dependent tooling. This requires a lockfile change and a full compatibility/performance validation, which is out of scope for the 0.8.11 hardening PR. [src: url: https://devblogs.microsoft.com/typescript/announcing-typescript-7-0/] +- **Impact**: potentially shorter local and CI type-check/build times, lower build memory use, and faster editor diagnostics; no confirmed Kronn-specific gain has been measured yet. +- **Where (pointers)**: `frontend/package.json` (`build` script and dev dependencies), `frontend/pnpm-lock.yaml`, `frontend/eslint.config.js`, `frontend/vite.config.ts`. +- **Suggested direction (non-binding)**: Add TypeScript 7 under an alias for the `tsc` executable and retain `typescript` as an alias to `@typescript/typescript6` for typescript-eslint. Keep `vite build`, lint and Vitest unchanged initially. Benchmark the existing and new paths on the same machine/CI runner; compare diagnostics from TypeScript 6 and 7 before changing defaults. Do not tune `--checkers` or `--builders` until a baseline exists. +- **Acceptance criteria**: `pnpm build`, `pnpm test`, and lint remain green; the TypeScript 7 build reports no unexpected diagnostic delta; measured build duration and peak memory are recorded; rollback to TypeScript 6 is one dependency/config change. +- **Next step**: create a dedicated performance/toolchain PR after 0.8.11. +- **Status**: OPEN diff --git a/frontend/scripts/assemble-generated-types.mjs b/frontend/scripts/assemble-generated-types.mjs index 76028b22..8aab920f 100644 --- a/frontend/scripts/assemble-generated-types.mjs +++ b/frontend/scripts/assemble-generated-types.mjs @@ -18,8 +18,90 @@ import { fileURLToPath } from 'node:url'; const HERE = dirname(fileURLToPath(import.meta.url)); const BINDINGS = join(HERE, '..', '..', 'backend', 'bindings'); +const RUST_SRC = join(HERE, '..', '..', 'backend', 'src'); const OUT = join(HERE, '..', 'src', 'types', 'generated.ts'); +// ─── serde-default → optional (C1) ───────────────────────────────────────── +// ts-rs only emits `?` for `skip_serializing_if` / `#[ts(optional)]`; a bare +// `#[serde(default)]` — the contract that lets API clients omit the field — +// stays REQUIRED in the binding. That information only exists in the Rust +// source, so scan it: struct → set of fields to optionalize in the aggregate. +// Container-level `#[serde(default)]` marks every field. Handles per-field +// `serde(rename)`, container `rename_all = "camelCase"`, `#[ts(rename)]`. + +const toCamel = s => s.replace(/_([a-z0-9])/g, (_, c) => c.toUpperCase()); + +export function scanRustOptionals(dir) { + /** @type {Map>} TS type name → field names to mark `?` */ + const optionals = new Map(); + const stack = [dir]; + while (stack.length) { + const d = stack.pop(); + for (const name of readdirSync(d)) { + const p = join(d, name); + if (statSync(p).isDirectory()) { stack.push(p); continue; } + if (!name.endsWith('.rs')) continue; + const src = readFileSync(p, 'utf8'); + // One match per struct: the attribute block directly above + the body. + for (const m of src.matchAll( + /((?:^[ \t]*#\[[^\n]*\]\s*\n|^[ \t]*\/\/\/[^\n]*\n)+)^[ \t]*pub struct (\w+)\s*\{([\s\S]*?)^\}/gm + )) { + const [, attrs, rustName, body] = m; + if (!/derive\([^)]*\bTS\b/.test(attrs) && !/#\[ts\(/.test(attrs)) continue; + // Only REQUEST-direction types: there `serde(default)` (or a plain + // `Option`, which serde reads as None when missing) means "the + // client may omit this". On a response/bidirectional type the field + // is always present in the payload — optionalizing it would force + // spurious null-checks at every read site. Direction = Deserialize + // without Serialize, or an explicit `…Request` name (some request + // types derive Serialize incidentally). + const tsName = attrs.match(/#\[ts\([^\]]*rename\s*=\s*"(\w+)"/)?.[1] ?? rustName; + const derives = attrs.match(/derive\(([^)]*)\)/)?.[1] ?? ''; + const isRequest = (/\bDeserialize\b/.test(derives) && !/\bSerialize\b/.test(derives)) + || /Request$/.test(tsName); + if (!isRequest) continue; + const containerDefault = /#\[serde\((?:[^\]]*,\s*)?default\s*[,)\]]/.test(attrs); + const renameAll = attrs.match(/rename_all\s*=\s*"(\w+)"/)?.[1]; + const set = optionals.get(tsName) ?? new Set(); + // Walk fields with the attribute lines attached to each. + for (const f of body.matchAll( + /((?:^[ \t]*#\[[^\n]*\]\s*\n)*)^[ \t]*pub (?:r#)?(\w+)\s*:\s*([^,\n]+)/gm + )) { + const [, fAttrs, fName, fType] = f; + if (/#\[serde\([^\]]*skip[,)\s]/.test(fAttrs)) continue; // not exported + const fieldDefault = /#\[serde\([^\]]*\bdefault\b/.test(fAttrs); + // serde deserializes a MISSING `Option` as None even without + // `default` — on a request type every Option field is omittable. + const isOption = /^Option\s* basename(a).localeCompare(basename(b))); @@ -53,6 +137,10 @@ for (const f of files) { // JSON.parse yields `number` anyway (not BigInt). Matches the convention the // hand-maintained file used before this generator existed. src = src.replace(/\bbigint\b/g, 'number'); + // ts-rs leaves trailing spaces after fields that precede doc comments — + // thousands of `git diff --check` violations. Normalize per line. + src = src.split('\n').map(l => l.replace(/[ \t]+$/, '')).join('\n'); + src = applyOptionals(name, src, OPTIONALS); blocks.push(src); } @@ -66,3 +154,4 @@ const header = `// ╔═══════════════════ writeFileSync(OUT, header + '\n' + blocks.join('\n\n') + '\n'); console.log(`assembled ${blocks.length} types → ${OUT}`); +} diff --git a/frontend/scripts/check-types-drift.mjs b/frontend/scripts/check-types-drift.mjs deleted file mode 100644 index 1e64952a..00000000 --- a/frontend/scripts/check-types-drift.mjs +++ /dev/null @@ -1,205 +0,0 @@ -#!/usr/bin/env node -// C8 (0.8.11) — FIELD-level anti-drift guard for the hand-curated -// `src/types/generated.ts`. -// -// `generated.ts` is (still) hand-maintained with deliberate frontend-friendly -// simplifications (u64→number, serde-default→optional, JsonValue widening); a -// faithful ts-rs regen is a separate migration (see assemble-generated-types.mjs -// + TD-20260701). Many Rust types are backend-only and INTENTIONALLY absent from -// the aggregate, so a missing-whole-type check is all false positives. -// -// The REAL recurring pain (hit 4× in 0.8.x) is: a Rust type the frontend DOES -// use gains a field, and generated.ts is never updated → the field is silently -// invisible to the UI (parent_run_id, on_timeout, Interrupted…). So this guard, -// for every type that generated.ts ALREADY declares, compares its field NAMES -// against the ts-rs binding and fails if the binding has fields the aggregate is -// missing. Field NAMES only (not types) — so the deliberate bigint/optional -// simplifications never trip it. Tagged unions are compared too: the union of -// field names across ALL top-level `{ … }` variants on each side (so drift in -// a non-first variant is caught). Pure aliases (no object body) are skipped — -// and reported, so "OK" can't silently mean "compared nothing". -// Run after `cargo test export_bindings`. -import { readFileSync, readdirSync, statSync, existsSync } from 'node:fs'; -import { join, basename, dirname } from 'node:path'; -import { fileURLToPath } from 'node:url'; - -const HERE = dirname(fileURLToPath(import.meta.url)); -const BINDINGS = join(HERE, '..', '..', 'backend', 'bindings'); -const GENERATED = join(HERE, '..', 'src', 'types', 'generated.ts'); - -if (!existsSync(BINDINGS)) { - console.error(`[types-drift] bindings dir missing — run: (cd backend && cargo test export_bindings)`); - process.exit(2); -} - -function walk(dir) { - const out = []; - for (const n of readdirSync(dir)) { - const p = join(dir, n); - if (statSync(p).isDirectory()) out.push(...walk(p)); - else if (n.endsWith('.ts')) out.push(p); - } - return out; -} - -/** Strip `/* … *​/` block comments + `// …` line comments so field scans don't - * pick up doc-comment prose. */ -function stripComments(s) { - return s.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^\s*\/\/.*$/gm, ''); -} - -/** Skip a string literal starting at src[i] (a quote char); returns the index - * of the closing quote (or end of src). Keeps braces/pipes inside strings - * from confusing the depth trackers below. */ -function skipString(src, i) { - const q = src[i]; - for (i++; i < src.length; i++) { - if (src[i] === '\\') i++; - else if (src[i] === q) return i; - } - return i; -} - -/** Top-level field names in an object-type body. Handles nested objects by - * tracking brace depth — only depth-1 `name:` / `name?:` are fields. - * Handles ts-rs quoted keys too (`"type": "Agent"`). */ -function fieldNames(body) { - const clean = stripComments(body); - const names = new Set(); - let depth = 0; - // Walk token by token, recording keys at depth 1 immediately before a `:`. - // Alternatives: brace | quoted key | bare string (consumed, no depth effect) | bare key. - const re = /([{}])|"((?:[^"\\]|\\.)*)"\s*\??\s*:|("(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*')|(\b[a-zA-Z_$][a-zA-Z0-9_$]*)\s*\??\s*:/g; - let m; - while ((m = re.exec(clean))) { - if (m[1] === '{') depth++; - else if (m[1] === '}') depth--; - else if (m[2] !== undefined && depth === 1) names.add(m[2]); - else if (m[4] && depth === 1) names.add(m[4]); - } - return names; -} - -/** Extract the declaration for `export (interface|type) Name` from src. - * - interface → { kind: 'interface', rhs: balanced `{ … }` body } - * - type alias → { kind: 'type', rhs: right-hand side after `=` }, bounded at - * the first top-level `;` (or balanced end) so it can NEVER run past the - * current declaration into the next type. - * null if the name isn't declared. */ -function declaration(src, name) { - const re = new RegExp(`export (interface|type) ${name}\\b`); - const m = re.exec(src); - if (!m) return null; - let i = m.index + m[0].length; - if (m[1] === 'interface') { - // Skip to the opening `{` (past any extends clause), then balance. - while (i < src.length && src[i] !== '{' && src[i] !== ';') i++; - if (src[i] !== '{') return null; - let depth = 0; - const open = i; - for (; i < src.length; i++) { - const c = src[i]; - if (c === '"' || c === "'" || c === '`') { i = skipString(src, i); continue; } - if (c === '{') depth++; - else if (c === '}') { depth--; if (depth === 0) return { kind: 'interface', rhs: src.slice(open, i + 1) }; } - } - return null; - } - // type alias: skip generic params to the `=` at angle-depth 0. - let angle = 0; - for (;; i++) { - if (i >= src.length) return null; - const c = src[i]; - if (c === '<') angle++; - else if (c === '>') angle--; - else if (c === '=' && angle === 0) { i++; break; } - else if (c === ';') return null; - } - // Right-hand side ends at the first `;` outside any bracket/string nesting. - let depth = 0; - const start = i; - for (; i < src.length; i++) { - const c = src[i]; - if (c === '"' || c === "'" || c === '`') { i = skipString(src, i); continue; } - if (c === '{' || c === '(' || c === '[') depth++; - else if (c === '}' || c === ')' || c === ']') depth--; - else if (c === ';' && depth === 0) break; - } - return { kind: 'type', rhs: src.slice(start, i) }; -} - -/** Split a type-alias right-hand side on TOP-LEVEL `|` only (depth-aware, so - * pipes nested in variant bodies / generics / strings don't split). */ -function splitUnion(rhs) { - const parts = []; - let depth = 0; - let start = 0; - for (let i = 0; i < rhs.length; i++) { - const c = rhs[i]; - if (c === '"' || c === "'" || c === '`') { i = skipString(rhs, i); continue; } - if (c === '{' || c === '(' || c === '[' || c === '<') depth++; - else if (c === '}' || c === ')' || c === ']' || c === '>') depth--; - else if (c === '|' && depth === 0) { parts.push(rhs.slice(start, i)); start = i + 1; } - } - parts.push(rhs.slice(start)); - return parts; -} - -/** Union of field names across ALL top-level object variants of a declaration - * (tagged unions included — every `{ … }` variant contributes, not just the - * first). Returns null when the declaration has no object body at all - * (pure alias, e.g. `type OnInvalid = "Continue" | "Fail"`). */ -function unionFields(decl) { - if (!decl) return null; - if (decl.kind === 'interface') return fieldNames(decl.rhs); - let hasObjectVariant = false; - const names = new Set(); - for (const part of splitUnion(decl.rhs)) { - const t = part.trim(); - if (t.startsWith('{')) { - hasObjectVariant = true; - for (const n of fieldNames(t)) names.add(n); - } - } - return hasObjectVariant ? names : null; -} - -const generated = stripComments(readFileSync(GENERATED, 'utf8')); -const declared = new Set([...generated.matchAll(/export (?:interface|type) (\w+)/g)].map((m) => m[1])); - -const drift = []; -let compared = 0; -const skipped = []; // { name, reason } — declared but not field-comparable -for (const f of walk(BINDINGS)) { - const name = basename(f, '.ts'); - if (!declared.has(name)) continue; // backend-only type not in the aggregate → out of scope - const bindingSrc = stripComments(readFileSync(f, 'utf8')); - const want = unionFields(declaration(bindingSrc, name)); - const have = unionFields(declaration(generated, name)); - if (!want || !have) { - const side = !want && !have ? 'both sides' : !want ? 'binding side' : 'generated.ts side'; - skipped.push({ name, reason: `pure alias (no object body) on ${side}` }); - continue; - } - compared++; - const missing = [...want].filter((fld) => !have.has(fld)); - if (missing.length) drift.push({ name, missing }); -} - -const frontendOnly = declared.size - compared - skipped.length; // declared in generated.ts, no binding file -const coverage = - `[types-drift] coverage: ${compared} type(s) field-compared, ${skipped.length} skipped` + - ` (alias/no object body), ${frontendOnly} frontend-only (no binding), of ${declared.size} declared.`; - -if (drift.length) { - console.error(`[types-drift] ${drift.length} frontend type(s) are MISSING fields present in the Rust model:`); - for (const d of drift) console.error(` ${d.name}: ${d.missing.join(', ')}`); - console.error(`\nAdd the field(s) to src/types/generated.ts (see backend/bindings/.ts).`); - console.error(coverage); - process.exit(1); -} -if (skipped.length) { - console.log(`[types-drift] skipped (no field comparison): ${skipped.map((s) => s.name).sort().join(', ')}`); -} -console.log(coverage); -console.log(`[types-drift] OK — no field drift on the ${compared} field-compared frontend-declared types.`); diff --git a/frontend/src/components/__tests__/ActiveAuditsPopover.test.tsx b/frontend/src/components/__tests__/ActiveAuditsPopover.test.tsx index 75b8895e..1e0f6dbe 100644 --- a/frontend/src/components/__tests__/ActiveAuditsPopover.test.tsx +++ b/frontend/src/components/__tests__/ActiveAuditsPopover.test.tsx @@ -31,7 +31,7 @@ const proj = (id: string, name: string): Project => ({ repo_url: null, token_override: null, ai_config: { detected: false, configs: [] }, audit_status: 'NoTemplate', - ai_todo_count: 0, tech_debt_count: 0, needs_docs_migration: false, + ai_todo_count: 0, tech_debt_count: 0, needs_docs_migration: false, path_exists: true, default_skill_ids: [], briefing_notes: null, linked_repos: [], diff --git a/frontend/src/components/__tests__/ChatHeader.pendingFiles.test.tsx b/frontend/src/components/__tests__/ChatHeader.pendingFiles.test.tsx index cad940b0..1324af44 100644 --- a/frontend/src/components/__tests__/ChatHeader.pendingFiles.test.tsx +++ b/frontend/src/components/__tests__/ChatHeader.pendingFiles.test.tsx @@ -30,7 +30,7 @@ function makeDiscussion(overrides: Partial = {}): Discussion { language: 'en', participants: ['ClaudeCode' as any], messages: [], - message_count: 0, non_system_message_count: 0, + message_count: 0, non_system_message_count: 0, tier: "default" as const, summary_strategy: "Auto" as const, introspection_call_count: 0, archived: false, pinned: false, pin_first_message: false, workspace_mode: 'Isolated', diff --git a/frontend/src/components/__tests__/ChatHeader.profileEditor.test.tsx b/frontend/src/components/__tests__/ChatHeader.profileEditor.test.tsx index 018072a9..ea2626b2 100644 --- a/frontend/src/components/__tests__/ChatHeader.profileEditor.test.tsx +++ b/frontend/src/components/__tests__/ChatHeader.profileEditor.test.tsx @@ -32,7 +32,7 @@ function makeDiscussion(over: Partial = {}): Discussion { language: 'en', participants: ['ClaudeCode' as any], messages: [], - message_count: 0, non_system_message_count: 0, + message_count: 0, non_system_message_count: 0, tier: "default" as const, summary_strategy: "Auto" as const, introspection_call_count: 0, archived: false, pinned: false, pin_first_message: false, workspace_mode: 'Direct', diff --git a/frontend/src/components/__tests__/DiscussionSidebar.grouping.test.tsx b/frontend/src/components/__tests__/DiscussionSidebar.grouping.test.tsx index 3db30705..0bf40b44 100644 --- a/frontend/src/components/__tests__/DiscussionSidebar.grouping.test.tsx +++ b/frontend/src/components/__tests__/DiscussionSidebar.grouping.test.tsx @@ -67,7 +67,7 @@ const mkDisc = (over: Partial & { id: string }): Discussion => ({ participants: ['ClaudeCode'], messages: [], message_count: 0, - non_system_message_count: 0, + non_system_message_count: 0, tier: "default" as const, summary_strategy: "Auto" as const, introspection_call_count: 0, archived: false, pinned: false, pin_first_message: false, workspace_mode: 'Direct', @@ -84,7 +84,7 @@ const mkProject = (id: string, name: string, repo_url: string | null = null): Pr token_override: null, ai_config: { detected: false, configs: [] }, audit_status: 'NoTemplate', - ai_todo_count: 0, + ai_todo_count: 0, tech_debt_count: 0, needs_docs_migration: false, path_exists: true, created_at: '2026-05-15T10:00:00Z', updated_at: '2026-05-15T10:00:00Z', }); @@ -213,7 +213,7 @@ describe('DiscussionSidebar — grouping', () => { // System rows). Here the disc has 12 raw rows but only 7 user/agent ones. const discussions = [mkDisc({ id: 'd1', project_id: null, title: 'Click me', - message_count: 12, non_system_message_count: 7, + message_count: 12, non_system_message_count: 7, tier: "default" as const, summary_strategy: "Auto" as const, introspection_call_count: 0, })]; render(); await waitFor(() => expect(projectsApi.discSources).toHaveBeenCalled()); diff --git a/frontend/src/components/__tests__/DiscussionSidebar.markAllRead.test.tsx b/frontend/src/components/__tests__/DiscussionSidebar.markAllRead.test.tsx index 0e2e2d7e..157f2e77 100644 --- a/frontend/src/components/__tests__/DiscussionSidebar.markAllRead.test.tsx +++ b/frontend/src/components/__tests__/DiscussionSidebar.markAllRead.test.tsx @@ -54,7 +54,7 @@ const mkDisc = (id: string, msgCount: number, archived = false): Discussion => ( language: 'fr', participants: ['ClaudeCode'], messages: [], - message_count: msgCount, non_system_message_count: msgCount, + message_count: msgCount, non_system_message_count: msgCount, tier: "default" as const, summary_strategy: "Auto" as const, introspection_call_count: 0, archived, pinned: false, pin_first_message: false, workspace_mode: 'Direct', diff --git a/frontend/src/components/__tests__/DiscussionSidebar.sourceBadge.test.tsx b/frontend/src/components/__tests__/DiscussionSidebar.sourceBadge.test.tsx index d64cb95e..53707ab2 100644 --- a/frontend/src/components/__tests__/DiscussionSidebar.sourceBadge.test.tsx +++ b/frontend/src/components/__tests__/DiscussionSidebar.sourceBadge.test.tsx @@ -58,7 +58,7 @@ const mkDisc = (id: string, title: string): Discussion => ({ language: 'fr', participants: ['ClaudeCode'], messages: [], - message_count: 0, non_system_message_count: 0, + message_count: 0, non_system_message_count: 0, tier: "default" as const, summary_strategy: "Auto" as const, introspection_call_count: 0, archived: false, pinned: false, pin_first_message: false, workspace_mode: 'Direct', diff --git a/frontend/src/components/__tests__/HostSyncPreview.test.tsx b/frontend/src/components/__tests__/HostSyncPreview.test.tsx index 7c5994bd..f75827b4 100644 --- a/frontend/src/components/__tests__/HostSyncPreview.test.tsx +++ b/frontend/src/components/__tests__/HostSyncPreview.test.tsx @@ -16,7 +16,7 @@ function mkProject(id: string, name: string, path: string): Project { token_override: null, ai_config: { detected: false, configs: [] }, audit_status: 'NoTemplate', - ai_todo_count: 0, + ai_todo_count: 0, tech_debt_count: 0, needs_docs_migration: false, path_exists: true, created_at: '2026-04-27T00:00:00Z', updated_at: '2026-04-27T00:00:00Z', }; diff --git a/frontend/src/components/__tests__/MessageBubble.lintPill.test.tsx b/frontend/src/components/__tests__/MessageBubble.lintPill.test.tsx index 828a86f1..e27c13ab 100644 --- a/frontend/src/components/__tests__/MessageBubble.lintPill.test.tsx +++ b/frontend/src/components/__tests__/MessageBubble.lintPill.test.tsx @@ -76,7 +76,7 @@ function renderBubble(lint: LintReport | null) { describe('MessageBubble — anti-hallucination lint pill', () => { it('renders a fabricated (red) pill when a citation did not verify', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [ { raw: 'ghost.rs:1', kind: 'file', status: 'not_found', detail: 'file not found: ghost.rs' }, @@ -91,7 +91,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('renders an unsourced (amber) pill when only the heuristic flagged claims', () => { renderBubble({ - unsourced_count: 2, + unverified_count: 0, unsourced_count: 2, flagged_spans: [{ text: 'The retry uses backoff', reason: 'uses ' }], sources: [], fabricated_count: 0, @@ -104,7 +104,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('fabricated takes precedence over unsourced for the headline color', () => { renderBubble({ - unsourced_count: 3, + unverified_count: 0, unsourced_count: 3, flagged_spans: [{ text: 'x', reason: 'y' }], sources: [{ raw: 'a.rs:9', kind: 'file', status: 'out_of_bounds', detail: 'beyond length' }], fabricated_count: 1, @@ -123,7 +123,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { // green pill (see "renders a verified (green) pill …" below), so the // pre-0.8.7 shape (verified source = no pill) is no longer the contract. renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [], fabricated_count: 0, @@ -133,7 +133,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('clicking the pill toggles a detail panel with bad sources + flagged spans', () => { renderBubble({ - unsourced_count: 1, + unverified_count: 0, unsourced_count: 1, flagged_spans: [{ text: 'The cache lives in memory', reason: 'lives in' }], sources: [ { raw: 'ghost.rs:1', kind: 'file', status: 'not_found', detail: 'file not found: ghost.rs' }, @@ -162,7 +162,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { // but the render must not surface a pill on a User/System message. const userMsg = { ...makeAgentMessage({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [{ raw: 'x.rs:9', kind: 'file' as const, status: 'not_found' as const, detail: 'nope' }], fabricated_count: 1, @@ -179,7 +179,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('detail panel surfaces the "verified ≠ true" caveat', () => { renderBubble({ - unsourced_count: 1, + unverified_count: 0, unsourced_count: 1, flagged_spans: [{ text: 'x', reason: 'y' }], sources: [], fabricated_count: 0, @@ -193,7 +193,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { // Green pill ("verified"), shows the source list when expanded. it('renders a verified (green) pill when every cited source resolved', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [ { raw: 'src/foo.rs:14', kind: 'file', status: 'verified', detail: 'file exists' }, @@ -213,7 +213,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { // Option B (2026-05-30): instead of being hidden, they get the NEUTRAL // "unverifiable" pill — never a green chip Kronn didn't earn. renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [ { raw: 'https://example.com/doc', kind: 'url', status: 'unchecked', detail: 'URL — not network-checked' }, @@ -230,7 +230,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { // Priority guard : one bad citation must not be hidden under a green chip // just because another verified. Color encodes the worst signal. renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [ { raw: 'src/ok.rs:1', kind: 'file', status: 'verified', detail: 'file exists' }, @@ -243,7 +243,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('green pill click expands a positive "Verified sources" list', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [ { raw: 'src/foo.rs:14', kind: 'file', status: 'verified', detail: 'file exists' }, @@ -261,7 +261,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { // GREEN pill expanded onto an orange panel that read as "not good". // The drawer must echo the pill's severity so the CSS can colour it green. renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [{ raw: 'src/foo.rs:14', kind: 'file', status: 'verified', detail: 'file exists' }], fabricated_count: 0, @@ -272,7 +272,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('fabricated drawer carries data-severity="fabricated"', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [{ raw: 'ghost.rs:1', kind: 'file', status: 'not_found', detail: 'nope' }], fabricated_count: 1, @@ -293,7 +293,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { describe('severity priority matrix (fabricated > unsourced > verified)', () => { it('all three present → pill AND drawer are fabricated (red)', () => { renderBubble({ - unsourced_count: 4, + unverified_count: 0, unsourced_count: 4, flagged_spans: [{ text: 'claim alpha', reason: 'heuristic' }], sources: [ { raw: 'src/ok.rs:1', kind: 'file', status: 'verified', detail: 'file exists' }, @@ -309,7 +309,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('unsourced + verified (no fabricated) → pill AND drawer are unsourced (amber)', () => { renderBubble({ - unsourced_count: 1, + unverified_count: 0, unsourced_count: 1, flagged_spans: [{ text: 'claim beta', reason: 'heuristic' }], sources: [{ raw: 'src/ok.rs:1', kind: 'file', status: 'verified', detail: 'file exists' }], fabricated_count: 0, @@ -322,7 +322,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('verified only → pill AND drawer are verified (green)', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [{ raw: 'src/ok.rs:1', kind: 'file', status: 'verified', detail: 'file exists' }], fabricated_count: 0, @@ -335,7 +335,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('unsourced count > 0 but fabricated_count 0 with NO verified source → amber (heuristic-only)', () => { renderBubble({ - unsourced_count: 5, + unverified_count: 0, unsourced_count: 5, flagged_spans: [{ text: 'claim gamma', reason: 'heuristic' }], sources: [], fabricated_count: 0, @@ -348,7 +348,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { describe('pill label counts + i18n keys', () => { it('fabricated pill shows fabricated_count (not the source-array length)', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], // 3 bad sources but the backend-supplied count is the source of truth sources: [ @@ -367,7 +367,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('unsourced pill shows unsourced_count + disc.lintUnsourced', () => { renderBubble({ - unsourced_count: 9, + unverified_count: 0, unsourced_count: 9, flagged_spans: [{ text: 'x', reason: 'y' }], sources: [], fabricated_count: 0, @@ -380,7 +380,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('verified pill shows verifiedCount (count of verified sources, not array length)', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [ { raw: 'a.rs:1', kind: 'file', status: 'verified', detail: 'ok' }, @@ -401,7 +401,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { describe('drawer grouping per severity', () => { it('fabricated drawer lists every bad status under lintSourcesTitle, hides verified + unchecked', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [ { raw: 'bad-notfound.rs:1', kind: 'file', status: 'not_found', detail: 'missing' }, @@ -435,7 +435,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('verified drawer lists verified sources under lintVerifiedTitle, hides unchecked', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [ { raw: 'src/foo.rs:14', kind: 'file', status: 'verified', detail: 'file exists' }, @@ -454,7 +454,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('flagged spans listed under lintUnsourcedTitle on an amber drawer', () => { renderBubble({ - unsourced_count: 2, + unverified_count: 0, unsourced_count: 2, flagged_spans: [ { text: 'span one text', reason: 'r1' }, { text: 'span two text', reason: 'r2' }, @@ -471,7 +471,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('caveat is always present in the drawer (fabricated)', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [{ raw: 'ghost.rs:1', kind: 'file', status: 'not_found', detail: 'no' }], fabricated_count: 1, @@ -482,7 +482,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('caveat is always present in the drawer (verified)', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [{ raw: 'ok.rs:1', kind: 'file', status: 'verified', detail: 'exists' }], fabricated_count: 0, @@ -497,7 +497,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('only unchecked sources (url/user/inferred) → NEUTRAL "unverifiable" pill', () => { // Option B: surfaced honestly (not hidden, not green). renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [ { raw: 'https://example.com', kind: 'url', status: 'unchecked', detail: 'url' }, @@ -513,7 +513,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('unchecked sources do NOT suppress an amber pill when a claim is flagged', () => { renderBubble({ - unsourced_count: 1, + unverified_count: 0, unsourced_count: 1, flagged_spans: [{ text: 'flagged claim', reason: 'heuristic' }], sources: [{ raw: 'https://x', kind: 'url', status: 'unchecked', detail: 'url' }], fabricated_count: 0, @@ -526,7 +526,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { describe('mixed amber report (verified source + flagged claim)', () => { it('amber drawer shows the flagged span but does NOT list the verified source', () => { renderBubble({ - unsourced_count: 1, + unverified_count: 0, unsourced_count: 1, flagged_spans: [{ text: 'unsourced claim here', reason: 'heuristic' }], sources: [{ raw: 'src/verified.rs:1', kind: 'file', status: 'verified', detail: 'exists' }], fabricated_count: 0, @@ -550,7 +550,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { badStatuses.forEach(status => { it(`status "${status}" → fabricated (red) pill + listed in drawer`, () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [{ raw: `bad-${status}.rs:1`, kind: 'file', status, detail: `${status} detail` }], fabricated_count: 1, @@ -568,7 +568,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { describe('toggle lifecycle & guards', () => { it('toggles the drawer open → close → open across three clicks', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [{ raw: 'ok.rs:1', kind: 'file', status: 'verified', detail: 'exists' }], fabricated_count: 0, @@ -585,7 +585,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('aria-expanded mirrors the drawer state', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [{ raw: 'ok.rs:1', kind: 'file', status: 'verified', detail: 'exists' }], fabricated_count: 0, @@ -599,7 +599,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('System message never shows a pill even with a fabricated report', () => { const sysMsg = { ...makeAgentMessage({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [{ raw: 'x.rs:9', kind: 'file' as const, status: 'not_found' as const, detail: 'no' }], fabricated_count: 1, @@ -624,7 +624,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { describe('per-item data-status colour hook', () => { it('verified items carry data-status="verified"', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [{ raw: 'ok.rs:1', kind: 'file', status: 'verified', detail: 'exists' }], fabricated_count: 0, @@ -637,7 +637,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('bad items carry their own status code (e.g. out_of_bounds)', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], sources: [ { raw: 'a.rs:1', kind: 'file', status: 'out_of_bounds', detail: 'beyond' }, @@ -679,6 +679,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('unverified ranks BELOW fabricated and unsourced (red/amber win)', () => { renderBubble({ ...unverifiedReport(), fabricated_count: 1 }); expect(screen.getByTestId('lint-pill').getAttribute('data-severity')).toBe('fabricated'); + // unverified_count stays 1 (from the fixture) — the point IS the clash. renderBubble({ ...unverifiedReport(), unsourced_count: 1 }); // second render: query the latest pill — both mounted; assert at least one unsourced expect(screen.getAllByTestId('lint-pill').some(p => p.getAttribute('data-severity') === 'unsourced')).toBe(true); @@ -725,7 +726,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { describe('verified count shown alongside a warning pill', () => { it('unsourced + verified → BOTH the amber warning pill and a green verified pill', () => { renderBubble({ - unsourced_count: 1, + unverified_count: 0, unsourced_count: 1, flagged_spans: [{ text: 'a claim with no source', reason: 'heuristic' }], sources: [ { raw: 'src/a.rs:1', kind: 'file', status: 'verified', detail: 'exists' }, @@ -746,7 +747,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('fabricated + verified → the red pill AND the verified count both show', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], fabricated_count: 1, sources: [ @@ -760,7 +761,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('verified-only report does NOT render a duplicate verified pill (headline already green)', () => { renderBubble({ - unsourced_count: 0, + unverified_count: 0, unsourced_count: 0, flagged_spans: [], fabricated_count: 0, sources: [{ raw: 'src/a.rs:1', kind: 'file', status: 'verified', detail: 'exists' }], @@ -771,7 +772,7 @@ describe('MessageBubble — anti-hallucination lint pill', () => { it('warning with NO verified sources shows no extra pill', () => { renderBubble({ - unsourced_count: 2, + unverified_count: 0, unsourced_count: 2, flagged_spans: [{ text: 'x', reason: 'y' }], sources: [], fabricated_count: 0, diff --git a/frontend/src/components/__tests__/NewDiscussionForm.test.tsx b/frontend/src/components/__tests__/NewDiscussionForm.test.tsx index 39339666..815f5010 100644 --- a/frontend/src/components/__tests__/NewDiscussionForm.test.tsx +++ b/frontend/src/components/__tests__/NewDiscussionForm.test.tsx @@ -24,7 +24,7 @@ const PROJECT_WITH_REPO: Project = { token_override: null, ai_config: { detected: false, configs: [] }, audit_status: 'NoTemplate', - ai_todo_count: 0, + ai_todo_count: 0, tech_debt_count: 0, needs_docs_migration: false, path_exists: true, created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-01T00:00:00Z', }; @@ -36,7 +36,7 @@ const PROJECT_WITHOUT_REPO: Project = { token_override: null, ai_config: { detected: false, configs: [] }, audit_status: 'NoTemplate', - ai_todo_count: 0, + ai_todo_count: 0, tech_debt_count: 0, needs_docs_migration: false, path_exists: true, created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-01T00:00:00Z', }; diff --git a/frontend/src/components/__tests__/ProjectCard.audit-resume.test.tsx b/frontend/src/components/__tests__/ProjectCard.audit-resume.test.tsx index bd4a15d7..fcea5c6e 100644 --- a/frontend/src/components/__tests__/ProjectCard.audit-resume.test.tsx +++ b/frontend/src/components/__tests__/ProjectCard.audit-resume.test.tsx @@ -49,7 +49,7 @@ const PROJECT: Project = { token_override: null, ai_config: { detected: false, configs: [] }, audit_status: 'NoTemplate', - ai_todo_count: 0, + ai_todo_count: 0, tech_debt_count: 0, needs_docs_migration: false, path_exists: true, created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-01T00:00:00Z', }; diff --git a/frontend/src/components/__tests__/ProjectCard.header-a11y.test.tsx b/frontend/src/components/__tests__/ProjectCard.header-a11y.test.tsx index ffc15648..36d8905e 100644 --- a/frontend/src/components/__tests__/ProjectCard.header-a11y.test.tsx +++ b/frontend/src/components/__tests__/ProjectCard.header-a11y.test.tsx @@ -38,7 +38,7 @@ const PROJECT: Project = { token_override: null, ai_config: { detected: false, configs: [] }, audit_status: 'NoTemplate', - ai_todo_count: 0, + ai_todo_count: 0, tech_debt_count: 0, needs_docs_migration: false, path_exists: true, created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-01T00:00:00Z', }; diff --git a/frontend/src/components/__tests__/ProjectCard.migration.test.tsx b/frontend/src/components/__tests__/ProjectCard.migration.test.tsx index 15e6c068..fe01f617 100644 --- a/frontend/src/components/__tests__/ProjectCard.migration.test.tsx +++ b/frontend/src/components/__tests__/ProjectCard.migration.test.tsx @@ -38,7 +38,7 @@ function legacyProject(overrides: Partial = {}): Project { token_override: null, ai_config: { detected: false, configs: [] }, audit_status: 'NoTemplate', - ai_todo_count: 0, + ai_todo_count: 0, tech_debt_count: 0, path_exists: true, needs_docs_migration: true, created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-01T00:00:00Z', diff --git a/frontend/src/components/__tests__/ProjectCard.remap.test.tsx b/frontend/src/components/__tests__/ProjectCard.remap.test.tsx index eaeda32e..228f05d7 100644 --- a/frontend/src/components/__tests__/ProjectCard.remap.test.tsx +++ b/frontend/src/components/__tests__/ProjectCard.remap.test.tsx @@ -36,7 +36,7 @@ function project(overrides: Partial = {}): Project { token_override: null, ai_config: { detected: false, configs: [] }, audit_status: 'NoTemplate', - ai_todo_count: 0, + ai_todo_count: 0, tech_debt_count: 0, needs_docs_migration: false, path_exists: false, created_at: '2026-01-01T00:00:00Z', diff --git a/frontend/src/components/__tests__/ProjectList.missing-path.test.tsx b/frontend/src/components/__tests__/ProjectList.missing-path.test.tsx index 1d24a369..0c9f5524 100644 --- a/frontend/src/components/__tests__/ProjectList.missing-path.test.tsx +++ b/frontend/src/components/__tests__/ProjectList.missing-path.test.tsx @@ -31,7 +31,7 @@ function proj(id: string, name: string, path: string, path_exists?: boolean): Pr id, name, path, repo_url: null, token_override: null, ai_config: { detected: false, configs: [] }, - audit_status: 'NoTemplate', ai_todo_count: 0, + audit_status: 'NoTemplate', ai_todo_count: 0, tech_debt_count: 0, needs_docs_migration: false, path_exists, created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-01T00:00:00Z', } as Project; diff --git a/frontend/src/components/__tests__/TestModeBanner.test.tsx b/frontend/src/components/__tests__/TestModeBanner.test.tsx index 0e3a0b25..c0f9c89a 100644 --- a/frontend/src/components/__tests__/TestModeBanner.test.tsx +++ b/frontend/src/components/__tests__/TestModeBanner.test.tsx @@ -16,7 +16,7 @@ function disc(overrides: Partial = {}): Discussion { return { id: 'd-1', project_id: 'p-1', title: 'Switch theme tokens', agent: 'ClaudeCode' as any, language: 'en', - participants: ['ClaudeCode' as any], messages: [], message_count: 0, non_system_message_count: 0, + participants: ['ClaudeCode' as any], messages: [], message_count: 0, non_system_message_count: 0, tier: "default" as const, summary_strategy: "Auto" as const, introspection_call_count: 0, archived: false, pinned: false, pin_first_message: false, workspace_mode: 'Isolated', worktree_branch: 'kronn/switch-theme', diff --git a/frontend/src/components/settings/AgentsSection.tsx b/frontend/src/components/settings/AgentsSection.tsx index 7cc875c1..cb2f65dc 100644 --- a/frontend/src/components/settings/AgentsSection.tsx +++ b/frontend/src/components/settings/AgentsSection.tsx @@ -739,10 +739,12 @@ export function AgentsSection({ modelsUrl: 'https://docs.anthropic.com/en/docs/about-claude/models', }, codex: { - economy: ['gpt-5-codex-mini', 'gpt-5.1-codex', 'gpt-5-codex'], - default: ['gpt-5.5', 'gpt-5.4', 'gpt-5.1-codex'], - reasoning: ['gpt-5.5', 'gpt-5.4', 'gpt-5.3-codex', 'gpt-5.2-codex', 'gpt-5.1-codex-max'], - fallbackEconomy: 'gpt-5-codex-mini', fallbackDefault: null, fallbackReasoning: 'gpt-5.4', + // 2026-07 refresh: the gpt-5.6 generation (sol = frontier, + // terra = balanced, luna = fast/affordable) + 5.5/5.4 line. + economy: ['gpt-5.6-luna', 'gpt-5.4-mini', 'gpt-5-codex-mini'], + default: ['gpt-5.6-terra', 'gpt-5.5', 'gpt-5.4'], + reasoning: ['gpt-5.6-sol', 'gpt-5.6-terra', 'gpt-5.5', 'gpt-5.4'], + fallbackEconomy: 'gpt-5.6-luna', fallbackDefault: null, fallbackReasoning: 'gpt-5.6-sol', modelsUrl: 'https://developers.openai.com/codex/models', }, gemini_cli: { diff --git a/frontend/src/components/workflows/ApiCallStepCard.tsx b/frontend/src/components/workflows/ApiCallStepCard.tsx index fcd491cd..29185d70 100644 --- a/frontend/src/components/workflows/ApiCallStepCard.tsx +++ b/frontend/src/components/workflows/ApiCallStepCard.tsx @@ -1,7 +1,7 @@ import { useState, useEffect, useMemo, useRef } from 'react'; import { Plug, Play, Loader2, ChevronDown, ChevronRight as ChevRight, KeyRound, Link2, X as XIcon } from 'lucide-react'; import { workflows as workflowsApi } from '../../lib/api'; -import type { AgentType, McpConfigDisplay, McpServer, WorkflowStep, ExtractSpec, StepType, QuickApi } from '../../types/generated'; +import type { AgentType, McpConfigDisplay, McpServer, WorkflowStep, ExtractSpec, StepType, QuickApi, JsonValue } from '../../types/generated'; import { ApiCallAiHelper } from './ApiCallAiHelper'; import { Dropdown } from '../Dropdown'; import { authSlotsForServer, type AuthSlot } from './apiCallAuth'; @@ -842,7 +842,8 @@ function BodyEditor({ label, }: { value: unknown; - onChange: (next: unknown) => void; + // The committed value comes from JSON.parse — structurally a JsonValue. + onChange: (next: JsonValue | null) => void; label: string; }) { // Local draft so the user can type freely without losing focus on diff --git a/frontend/src/components/workflows/QuickApiForm.tsx b/frontend/src/components/workflows/QuickApiForm.tsx index 38f60274..5fc20fd1 100644 --- a/frontend/src/components/workflows/QuickApiForm.tsx +++ b/frontend/src/components/workflows/QuickApiForm.tsx @@ -17,6 +17,7 @@ import type { PromptVariable, QuickApi, WorkflowStep, + JsonValue, } from '../../types/generated'; import { ApiCallStepCard, type ApiPluginOption } from './ApiCallStepCard'; @@ -96,7 +97,7 @@ export function QuickApiForm({ const [apiQuery, setApiQuery] = useState | null>(editApi?.api_query ?? null); const [apiPathParams, setApiPathParams] = useState | null>(editApi?.api_path_params ?? null); const [apiHeaders, setApiHeaders] = useState | null>(editApi?.api_headers ?? null); - const [apiBody, setApiBody] = useState(editApi?.api_body ?? null); + const [apiBody, setApiBody] = useState(editApi?.api_body ?? null); const [apiExtract, setApiExtract] = useState(editApi?.api_extract ?? null); const [apiPagination, setApiPagination] = useState(editApi?.api_pagination ?? null); const [apiTimeoutMs, setApiTimeoutMs] = useState(editApi?.api_timeout_ms ?? null); diff --git a/frontend/src/components/workflows/RunDetail.tsx b/frontend/src/components/workflows/RunDetail.tsx index c4e74538..a76b1f25 100644 --- a/frontend/src/components/workflows/RunDetail.tsx +++ b/frontend/src/components/workflows/RunDetail.tsx @@ -29,6 +29,9 @@ export interface RunDetailProps { onDelete: () => void; /** Cancel a Running workflow run. Button is only rendered for Running status. */ onCancel?: () => void; + /** A2 — resume an Interrupted run. Button is only rendered for Interrupted + * status; the top-level runs only (children resume through their parent). */ + onResume?: () => void; /** 0.7.0 Phase 4 — submit a gate decision (approve / request_changes / reject). */ onDecide?: (payload: DecideRunRequest) => Promise | void; /** 2026-06-13 — jump to a (sub-)workflow's run list, e.g. from a fan-out @@ -677,7 +680,7 @@ function ProducedBranchesPanel({ ); } -export function RunDetail({ run, workflowSteps, onDelete, onCancel, onDecide, onNavigateToWorkflow, onNavigateToRun }: RunDetailProps) { +export function RunDetail({ run, workflowSteps, onDelete, onCancel, onResume, onDecide, onNavigateToWorkflow, onNavigateToRun }: RunDetailProps) { const { t } = useT(); const [expandedStep, setExpandedStep] = useState(null); @@ -778,6 +781,16 @@ export function RunDetail({ run, workflowSteps, onDelete, onCancel, onDecide, on {t('wf.cancelRun')} )} + {run.status === 'Interrupted' && !run.parent_run_id && run.run_type !== 'batch' && onResume && ( + + )}