fix(runtime): recognize all ATX headings in compact summaries - #1
Closed
me2seeks wants to merge 1 commit into
Closed
Conversation
Use one CommonMark-compatible matcher for section transitions and content exclusion so indented or bare heading skeletons cannot receive a trusted section marker. Generated-by: Maka
6 tasks
Owner
|
Thanks for preparing this! It raced with the fix that already landed: d810079 (in apache#3039's merged head, upstream merge 170b313) closed the same gap minutes before the merge — content exclusion via Your single captured Generated-by: Claude Code |
jackwener
pushed a commit
that referenced
this pull request
Aug 22, 2026
…ons (apache#2600) (apache#3001) * feat(runtime): surface worker disconnect as unknown outcome for mutations (apache#2600) A filesystem mutation whose worker fails after dispatch may already have landed on disk, but the host reported it as a generic error. The model had no way to tell "the write may have happened" from "nothing ran", so it treated a half-applied mutation as a clean failure. Close that gap for the disconnect concern: - process-runner tracks a `dispatched` flag (set once Node's 'spawn' event fires and stdin is written) and surfaces it on both the resolved result and the rejection error. - client splits the ambiguous launch failures by that flag: `spawn_failed` (never started, nothing could have been written) vs new `worker_io_incomplete` (ran but the result was lost). `aborted` keeps its reason but carries `dispatched` so a pre-flight cancel is distinct from a post-dispatch kill. Worker-response failures are marked dispatched. - protocol gains an `outcome_unknown` error code so the worker can report "I may have applied this before I lost the ability to answer". - the boundary executor converts a mutating op that fails with a post-dispatch reason into ToolOutcomeUnknownError, which already flows to a structured uncertainOutcome result. Reads and pre-flight failures pass through unchanged. This is the first of four commits addressing apache#2600; it independently closes the "post-dispatch unknown outcomes" concern. Generated-by: ZCode * refactor(runtime): extract filesystem-authority contract (apache#2600) Issue apache#2600 asks for the filesystem-authority contract to live in one place, separate from the individual editing tools. Introduce that module as pure types and a classifier, with no I/O. - FilesystemTargetIdentity: opaque decimal-string dev/ino. String rather than bigint because bigint cannot cross the worker's JSON protocol boundary, and identity is only compared for equality, never used to build a path. - FilesystemTargetDescriptor: a discriminated union so the "no identity to compare" case is an explicit `missing` arm, never an accidentally-absent optional field. A future "skip the identity check" change cannot compile without handling `missing`, which closes the "no identity -> CAS passes" regression class. - FilesystemMutationOutcome: applied | rejected | unknown. - classifyFailedMutationOutcome + UNKNOWN_OUTCOME_REASONS: moved out of filesystem-executor.ts so the executor consumes the contract instead of restating it. The classifier's invariant (every member reason is semantically dispatched, so membership alone suffices; only `aborted` straddles pre-flight/post-dispatch and gates on the flag) is documented at the contract. This is the second of four commits for apache#2600. It is types plus a behaviour- neutral refactor; the descriptor and identity are wired into the worker protocol and the fd-pinned read-modify-write in the following commit. Adds filesystem-authority-contract.test.ts covering the classifier branches and the type-level constraints. Generated-by: ZCode * feat(runtime): capture target identity at lock acquisition for CAS (apache#2600) Close the queue window for issue apache#2600 concern #1: a path replaced while a mutation waits for the write lock must be detected, not silently written. The identity is now captured at lock acquisition (T0) — before the call enters the lock queue — not re-derived inside client.execute after the lock is held (T1). Re-deriving at T1 would sample the post-replacement inode, making the CAS self-fulfilling and re-opening the window. - protocol v6: FilesystemWorkerTargetSchema gains an optional identity {dev, ino} (opaque decimal strings; bigint cannot cross the JSON boundary). superRefine rejects a missing target carrying an identity. - filesystem-executor: writeLockTarget returns {key, canonicalPath}; captureIdentityAtLockAcquisition stats the canonical path at T0; run() receives expectedIdentity and passes it to worker.execute; the FilesystemWorkerExecuteInput carries an expectedIdentity field. - client: deleted captureTargetIdentity (the T1 capture); client.execute uses the caller-supplied expectedIdentity verbatim. - worker assertTargetUnchanged compares the on-disk inode against the T0 identity (follow/entry stat modes match the targetType derivation); a non-missing WRITE target must carry an identity or the request is rejected (reads are exempt — they do not mutate). - post-write orphan check: assertPathStillMatchesIdentity re-stats the path after a write and reports outcome_unknown if the inode no longer matches (the write went to an orphaned inode; the visible file is the replacement). Red-line test verifies the worker receives the T0 identity (before a replacement), proving the capture happens at lock acquisition. Direct unit tests cover the post-write orphan check (match / replaced / disappeared). This is the third of four commits for apache#2600. Generated-by: ZCode * feat(runtime): extend post-write orphan check to edit/format/update (apache#2600) Commit 3 added the post-write identity check to the write operation only. Extend it symmetrically to edit, format_json, and apply_patch update so a path swapped during any read-modify-write is reported as outcome_unknown, not a misleading success. The delete path is already covered by the T0 identity CAS in assertTargetUnchanged (it runs for every operation, uses lstat for the directory-entry semantics that create/delete use, and rejects with path_changed when the inode mismatches). format_json's invalid-JSON branch already returns ok:false without writing. No additional changes needed for either. Adds red-line tests proving a delete and an edit whose target was replaced after authorisation are rejected (path_changed) and the replacement file is left untouched. This is the fourth and final commit for apache#2600. Generated-by: ZCode * test(runtime): pass expected identity in Linux filesystem worker smoke (apache#2600) The T0 identity CAS (b51c6a5) made the worker refuse a write mutation on an existing target that carries no identity. The macOS smoke test was updated to pass one, but the Linux smoke test's Edit call was missed — it runs only on linux+bwrap, so local runs never execute it and only CI's Ubuntu runner hit the refusal. Generated-by: ZCode * test(runtime): make the T0 and missing-target tests protect observable behavior (apache#2600) Two test-quality fixes from review: - The T0 identity test replaced the path inside the worker, i.e. after the lock had been granted — a regression that captured the identity at T1 (post-lock) would still pass, because the replacement happened after any T1 capture point. Rework it to exercise the real queue window: a first mutation blocks inside the worker while holding the path's write lock, a second mutation queues behind it, the path is replaced while the second waits, then the gate releases. Assert the second worker call receives the pre-replacement dev/ino. A T1 capture now samples the replacement's inode and fails the assertion. - The missing-target create test used an empty patch and only asserted the error was not path_changed — it accepted invalid_request, which is exactly what an over-broad mandatory-identity check would throw for missing write targets. Use a valid create patch, assert success, and assert the created file content, so that regression fails loudly. Generated-by: ZCode * feat(runtime): fd-pinned mutation primitive for both backends (apache#2600) The previous T0-identity CAS validated the target and then re-opened the pathname, so a swap between validation and the open could still divert the write onto the replacement — detection after the fact, not prevention. The local/workspace path additionally bypassed the identity authority entirely. Introduce file-stable-write.ts, the fd-pinned mutation primitive both backends now consume, and reconcile the cooperative missing↔existing transitions that previously surfaced as invalid_request: - openStableTarget: open the approved object once — 'r+' with O_NOFOLLOW for existing targets (identity validated by fstat on the descriptor, BEFORE any truncation, so a rejected validation leaves the file intact; write-only targets fall back to O_WRONLY), 'wx' for approved-missing targets (a file that appeared in the gap is path_changed, never truncated). writeThroughHandle truncates and writes at position 0 through the pinned descriptor; ENOSPC/EIO/EDQUOT/EFBIG surface as outcome_unknown — a half-written file is not a clean failure (apache#2600 review P2-2). hostVisibilityAfterWrite describes whether the path still resolves to the pinned inode after the write. - worker: write/edit/format_json/apply_patch-update run read/transform/ write through the pinned handle; format_json's invalid-JSON branch still returns ok:false without writing. - local: LocalWorkspaceExecutor gains readModifyWrite (optional on the workspace interface; remote/isolated workspaces keep the path-based fallback, documented as unprotected). Identity capture at lock acquisition no longer depends on a worker being wired (apache#2600 review P1-2), and apply_patch update resolves the existing-target requirement before any create runs. - client: the missing↔existing transitions while queued are reconciled against the T1 reality — a stale identity on a vanished target is dropped so "delete then rewrite" stays a clean apply, and a write whose target appeared while queued fails with a meaningful path_changed, never invalid_request (apache#2600 review P2-1). The duplicated mutation catch blocks collapse into settleMutationFailure, which also maps the primitive's StableWriteFailure codes. - tests: deterministic race for the pin (swap between validation and the write; bytes land on the original inode, replacement untouched), wx gap-creation rejection, both client transition directions, and the lock-serialisation causal barrier moved onto readModifyWrite. The tautological type-level contract tests are removed. Generated-by: ZCode * feat(runtime): compare-and-delete with tombstone verification (apache#2600) The delete path could remove a replacement: the entry identity was checked and then fs.unlink(path) ran against the pathname, so a rename between the check and the unlink deleted the replacement while the operation reported { ok: true } — a silent unauthorized deletion with no post-operation validation (apache#2600 review: "delete can remove replacement"). POSIX has no atomic compare-and-unlink, so prevention by unlink is impossible; make the capture atomic instead. compareAndDeleteEntry renames the entry to a private unpredictable tombstone in the same directory, verifies the tombstone carries the approved identity (lstat — works for regular files and symlinks alike), and only then unlinks the tombstone. A mismatch means a replacement was installed in the window: it is renamed back to the path — restored, not deleted — and the operation reports path_changed with a message saying so. If the final tombstone unlink fails after a verified match, the entry is preserved at the tombstone and the failure is reported as outcome_unknown, never as success. rename(2) moves the directory entry itself and needs no permission on the file (only write+execute on the parent, exactly like unlink), so deleting read-only/write-only files keeps working. Wired into both backends: the worker's apply_patch delete, and the local executor's applyPatch delete with the approved identity threaded through WorkspaceApplyPatchInput. Tests: plain removal leaves no tombstone; a replacement installed after the check is restored byte-for-byte and reported path_changed; a symlink entry is deleted by its own identity without following the link. Generated-by: ZCode * fix(runtime): no-replace tombstone restore and directory rejection (apache#2600) Two delete-path fixes from review of the compare-and-delete primitive: - The restore could destroy a concurrent newcomer: after the tombstone captured replacement C, another process creating B at the original path meant the rename-based restore atomically overwrote B — the exact class of replacement loss this module exists to prevent. Node exposes no RENAME_NOREPLACE, but link() is natively no-replace: the restore now links the tombstone back and drops the tombstone name, guarding the platform wrinkle where link() follows a symlink source by verifying the restored inode against the tombstone before dropping it. On EEXIST — or any link failure — the tombstone is preserved and the failure reported as outcome_unknown with the location, so nothing is ever lost. - A directory entry was moved into the tombstone and then stranded there: renaming a directory succeeds, but the tombstone unlink cannot (EISDIR/EPERM), so the directory vanished from its path and hid under a stray name where the previous plain unlink simply failed. Directories are now rejected up front, before anything is moved. Also fixes the exact-head Windows sandbox smoke: its pre-existing-file write now supplies the identity captured at lock acquisition (lstat + { dev, ino }), as the boundary executor does in production. Tests: a reoccupied path preserves both the newcomer and the captured entry; a free path restores the captured entry without a tombstone leak; a directory is rejected untouched. Generated-by: ZCode * fix(runtime): type-aware tombstone restore and structured directory refusal (apache#2600) Three delete-path fixes from review of the no-replace restore: - Restore the captured entry itself, not whatever it points at: link() dereferences a symlink source on darwin (implementation-defined per POSIX; verified directly on darwin), so a link-based restore of a captured symlink would plant a regular-file alias of the TARGET at the path — a foreign entry later reads/writes silently edit, while the original link stays hidden on the tombstone. The restore is now type-aware: symlinks are recreated with symlink(readlink(...)) — natively no-replace (EEXIST), round-trips the target string exactly — and only regular files use link(), which cannot misbehave for them. The inode guard stays deliberately hands-off on mismatch: after a concurrent rename-over, the entry at the path may be a foreign one, and unlinking it would destroy third-party data — the exact loss class this module prevents. - Make the directory refusal survive the race this module is about: the lstat pre-check is not an enforcement point, so a directory swapped in between the check and the capture was moved onto the tombstone and stranded there (link() cannot restore a directory; EPERM). Enforcement now lives after the atomic capture: the captured entry's type is checked first, and a directory is renamed straight back before any identity comparison. The post-capture logic is extracted as deleteCapturedTombstone for the forced-directory regression. - Throw the failure type callers understand: the bare directory-refusal Error fell through normalizeOperationError to a generic filesystem_error, so the model never learned what was refused. The refusal is now a StableWriteFailure with its own is_directory code, added to the worker protocol's error enum so the message survives classification on both backends. Tests: a swapped-in symlink is restored as a symlink pointing at its own target — the path is never left holding a foreign regular file; a directory forced onto the tombstone is renamed back, not stranded; the worker surfaces is_directory with the refusal message end to end. Generated-by: ZCode
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
2 participants
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
Close the remaining apache#3039 review gap by recognizing CommonMark ATX headings with 0–3 leading spaces and bare marker runs. One matcher now owns both non-required-H2 transitions and heading content exclusion, so an information-free heading skeleton cannot receive
sections_v1.Verification
npm --workspace @maka/runtime run buildnode --test packages/runtime/dist/__tests__/history-compact-summarizer.test.js— 44 passedgit diff --checkAI use
Tool(s) and scope: OpenAI Codex (Maka) implemented the focused parser fix, regression, simplification audit, and verification under the contributor’s direction.
Final squash trailer:
Generated-by: Maka