Resolve few fixes - Increase stability#115
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Hardening-focused PR that improves Kronn’s operational reliability (timeouts, crash recovery, concurrency correctness) and makes several previously silent UI/backend failures visible to users/operators via toasts and stronger logging.
Changes:
- Frontend workflow UX hardening: abort/launch-generation guards for SSE streaming, out-of-order detail fetch protection, visible trigger error toasts, and launch-variable pattern validation with new regression tests.
- Frontend settings/MCP/chat UX: central “action failed” toast path + optimistic-state reverts (with tests) and additional error toasts for MCP and auto-skill activation.
- Backend reliability/security posture: bounded network/process operations, cron trigger window correctness, Cancelled stickiness, boot “Interrupted” notifications, safer config/secret sync writes, and more robust DB handling (poison/panic containment) with expanded tests/docs/changelog updates.
Reviewed changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/pages/WorkflowsPage.tsx | Prevents zombie SSE updates across launches/unmount; adds ordering guards and improved error surfacing in workflow launch flow. |
| frontend/src/pages/WorkflowsPage.css | Improves branch-map hover hit-target behavior via group-based hover + invisible hit path styling. |
| frontend/src/pages/SettingsPage.tsx | Adds shared toast failure handler and optimistic-update reverts for settings mutations; wires toast into DebugSection. |
| frontend/src/pages/McpPage.tsx | Adds user-visible error toasts for failed MCP config mutations. |
| frontend/src/pages/tests/WorkflowsPage.test.tsx | Adds regressions for pattern validation before modal close and trigger failure toast visibility. |
| frontend/src/pages/tests/SettingsPage.test.tsx | Adds regression ensuring max-agents slider reverts + toasts on failed save. |
| frontend/src/lib/i18n.ts | Adds new i18n keys for surfaced errors and workflow modal validation messages. |
| frontend/src/components/workflows/StepBranchMap.tsx | Adds a “fat” invisible SVG hit target to make arc hover/tooltip usable. |
| frontend/src/components/settings/ProfilesSection.tsx | Surfaces profile mutation failures via toast with extracted user-facing error. |
| frontend/src/components/settings/IdentitySection.tsx | Surfaces global-context save failures via toast; keeps dirty state so retries can happen. |
| frontend/src/components/settings/DebugSection.tsx | Makes debug-mode toggle revert + toast on failed persistence; adds toast prop. |
| frontend/src/components/settings/AgentsSection.tsx | Surfaces agent-access and token/key mutation failures via toast. |
| frontend/src/components/settings/tests/DebugSection.test.tsx | Pins revert+toast behavior on debug-mode save failure. |
| frontend/src/components/ChatInput.tsx | Warns user when auto-skill activation fails but message still sends. |
| frontend/scripts/check-types-drift.mjs | Improves type-drift coverage for unions/quoted keys and reports honest comparison coverage. |
| frontend/package.json | Prefixes pnpm test:e2e with Playwright Chromium shell install to avoid missing-browser failures. |
| frontend/e2e/README.md | Documents Playwright browser revision pitfalls and local remediation steps. |
| docs/tech-debt/TD-20260709-nonisolated-overlapping-runs.md | Adds tech-debt writeup on overlapping runs in non-isolated mode and mitigation options. |
| docker-compose.yml | Adds explicit KRONN_IN_DOCKER=1 container marker. |
| CHANGELOG.md | Documents the hardening pass fixes and their operational impact. |
| backend/src/workflows/workspace.rs | Bounds workspace hook execution with timeout + kill-on-drop semantics. |
| backend/src/workflows/trigger.rs | Fixes cron evaluation to fire exactly-once within (since, now] windows; adds deterministic tests. |
| backend/src/workflows/tracker/github.rs | Adds reqwest client timeouts to avoid engine tick stalls. |
| backend/src/workflows/template.rs | Fixes envelope status coercion so non-string status doesn’t silently become OK. |
| backend/src/workflows/steps.rs | Improves repair/escalation logging, honors declared NO_RESULTS action, and applies output cleaning unconditionally. |
| backend/src/workflows/runner.rs | Adds cancel-aware/robust status writes, logs hook failures, adds rollback cancel checks, and improves DB error visibility. |
| backend/src/workflows/mod.rs | Introduces tick-window tracking and makes concurrency check+insert atomic in a single DB closure. |
| backend/src/workflows/batch_step.rs | Recovers child batch terminal state from DB after WS lag to avoid false 2h timeouts. |
| backend/src/main.rs | Aligns auth-token behavior, improves docker detection, notifies boot-interrupted runs, and logs config-save failures accurately. |
| backend/src/db/workflows.rs | Returns reconciled run info for boot notifications; adds Cancelled stickiness; loud JSON corruption fallbacks; 404 signal on concurrent delete. |
| backend/src/db/tests.rs | Updates reconcile tests and adds Cancelled-stickiness regression coverage. |
| backend/src/db/mod.rs | Stores boot-interrupted runs for notification; recovers poisoned DB mutex and catches panics inside with_conn. |
| backend/src/db/learnings.rs | Logs unknown persisted learning labels to avoid silently rewriting human decisions. |
| backend/src/core/ws_client.rs | Bounds WS handshake to prevent stuck reconnect tasks. |
| backend/src/core/worktree.rs | Canonicalizes path equality to make “branch checked out in main repo” guard reliable (macOS symlink/canonical path). |
| backend/src/core/scanner.rs | Serializes env-mutating test to avoid cross-test flakiness. |
| backend/src/core/run_notify.rs | Fixes URL resolution precedence and adds boot-interrupted webhook emission path. |
| backend/src/core/rtk_detect.rs | Updates docker detection test to use new explicit marker. |
| backend/src/core/net_expose.rs | Corrects LAN boot guard to require actually-enforced auth (enabled + token). |
| backend/src/core/mcp_scanner.rs | Makes MCP env decryption strict to prevent secret-clobbering on disk; expands host-sync tests. |
| backend/src/core/kronn_state.rs | Prevents clobbering corrupt state files by distinguishing missing vs unreadable/corrupt on mutation paths. |
| backend/src/core/kronn_state_test.rs | Adds regressions ensuring mutators/backfill don’t overwrite corrupt state. |
| backend/src/core/key_discovery.rs | Preserves/avoids clobbering corrupt Gemini settings and improves logging on removal failures; adds tests. |
| backend/src/core/env.rs | Replaces data-dir docker detection with explicit marker + runtime markers; updates tests accordingly. |
| backend/src/core/config.rs | Uses atomic persist path during migration re-save; updates docker marker behavior in tests. |
| backend/src/core/backup.rs | Improves prune logging, fixes backup copy behavior to avoid DB-wide stalls/panics, adds skip-on-restart-loop heuristics + tests. |
| backend/src/api/workflows.rs | Returns 404 on concurrent workflow delete, expands cancellable statuses, adds notify on gate-resume failures, improves DB error mapping. |
| backend/src/api/setup.rs | Preserves Codex OAuth fields by merging auth.json; makes imported-key persistence failures visible to the caller; fixes backup copy semantics. |
| backend/src/api/projects/clone.rs | Adds git low-speed abort envs to avoid indefinite blocking clones. |
| backend/src/api/mcp_remote.rs | Excludes Interrupted from duration averaging; ensures failures notify for MCP-triggered runs. |
| backend/src/api/git_ops.rs | Prevents interactive git prompts and adds low-speed abort envs for pushes. |
| backend/src/api/docs.rs | Bounds sidecar proxy request timeouts. |
| backend/src/api/discover.rs | Adds bounded reqwest client for repo discovery pagination. |
| backend/src/api/disc_git.rs | Makes test-mode state persistence failures roll back and return an error; logs DB cleanup failures. |
| backend/src/agents/runner.rs | Distinguishes stdout/stderr read errors from EOF; bounds Kiro installer curl; improves Ollama ctx-cap parsing and /api/show caching semantics; surfaces Ollama stream/in-band errors. |
| backend/src/agents/runner_test.rs | Adds/updates tests for Ollama stream parsing and in-band error surfacing. |
| backend/Dockerfile | Adds explicit KRONN_IN_DOCKER=1 marker for correct runtime posture detection. |
Comment on lines
+2252
to
+2256
| const badPattern = vars.find(v => { | ||
| const val = (launchingWorkflow.values[v.name] ?? '').trim(); | ||
| if (!v.pattern || !val) return false; | ||
| try { return !new RegExp(`^(?:${v.pattern})$`).test(val); } | ||
| catch { return false; } // invalid regex → let the backend decide |
Comment on lines
641
to
645
| onClick={async () => { | ||
| const updated = scanPaths.filter((_, j) => j !== i); | ||
| setScanPaths(updated); | ||
| try { await configApi.setScanPaths(updated); } catch (err) { console.warn('Settings action failed:', err); } | ||
| try { await configApi.setScanPaths(updated); } catch (err) { setScanPaths(scanPaths); toastActionFailed(err); } | ||
| }} |
Comment on lines
+662
to
+665
| const updated = [...scanPaths, newScanPath.trim()]; | ||
| setScanPaths(updated); | ||
| setNewScanPath(''); | ||
| try { await configApi.setScanPaths(updated); } catch (err) { console.warn('Settings action failed:', err); } | ||
| try { await configApi.setScanPaths(updated); } catch (err) { setScanPaths(scanPaths); toastActionFailed(err); } |
Comment on lines
+676
to
+679
| const updated = [...scanPaths, newScanPath.trim()]; | ||
| setScanPaths(updated); | ||
| setNewScanPath(''); | ||
| try { await configApi.setScanPaths(updated); } catch (err) { console.warn('Settings action failed:', err); } | ||
| try { await configApi.setScanPaths(updated); } catch (err) { setScanPaths(scanPaths); toastActionFailed(err); } |
Comment on lines
+706
to
+709
| onClick={async () => { | ||
| const updated = scanIgnore.filter((_, j) => j !== i); | ||
| setScanIgnore(updated); | ||
| try { await configApi.setScanIgnore(updated); } catch (err) { console.warn('Settings action failed:', err); } | ||
| try { await configApi.setScanIgnore(updated); } catch (err) { setScanIgnore(scanIgnore); toastActionFailed(err); } |
Comment on lines
+727
to
+730
| const updated = [...scanIgnore, newIgnorePattern.trim()]; | ||
| setScanIgnore(updated); | ||
| setNewIgnorePattern(''); | ||
| try { await configApi.setScanIgnore(updated); } catch (err) { console.warn('Settings action failed:', err); } | ||
| try { await configApi.setScanIgnore(updated); } catch (err) { setScanIgnore(scanIgnore); toastActionFailed(err); } |
Comment on lines
+740
to
+743
| const updated = [...scanIgnore, newIgnorePattern.trim()]; | ||
| setScanIgnore(updated); | ||
| setNewIgnorePattern(''); | ||
| try { await configApi.setScanIgnore(updated); } catch (err) { console.warn('Settings action failed:', err); } | ||
| try { await configApi.setScanIgnore(updated); } catch (err) { setScanIgnore(scanIgnore); toastActionFailed(err); } |
| 'config.konamiReveal': '🎮 Mode secret activé', | ||
| 'config.batmanRecruited': '🦇 Batman rejoint la Batcave', | ||
| 'skills.autoActivated': '⚡ Skill « {0} » activé automatiquement', | ||
| 'skills.autoActivateFailed': "Impossible d'activer automatiquement le skill « {0} » — message envoyé sans", |
Comment on lines
+1012
to
+1016
| // The workflow existed when we loaded it above but was deleted | ||
| // concurrently before the UPDATE landed → 404, not a fake success. | ||
| Err(e) if e.to_string().contains("workflow not found") => { | ||
| Json(ApiResponse::err_coded(ApiErrorCode::NotFound, "Workflow not found")) | ||
| } |
ae41d9d to
a480c24
Compare
Comment on lines
+191
to
+197
| let conn = match conn.lock() { | ||
| Ok(g) => g, | ||
| Err(poisoned) => { | ||
| tracing::error!("DB mutex was poisoned by a previous panic — recovering the lock"); | ||
| poisoned.into_inner() | ||
| } | ||
| }; |
Comment on lines
+278
to
+288
| // Abort the live-run SSE reader on unmount. Without this the fetch loop | ||
| // survives tab switches for the whole run duration (hours on long agent | ||
| // steps): dead setLiveRun calls, a refetch against an unmounted instance, | ||
| // and one zombie connection per launch eating the browser's per-host pool. | ||
| // Aborting only stops WATCHING — the backend run itself is unaffected. | ||
| useEffect(() => () => { abortRef.current?.abort(); }, []); | ||
| // Launch generation: abort() can't cancel run A's already-scheduled RAF or | ||
| // a resolved reader.read() microtask — those late callbacks would write | ||
| // A's text/status into run B's freshly-reset liveRun. Every stream | ||
| // callback checks it still belongs to the CURRENT launch. | ||
| const launchGenRef = useRef(0); |
a480c24 to
bf4ee78
Compare
Comment on lines
15
to
19
| "test": "vitest run", | ||
| "test:watch": "vitest", | ||
| "test:coverage": "vitest run --coverage", | ||
| "test:e2e": "playwright test", | ||
| "test:e2e": "playwright install chromium --only-shell && playwright test", | ||
| "test:e2e:ui": "playwright test --ui", |
bf4ee78 to
677659c
Compare
Signed-off-by: Romuald Priol <romuald.priol@protonmail.com>
677659c to
6d7e2ea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes all the things 🎉
Changes