Harden remaining parser, configuration, and installer edge cases

[SYKhayyat]

Problem

Remaining parser/configuration/installer edge cases can silently accept invalid input or make failures difficult to diagnose.

Verified scope

• Several config setters accept unknown keys while sibling setters reject them; typos become dead settings.
• Grammar errors have inconsistent line/column context and some valid-looking module names cannot be referenced.
• Safety values such as purge_ratio need domain validation.
• Tool-help probing must be bounded and should not rely on locale-sensitive output.
• Restore/install paths and Windows archive names need consistent atomic-write, dry-run, reserved-name, and path handling.

Proposed fix

Generate or centralize per-setting schemas; reject unknown keys with the exact field and source location; represent grammar spans with line and column; validate numeric safety domains; run external probes under cancellation/timeouts with stable machine-readable checks; and route restore/archive writes through common validated, atomic abstractions.

Acceptance criteria

Add focused negative tests for each invalid key/value/path and platform case. No typo silently widens behavior, no manager probe can block forever, and every refusal identifies the input and safe remediation.

Reference: AUDIT-code-audit-2026-08-23.md, remaining parser, configuration, installer, and platform findings.

[SYKhayyat]

AI agent notes for this issue

Config setters that accept unknown keys silently (audit B9):
These are in ksav/engine/typst/ksav.typ and accept ..args without validation:

• הגדרות_הערות at :1216
• הגדרות_מדורגות at :4953
• הגדרות_מדפים at :5136
• הגדרות_זרמים at :5262
• הגדרות_סימונים at :914

Compare with the setters that DO validate: הגדרות_מספור, הגדרות_טקסט_הערות, _mk_set, ערוץ/אזור. These reject unknown keys with diagnostics.

Fix pattern: The validating setters use _mk_set at ksav.typ:959-980, which checks each argument against a known-key set. Extend this pattern to the five non-validating setters. Each setter has a fixed set of valid keys — enumerate them from the setter's body to build the validation set.

גלישה on הגדרות_זרמים is doubly dead: _sf_spill at :5405 reads moves from region/channel records, not from the settings. The setting compiles but changes nothing. Either wire it through or remove it.

Grammar error context (line/column):

• Grammar errors are reported by the engine's parser in ksav/engine/src/. Check compile.rs and diagnostics.rs for the current error-formatting path.
• The audit says context is inconsistent — some errors have line/column, others don't. The fix is to ensure every grammar error carries a SourceSpan or equivalent.

purge_ratio domain validation:

• This safety value is in the settings/config path. It needs a range check (presumably 0.0–1.0). Find it in the settings definitions and add a guard.

Tool-help probing bounds:

• External tool probing (likely Girsa integration) must have cancellation/timeouts. The runtime.ts module manages process spawning. Ensure probes are wrapped with AbortSignal and a timeout.

Windows archive/install paths:

• save.ts and the installer logic handle file writes. The audit asks for consistent atomic-write, dry-run, reserved-name, and path handling. Check save.ts for the current write pattern and ensure it uses the common atomic abstraction.

Testing: Create focused negative tests — unknown keys, out-of-range values, Windows reserved names. The existing settings.test.mjs and diagnostics.test.mjs are good places to add these.

---

Comment added by Buffy (AI agent) to prepare this issue for automated resolution.

[SYKhayyat]

GitHub issue implementation notes

Prepared 2026-08-27 from the open issue bodies and repository audit. These notes are recommendations only; no issue is closed here.

#2 — Fix remaining rendering and note-layout correctness hazards

Root cause: note destination, reservation, carried placement, and final drawing are resolved in separate Typst paths. They use inconsistent destination aliases and typography/height calculations, while side-stream occupancy is not globally coordinated. Parenthesis handling also relies on a pre-blanked representation that loses quoted-string structure.

Recommended implementation: introduce one typed intermediate note-placement record per note containing destination, stream, height, pin state, source span, and page constraints. Validate ערוץ/אזור against declared destinations before layout. Feed the same typography/config object into both walking and drawing; clamp declared heights before allocation. Use one occupancy allocator for all side streams and carried notes, including pinned notes. Replace ad-hoc parenthesis counting with structural parser spans or a quote-aware scanner.

Regression coverage: fixtures for mixed destinations, undeclared regions, multiple side streams, page carry with pinned notes, declared-height clamping, quoted ) in channel arguments, and footer/page-number collision. Assert each note is drawn exactly once or fails with a source-located diagnostic.

#3 — Make the Ksav UI language switch complete and reliable

Root cause: UI language state is centralized, but rendering is not. The language switch rebuilds only selected shell controls; open panels, generated controls, ARIA/title/placeholder attributes, status/error notices, and CodeMirror search UI can retain old-language strings. UI language and document source vocabulary are also insufficiently separated for English generation.

Recommended implementation: define a typed catalog with exhaustive keys and a test that rejects missing translations. Make every user-visible string resolve at render time. Add a panel-registry localization/rebuild hook and a CodeMirror localization refresh/recreate path. Keep persisted UI language independent from document language. Centralize command/parameter alias mapping, including mark and nested configuration keys, so generated source follows the document language.

Regression coverage: browser test opens every panel, switches both directions, checks visible text plus ARIA/title/placeholder values, verifies status/error text, reloads, and confirms persistence. Add generation tests for English mark, nested dictionary keys, and mixed UI/document-language combinations.

#4 — Preserve git drawer input and improve panel/document workflows

Root cause: Git panel refresh reconstructs the entire panel and creates the commit message/name/email fields from empty defaults, so save/status refresh destroys user input. Panel lifecycle and focus restoration are not modeled as durable state transitions.

Recommended implementation: store Git form state keyed by document/repository/panel instance, update it on input, and hydrate fields during rebuild. Prefer keyed status/file-row updates over body replacement. Extend the panel registry with a common close/escape/scrim/focus contract; capture the originating editor selection and restore it after close. Model footnote insertion as an explicit localized multi-step command with cancellation.

Regression coverage: type a commit message, trigger save/status/language refresh, assert value and selection survive; test name/email too. Test visible close, Escape, scrim dismissal where appropriate, focus return, cancellation, and narrow-window behavior.

#5 — Harden remaining parser, configuration, and installer edge cases

Root cause: configuration setters historically accepted unknown keys; parser diagnostics and destination validation are inconsistent; external probes and filesystem/install paths require common safety policy. The repository currently has no purge_ratio owner/reference, so adding that setting would invent a contract.

Recommended implementation: retain centralized per-setter schemas and reject unknown keys with exact field/source location. Carry source spans through grammar errors. If purge_ratio is introduced by a real owning subsystem, constrain it to its documented domain (normally 0..=1) and test NaN/infinity/boundaries; otherwise do not add dead configuration. Use bounded, cancellable machine-readable tool probes. Centralize validated path handling, Windows reserved-name checks, archive traversal prevention, dry-run behavior, and atomic temp-write/rename semantics.

Regression coverage: unknown keys in every setter, invalid numeric values including NaN/infinity/out-of-range, malformed module names with line/column assertions, quoted-parenthesis parser cases, probe timeout/cancellation, CON/NUL/COM1-style names, .. archive entries, absolute paths, cross-volume rename fallback, and dry-run no-write assertions.

#6 — Audit and bound fire-and-forget UI actions and async failure reporting

Root cause: void async handlers form an implicit error policy. Some failures are surfaced, others are only logged or leave progress/disabled state behind. Cancellation is not consistently distinct from failure.

Recommended implementation: add one approved async-action runner accepting an action label, optional initiating control, cancellation predicate/signal, and localized failure reporter. It must set/clear progress and disabled state in finally, ignore expected cancellation separately, and preserve actionable error context. Route every user-triggered async action through it or await it directly. Add a static prohibition that flags bare fire-and-forget calls outside the wrapper, with narrowly documented infrastructure exceptions.

Regression coverage: document save/export, Git operations, panel refresh, asset operations, spell actions, and interop actions under resolve/reject/cancel paths. Assert notices, no unhandled rejections, and controls/progress restored after every outcome.

#7 — Replace full DOM rebuilds with stable keyed updates

Root cause: panel body, row lists, and preview surfaces replace whole containers in hot paths. This destroys DOM identity and resets scroll/focus/selection even when only one row/page changed.

Recommended implementation: add a framework-free keyed reconciliation utility with stable row/page IDs, model equality short-circuit, create/update/remove/move phases, and preservation of scroll plus focused element identity. Apply it first to panel rows and Git status, then preview pages/windowing. Keep expensive serialization outside keystroke handlers where possible.

Regression coverage: mutate one row in a large list and assert unchanged node identity, scrollTop/scrollLeft, focused control, and selected row remain. Add preview page hash/window tests and a repeat-update benchmark/threshold appropriate for CI.

#8 — Provide a first-class LibreOffice-style document workflow

Root cause: list/table/note operations exist as source-generating commands but are scattered across hydras, menus, and contextual surfaces. Keyboard and caret semantics are not presented as a coherent editor workflow.

Recommended implementation: add discoverable list toolbar/context actions backed by existing list operations; expose create, indent, outdent, move, and list-type changes with bilingual catalog entries and bindings. Specify and implement Enter/Tab/Shift+Tab/Backspace continuation/breakout behavior. Ensure generated markup uses document language, while UI labels use UI language. Add focus/caret restoration around dialogs and generated insertions.

Regression coverage: fresh/nested/mixed lists, continuation and breakout, selection/caret placement, mixed document language, footnote/table insertion, narrow-screen panel focus, and browser journeys from toolbar/menu/shortcut.

#9 — Overhaul UI for intuitive LibreOffice-style editing

Root cause: the shell distributes common actions across contextual bars, menus, palettes, hydras, drawers, and source controls without a stable information architecture. This overlaps #3, #6, #7, and #8.

Recommended implementation: write a short navigation specification first. Establish conventional menu/toolbar groupings, contextual formatting controls, predictable panel lifecycle, shortcut discoverability, and Hebrew/English responsive layouts. Make the spec the contract for changes rather than attempting a wholesale visual rewrite. Coordinate each new action with the async runner, i18n catalog, and keyed rendering utility.

Regression coverage: browser journeys for create/edit/style/list/table/note/language/direction/panel/save/export; bilingual and narrow viewport runs; assert close/escape/outside-click/focus behavior and help/shortcut documentation parity.

#10 / #12 — README binding-count regression / open PR

Root cause: README rewrite removed the generated binding-count claim while the documentation test intentionally checks it against Object.keys(DEFAULT_KEYS).length.

Recommended implementation: retain the source-derived claim or replace the hard-coded count with generated documentation if the project wants to eliminate drift. Current local fix restored all 97 bindings; #12 is a PR duplicating that fix. Leave issue/PR state unchanged per request.

Regression coverage: documentation test computes the count from bindings and checks README/docs claims; add a generation/check command if hard-coded text is retained.

#11 — Adopt Leo editor-style node workflow

Root cause: current notes/sections/panels approximate hierarchy, but there is no durable node identity or many-to-one clone model. Adding a UI outline without a document-level model would create divergence between outline, editor, undo, persistence, and export.

Recommended implementation: stage this behind an opt-in document format/version. First define a node model with stable IDs, headline/body/children, clone references, directives, serialization, undo/redo semantics, and flatten/export rules. Then add outline panel/operations using the existing structure and panel infrastructure. Implement clone propagation through shared IDs rather than duplicated text. Add directive parsing (at-document, at-note, at-shared, at-export) with source spans, then migrate notes compatibly.

Regression coverage: model-only tests for create/rename/move/promote/demote/clone/edit propagation/cycle rejection/undo/persistence; parser tests for directives; export tests; panel tests for focus/scroll/keyed updates. Do not make the legacy format silently reinterpret as nodes.