EPIC-14B: local text-filter commands + command palette - #56
Conversation
New TextFilters SPM target (Foundation only, per epic-14-implementation.md §5): TextFilterCommand, TextFilterError, TextFilterCommandDiscovery, TextFilterLaunchContext, and TextFilterRunner backed by TextFilterProcessSession. TextFilterProcessSession launches with Process.arguments always [] (input travels only via stdin, never shell-interpolated), an explicit working directory (the document's containing folder, else home) and a fixed minimal environment (hard-coded PATH; HOME/TMPDIR; MACDOWN_DOCUMENT_PATH/MACDOWN_SELECTION_LENGTH; nothing inherited from the app's own process environment). stdin is written and stdout/stderr are read concurrently via readability handlers (never sequentially, which would deadlock a script whose output exceeds one pipe buffer); SIGPIPE is ignored process-wide so a script that never reads stdin can't crash the app. A watchdog Task mirrors PDFNavigationDelegate's existing watchdog-vs-continuation race exactly: whichever of normal exit, timeout, or caller cancellation resolves first wins, guarded by a lock so only the first resolution is ever honored. Stdout is capped at Limits.maxOutputBytes (reading stops and the process is terminated on overflow); stderr is separately capped at 64 KiB purely for the error message. TextFilterCommandDiscovery re-scans its directory on every call (no cache to invalidate) and only ever considers executable regular files, never recursing into subfolders. 26 tests covering the happy path, every failure mode in §9's table (launch failure, non-zero exit, timeout, cancellation, oversized output, non-UTF-8 output), the security/trust boundary in §10 (no shell interpolation even for stdin containing shell metacharacters, environment contains only the documented variables, working directory correctness), and the §15 adversarial corpus (a forked grandchild process is never waited on, concurrent invocations of the same script don't corrupt each other's output, running a filter does not block concurrent main-actor work). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
TextFilterCoordinator (mirrors ExportCoordinator's shape) resolves the key window's active editor/document, runs the selected command with the selection as input when non-empty, else the whole document, and applies the output as exactly one undoable edit — the existing public applyDocumentReplacement for the whole-document case, and a small local range replacement using EditorTextSystem's own public textView/undoManager for the selection case (the same breakUndoCoalescing -> insertText -> breakUndoCoalescing idiom EditorCore already uses, without widening its public surface). A stale selection (an incompatible edit happened while the command ran) is clamped to the live text rather than rejected. Cancellation shows no alert (a deliberate withdrawal); every other failure shows one naming the command. TextFilterCommands adds the "Commands" menu: discovered filters (re-scanned on every SwiftUI commands re-evaluation, per TextFilterCommandDiscovery's own no-cache design), "Show Commands Folder," and "Add Example Scripts" — deliberately manual, no auto-install. BundledExampleScripts embeds two example scripts (uppercase, sort lines) as string literals rather than bundled resource files, installed executable and never overwriting a file the user already has. 8 app-level tests cover the pure, extractable logic: range clamping (valid/past-end/negative/empty-document cases) and script installation (executable bit, no-overwrite, directory creation). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CommandPaletteModel is pure row-building/selection logic (query filtering, wraparound selection movement, dispatch-by-kind invocation) kept free of any AppKit/SwiftUI presentation so it is directly testable without presenting UI. AppPaletteCommand is the small, explicit array of app commands (New File, Open, Save, Export, Toggle Sidebar, Show Commands Folder); CommandPaletteRow visibly distinguishes a discovered text filter from a built-in app command (§10) via its kind, both in the row UI and its accessibility label. CommandPaletteView is fully keyboard-operable — type to filter, arrow keys to move the selection, Return to invoke, Escape to dismiss — per §12, with accessibility identifiers/labels on the search field and each row. CommandPalettePanel hosts it in a small floating NSPanel created per invocation; WindowCoordinator.toggleCommandPalette() (⌘⇧P, wired in TextFilterCommands) shows or closes it by scanning NSApp.windows rather than tracking a stored panel reference, keeping WindowCoordinator's own class body under its lint budget. 9 tests cover row filtering/combination/case-insensitivity, wraparound selection movement (including the empty-list no-op case), dispatch to the correct handler by row kind, and that refreshRows() re-scans text filters on every call rather than caching a stale list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds planning/extension-api-design.md (issue #15 deliverable 4): a design-only description of a possible post-1.0 JavaScriptCore third-party extension API — what it could register (a Contributing-shaped transform), the JSContext sandboxing model (no filesystem/network/process access), open distribution/discovery questions, and its relationship to E19/E20/E21. No code; nothing described ships in 1.0. Reconciles epic-14-implementation.md (Slices 5-8 now done, §20 records the concrete implementation decisions where earlier sections were illustrative) and planning/epics/README.md's E14 row (now done, pending on-device journey verification) with what actually shipped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adversarial merge-gate review — 17 validated findingsReviewed head I stopped only after the final full re-read produced no additional independent validated finding. Current result: 2 P0, 5 P1, 6 P2, 4 P3. Per Summary
1. P0 — stale filter results can overwrite/corrupt live editsProof. A deterministic selection example: Whole-document mode is worse: if a slow uppercase filter snapshots This is not an unfamiliar problem in this repo: Impact. Silent authored-text corruption/data loss -> P0 by the release-hardening definition. Concurrent filters can cause the same failure even without a manual edit: both snapshot an old revision, the first changes it, and the later completion applies stale coordinates/output. Remediation. Capture a filter baseline containing at least originating tab/document identity, document mutation generation, editor identity/content revision, snapshot text, selection/range, and any path state that affects launch semantics. After Tests to add. Selection edit during slow filter; whole-document edit during slow filter; external reload/Save-As during filter; close/reopen; two concurrent filters finishing out of order. Every stale case must preserve the live text exactly and produce zero undo mutation. 2. P0 — process exit is treated as I/O completion, so successful output can be truncatedProof from the state machine. Process exit only means the writer has exited; it does not mean every byte already buffered in the pipe has been delivered through asynchronous readability callbacks. I reproduced the exact topology with a Foundation/POSIX harness: direct child writes 1 MiB of zero bytes and exits 0; termination resolves first and the read handler is removed immediately. 24/30 runs captured fewer than the expected 1,048,576 bytes (representative captures: 1,032,192; 1,007,616; 1,040,384). The PR adds an even stronger deterministic discard condition via There is a second race in the same root state machine.
Stderr has the same exit/drain ordering problem for diagnostics. Impact. An exit-0 formatter can silently replace the user's selection/document with a valid UTF-8 prefix of the intended output. That is document corruption, not merely a diagnostic truncation -> P0. Remediation. Do not model direct-process exit as terminal success. Track independent state for process exit status, stdout EOF, stderr EOF, and forced-failure reason. A normal result becomes eligible only after the direct process has exited and both streams are drained to EOF. Enforce the stdout cap atomically before/while appending; once it is exceeded, failure must dominate a later zero exit. Forced timeout/cancel/overflow should initiate a real termination protocol (finding 4), then close/drain deterministically before returning. Tests to add. Repeated large exact-output test (e.g. 1 MiB/4 MiB ASCII with exact byte equality); 3. P1 — Markdown editing assists can transform filter outputProof. Selection replacement calls
Concrete path: select The same class of failure applies to matching characters/delimiters such as Impact. A valid, successful text filter does not get byte/text fidelity through the editor seam. This is a core transform-contract failure. Remediation. Put external/filter replacement behind an Tests to add. App-target tests with Markdown assists enabled where a filter replaces a selection/whole document with each structural one-character output and the final text is exactly stdout, with one undo step. 4. P1 — timeout/cancellation does not guarantee the process has stoppedProof. Apple's own The existing timeout test only asserts that the caller returned before the fixture's five-second sleep; it never records/checks the PID. Therefore it passes even when the command remains alive. The cancellation test has the same blind spot. A TERM-ignoring process that also keeps stdin open can additionally leave the background stdin writer blocked with its input The current adversarial grandchild fixture explicitly codifies another lifetime escape: a background child is allowed to remain alive after the direct shell exits. That may have been an intentional architecture choice, but it conflicts with treating “bounded execution” as a bound on the text-filter process tree rather than merely a bound on how long MacDown waits. Remediation. Implement an explicit shutdown protocol: request graceful termination, await a short bounded grace period, escalate to Tests to add. A TERM-ignoring direct child writes its PID; after timeout/cancellation returns, 5. P1 — filter tasks are unowned, so document/window close does not cancel themProof. Therefore the architecture comment in Repeated invocations are equally ungoverned; there is no per-document serialization/supersession policy, which combines with finding 1 to create out-of-order stale writes. Remediation. Give running filters an explicit owner, preferably the originating document/tab/window controller. Store task/session handles keyed by document identity, cancel them on close/tab removal/eviction, and define whether a new filter cancels, queues behind, or independently coexists with an existing filter. Capture origin context directly rather than rediscovering it from the later key window. Tests to add. Start a long filter, close the originating window, assert task cancellation + process death + zero mutation/alert. Start two filters and assert the chosen ordering/supersession behavior. 6. P1 — command-palette panel has a retain cycle and incorrect lifetime assumptionProof. var panel: CommandPalettePanel?
let created = CommandPalettePanel(coordinator: self) { panel?.close() }
panel = createdThe panel retains its hosting/content view; the SwiftUI root view retains The comment in The toggle then scans Remediation. Make palette ownership explicit. Store an observation-ignored panel reference on the coordinator (or a dedicated palette controller), break ownership on Tests to add. Open -> Escape -> open again; open -> close button -> open again; invoke command -> open again. Repeat 100 times and assert at most one live panel and that a weak reference to each dismissed panel becomes 7. P1 — palette contextual app commands run against the palette itselfProof. The palette is made key via Several standard actions resolve their target from
Impact. The acceptance criterion “command palette invokes app commands” is not met for core app commands; Save is particularly misleading because the palette disappears as though the action succeeded while the dirty document remains unsaved. Remediation. Capture the originating document/window context before presenting the palette and make command actions target that context explicitly. Do not use ambient Tests to add. Through a real 8. P2 — palette semantics and enablement have already drifted from the real commandsProof. New File -> newDocument(addAsTab: true)
New Tab -> newDocument(addAsTab: true)They are identical. The real The palette has no enabled/disabled state at all. It can offer Save/Save As/Close/New File when their real menu equivalents are disabled, and it displays runnable-looking text filters when no active editor exists; choosing one can simply do nothing. Remediation. Stop duplicating command semantics. Define a small shared command descriptor/bridge used by both Tests to add. Assert palette/menu parity for every shared command under: no window, untitled doc, folder-backed doc, dirty/clean doc, multiple tabs, and no active editor. 9. P2 — palette rediscovery turns a specified launch failure into a silent no-opProof. This directly contradicts Additionally, Remediation. Snapshot discovered Tests to add. Discovery returns a command for initial rows and 10. P2 — bounded PATH omits the normal Apple-Silicon Homebrew toolchainProof. Launch context hard-codes: MacDown's own CI is now macOS-26/arm64. Homebrew's documented default prefix on Apple Silicon is This is not made safer by omission in any meaningful capability sense: user filters are already explicitly trusted unsandboxed executables and can invoke Remediation. Keep the non-inherited deterministic PATH, but include known platform prefixes, e.g. Tests to add. Unit-test the frozen PATH policy for arm64/Intel assumptions and document that shell profile/custom PATH entries are intentionally not inherited. 11. P2 — distinct executables can be visually indistinguishableProof.
Impact. A user can be presented with two identical actions backed by different executables and has no reliable way to know which one they are invoking. Remediation. Detect display-name collisions during discovery and disambiguate them with the real filename/extension (or always show the filename as secondary text). Add Tests to add. Colliding filename corpus; assert distinct visible labels and stable ordering across repeated discovery. 12. P2 — the acceptance evidence does not exercise the adapter behavior that mattersProof. Likewise, Finally, CI executes package Remediation. Add real app-target integration tests for the editor mutation/undo/fidelity/stale/lifecycle cases and standard palette actions. Execute the non-UI The new tests should specifically kill every P0/P1 path above, not just re-test helpers. 13. P2 — cancellation behavior diverges from the authoritative product contractProof. Issue #15's mandatory safety contract says launch failure, non-zero exit, timeout, cancellation, or unusable output preserves original text and surfaces a useful error. That may be a reasonable UX decision for a cancellation caused by the user closing a window, but the source-of-truth product contract was not amended; the issue remains open and authoritative. Remediation. Make an explicit owner decision and reconcile all three layers. Prefer distinguishing cancellation reason: an explicit user dismissal/window close may be silent if the product contract is amended to say so; unexpected/system cancellation should surface status. Do not leave the issue, architecture, tests, and implementation asserting different policies. Tests to add. Explicit-user-close cancellation versus programmatic/system cancellation, with the exact agreed visibility behavior asserted. 14. P3 — the manual matrix says empty stdout is an error, while code defines it as deletionProof. PR manual instructions say a command that “produces no output” should preserve original text and show an error. Unix-filter semantics make the implementation choice defensible — an empty transform can intentionally delete the selection/document — but the human verification handoff is currently guaranteed to score the same behavior both pass and fail depending on which source the tester reads. Remediation. Pick one policy and make PR body, architecture, tests, and UI agree. If retaining current semantics, change the manual failure case to invalid UTF-8/oversized output and explicitly test “zero exit + empty stdout deletes selected/whole text as one undoable edit.” 15. P3 — “executable regular file” discovery accepts symlinksProof. A Foundation probe of the predicate shape reports an executable symlink as Remediation. Actually check Tests to add. Executable symlink to a file; symlink to a directory; broken symlink. 16. P3 — stderr can exceed the stated 64 KiB capProof. The code is: if stderrBuffer.count < Self.maxStderrBytes {
stderrBuffer.append(chunk)
}If the buffer is at 65,000 bytes and the next normal read chunk contains 4,096 bytes, the retained buffer becomes 69,096 bytes. The remaining capacity is never calculated. The PR explicitly states stderr is “capped at 64 KiB.” Truncating at arbitrary callback boundaries can also leave a partial UTF-8 sequence, after which the strict Remediation. Append at most Tests to add. >64 KiB ASCII stderr must result in <=65,536 retained UTF-8 bytes; add a multibyte boundary case. 17. P3 — example installation can violate its “never overwrite” promiseProof. guard !fileManager.fileExists(atPath: url.path) else { continue }
try script.contents.write(to: url, atomically: true, encoding: .utf8)Apple documents that atomic NSString/String file writes replace an existing destination. Therefore another process/user creating A second issue in the same installer: Remediation. Create each tiny script with exclusive-create semantics ( Tests to add. Injectable filesystem/exclusive-create failure; chmod failure; concurrent creator race; confirm an existing user file is never replaced. Merge gate / recommended remediation order
After those changes I would re-run the same hostile passes, particularly the process I/O state machine and AppKit palette/window lifecycle, because those remediations will alter architecture rather than merely patch local lines. Current verdict: do not merge |
… 6 P2, 4 P3) Resolves the full finding set from the PR #56 adversarial review (issuecomment-5560464988): P0 (data corruption): stale filter completions could overwrite live edits (baseline generation/revision guard, mirroring performJSONFormatting); process exit was treated as I/O completion, truncating output (independent exit/stdout-EOF/stderr-EOF tracking, atomic oversized-cap decision). P1: filter output could be reinterpreted by Markdown editing assists (new EditorTextSystem.applyExternalReplacement seam); timeout/cancellation didn't confirm the process actually died (SIGTERM-then-SIGKILL escalation with confirmed death); filter tasks were unowned so closing the window didn't cancel them (per-tab task registry on WindowController); the command-palette panel leaked via a retain cycle (explicit coordinator-owned strong reference, weak back-reference); palette commands resolved against the palette itself instead of the originating window (captured origin controller threaded through explicit-target coordinator methods). P2: palette command semantics had drifted from the real menu; palette rediscovery turned a launch failure into a silent no-op; the PATH omitted Apple-Silicon Homebrew; colliding display names were indistinguishable; app-target tests were build-only in CI; cancellation's visibility contradicted the acceptance criterion (reconciled by explicit amendment). P3: the manual test matrix contradicted implemented semantics; discovery accepted executable symlinks; the stderr cap wasn't actually hard; example installation had a check-then-overwrite race. CI now executes MacDown2Tests (not just build-for-testing), serialized. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Remediation complete — all 17 findings resolvedHead P0 (both were real data-corruption paths, fixed architecturally, not patched):
P1: P2/P3: all six P2s and four P3s are fixed as described in the updated PR description — PATH now covers Apple-Silicon Homebrew, colliding names are disambiguated, symlinks are rejected in discovery, the stderr cap is a real hard cap with lossy decoding, example install uses exclusive-create semantics, palette rows reflect real availability and a real discovery snapshot, and CI now actually executes Verification: package suite 1090/1090 (was 1080; +10), full app-target suite 94/94 (was 88; +6 new integration tests directly exercising findings #1/#3/#5/#14), SwiftFormat/SwiftLint --strict clean, and app/CLI/Release builds all green — see the updated PR description for the full command list. CI is currently running against this head. Manual on-device palette/filter keystroke journeys remain unverified per the same non-interactive-session caveat as before (updated matrix in the PR description, including the finding #3/#14 cases to check by hand). Ready for another adversarial pass whenever convenient. |
…port Second independent adversarial pass on 1f46609. Fixes: - #1/#3 (P0/P2): TextFilterTerminalState is a new pure one-shot verdict state machine — .exited commits only on real exit + real stdout EOF + real stderr EOF, never a timer; once committed, permanent. - #2 (P1): TextFilterProcessGroup spawns the command as the atomic leader of its own process group (posix_spawn + POSIX_SPAWN_SETPGROUP) and SIGTERM/SIGKILL-contains the whole group, not just the direct child, confirming emptiness before ever reporting completion. - #4 (P2): palette-invoked Open/Open Folder/New File's post-creation open/Save As now thread an explicit origin window through their full async chain instead of re-resolving NSApp.keyWindow after an await; WorkspaceModel.saveAs(to:) is a new AppKit-agnostic Workspace intent. - #5 (P2): the palette auto-dismisses when its origin window closes, revalidates row availability at invocation time (not just at the last rebuild), and Toggle Sidebar now requires a live origin. - #6 (P2): app + CLI Release builds actually run (previously unrun despite DoD claiming they were green) and are now a durable CI gate. - #7 (P3): a cancelled/superseded filter task no longer surfaces a stale alert, and the runModal() fallback for a gone origin window is removed. - Required Change A: Export… removed entirely from the command palette. See planning/epic-14-implementation.md §22 for the full finding-by-finding record and real command/test-count evidence. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Second adversarial remediationBaseline reviewed: Full finding-by-finding record:
Preserved: every first-pass fix (§21) remains in place and passing; no regressions found in this pass's own verification. Tests added: Verification run on Real counts from the runs actually executed against this head — not copied forward from the first pass's 1090/94. Residual, honestly unverified: the manual UI matrix in the PR description — including the two new items this pass adds (" This PR remains a Draft, pending a human completing that manual matrix and another independent adversarial review pass. |
Third adversarial merge-gate review — 10 validated findingsReviewed head I did not treat the second-remediation checklist as the review boundary. I re-read the new POSIX process machinery, terminal-state model, pipe/FD lifecycle, Darwin process-group semantics, filter/editor integration, palette origin propagation, no-origin behavior, tests, CI, §22, the PR description, issue #15, and the release-hardening contract from scratch. I then repeated focused passes on process success/failure ordering, forced containment, PID/PGID lifetime, concurrent spawn descriptor inheritance, cancellation, palette Save fallback, and evidence integrity until the final full re-read stopped producing independent findings. Result: 10 independent findings — 3 P0, 4 P2, 3 P3. The three P0s block merge. The PR should remain Draft.
1. P0 — forced containment can turn partial stdout into “successful complete output”The second remediation correctly removed fabricated EOF flags:
That means “real EOF” is not sufficient proof of a complete successful transform if MacDown itself caused the EOF by killing a producer that had not finished. A deterministic reproducer is conceptually: #!/bin/sh
(
printf PREFIX
sleep 2
printf SUFFIX
) &
exit 0The direct shell exits 0. The background writer emits This is authored-text corruption/data loss -> P0. Required remediationOnce MacDown decides it must forcibly contain a still-output-owning group because natural drainage did not complete, success must become impossible before any signal is sent. Either:
Do not let signal-induced EOF satisfy the successful-completion predicate. The state model should encode that forced containment disqualifies Regression evidenceAdd a real-process test where a background descendant prints a prefix, waits longer than the post-exit drain threshold, then would print a suffix. The only acceptable result is a failure preserving the source — never successful Also add a pure state test proving 2. P0 — reaping the process-group leader before containment makes later
|
…dge cases Third independent adversarial pass on d40d3ee. Fixes: - #1 (P0): forced containment can no longer launder into success — observeExitAndDrainage() commits .incompleteOutput before it ever contains a still-alive descendant, not after. - #2 (P0): TextFilterProcessGroup no longer reaps the group leader on exit — waitid(WNOWAIT) observes the exit fact while leaving the zombie held, pinning the pid/pgid identity until reapLeader() explicitly releases it once all group-lifetime work is done. groupHasLiveMembers() replaces kill(-pgid,0) (unusable once a zombie leader is held) with real sysctl(KERN_PROC_PGRP) membership enumeration. - #3 (P0): every posix_spawn setup call is checked; a failure throws before spawning on a partially-configured process. - #4 (P2): POSIX_SPAWN_CLOEXEC_DEFAULT replaces the parent-side fcntl(FD_CLOEXEC) loop, closing the concurrent-spawn FD-inheritance window atomically. - #5 (P2): palette Save no longer falls through to WorkspaceModel's ambient saveAs() for an untitled/unavailable-backed document — requiresDestinationToSave + saveDocumentFromExplicitOrigin() route it through the explicit-origin destination flow instead. - #6 (P2): New Tab/Open/Open Folder are hidden, not just no-ops, when the palette has no live document origin. - #7 (P2): containment is honestly documented and tested as scoped to the initial process group, not "the whole process tree" — a descendant that calls setsid() is explicitly outside the contract. - #8 (P3): removed the process-wide signal(SIGPIPE, SIG_IGN) mutation in favor of descriptor-scoped F_SETNOSIGPIPE on just the parent's own stdin-write pipe. - #9 (P3): fixed a test that recorded $$ inside a subshell and so verified the wrong process's pid. - #10 (P3): amended issue #15's cancellation wording to match the shipped silent-withdrawal behavior. See planning/epic-14-implementation.md §23 for the full finding-by-finding record and real command/test-count evidence. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Third adversarial remediationBaseline reviewed: Full finding-by-finding record:
Preserved: every fix from §21/§22 remains in place and passing. Tests added: A deliberate scope decision, stated plainly: finding #3's suggested full syscall-mocking seam for fault injection was assessed and not built — most of the ~10 POSIX calls essentially never fail outside OOM; two realistic failure paths (nonexistent working directory, closed source fd) are covered with real, deterministically-triggerable failures instead. See §23 for the reasoning. Verification run on Real counts from the runs actually executed against this head. Residual, honestly unverified: the manual UI matrix in the PR description — including the two new items this pass adds (palette Save's destination panel for an untitled/unavailable-backed document from origin A; New Tab/Open/Open Folder hidden with no document origin) — was not driven interactively. Not inferred passed. This PR remains a Draft. This pass's own review explicitly called for another independent hostile pass afterward, not only a re-check of these ten items — that has not happened yet. Zero findings is not being claimed here. |
|
CI note: the first |
1 participant
What this changes
Implements the remaining scope of issue #15 (EPIC-14): local user text-filter commands and their command-palette/Commands-menu integration ("E14B" — Slices 5-8 of
planning/epic-14-implementation.md). #54 already shipped the contribution SPI,TOCContribution, and Preview/Export integration (Slices 1-4); #55 reconciled the architecture doc between the two. This PR is the implementation.Update (first remediation): an adversarial review of the initial implementation (head
aacf824) found 17 contract defects (2 P0 data-corruption paths, 5 P1, 6 P2, 4 P3) — the consolidated review. All 17 were remediated at head1f46609; seeplanning/epic-14-implementation.md§21.Update (second remediation): an independent second pass on
1f46609found the first remediation's own process/palette fixes were incomplete (7 findings — 1 P0, 3 P2, 1 P3, plus a required change), all remediated at headd40d3ee; see §22.Update (third remediation): an independent third pass — re-reading the whole process/palette implementation from scratch, not only re-checking §22's seven items — found 10 further findings (3 P0, 4 P2, 3 P3) in the second pass's own new process machinery and in palette edge cases §22 did not cover. All ten are remediated in this branch; see §23 for the finding-by-finding record. The description below reflects that state. §21/§22 stay as their own passes' historical record and are superseded wherever this description disagrees with them — most notably: process-group emptiness is no longer checked via
kill(-pgid, 0), the group leader is deliberately never reaped until this session is completely done with it, everyposix_spawnsetup call is checked, and "process containment" is now honestly scoped to the invocation's initial process group, not "the whole process tree."How it works
TextFilters(SPM target,Foundationonly):TextFilterCommand,TextFilterError,TextFilterCommandDiscovery,TextFilterLaunchContext, andTextFilterRunnerbacked byTextFilterProcessSession/TextFilterProcessGroup/TextFilterTerminalState.Process.argumentsis always[]; input travels solely via stdin, never shell-interpolated.PATHcovering both Apple-Silicon and Intel Homebrew prefixes;HOME/TMPDIR;MACDOWN_DOCUMENT_PATH/MACDOWN_SELECTION_LENGTH) — nothing inherited from the app's own process environment.TextFilterTerminalStatecommits.exitedonly when child-exit, stdout-EOF, and stderr-EOF have all three actually been observed, and once any verdict commits it is permanent. Critically,TextFilterProcessSession.observeExitAndDrainage()now commits a fail-closed.incompleteOutputverdict before it ever contains a still-alive descendant — containing is what closes a remaining writer's fd and produces "real" EOF, and that EOF must never be able to retroactively turn a deliberately-killed partial stream into success. (The second pass had already eliminated a timer fabricating EOF; this pass closes the remaining ordering gap where containment's own side effect could still satisfy the old check.)waitid(..., WNOWAIT), deliberately without reaping it — a reaped pid becomes immediately eligible for kernel reuse, and this invocation addresses the whole group by that same pid/pgid number for as long as containment might still need to signal it.reapLeader()is a separate, explicit, idempotent step called only once every group-lifetime operation is complete. Becausekill(-pgid, 0)cannot tell a deliberately-held zombie leader from a real live member, group-emptiness is now checked by enumerating real membership viasysctl(KERN_PROC_PGRP)and inspecting each member's actual process state.TextFilterProcessGroup.spawn()— file-actions/attributes setup, bothstrdupallocations,posix_spawnitself — is checked; a failure anywhere throws with which step failed, before launching on a partially-configured process.POSIX_SPAWN_CLOEXEC_DEFAULTreplaces the previous parent-sidefcntl(FD_CLOEXEC)loop, closing every non-stdio descriptor in the child atomically as part of the spawn itself — no window remains during which a concurrently-spawning session's ownfork()could inherit this session's pipes.SIGTERM→ bounded grace →SIGKILLthe invocation's initial process group and confirm it empty before ever reporting completion (TextFilterError.terminationUnconfirmedif it can't be confirmed) — a real, non-trivial guarantee that defeats a plain backgrounded job or an outlived pipeline stage. It is not "every descendant however it regroups itself": a descendant that callssetsid()to leave the group is explicitly, and now testably, outside this contract.Limits.maxOutputBytes(4 MB default, enforced atomically). Stderr is hard-capped at 64 KiB, lossily decoded so a truncated multibyte boundary doesn't discard the whole diagnostic.SIGPIPEmutation (third-pass finding [EPIC-07] Native Markdown preview: Textual rendering, split view, scroll sync #8):signal(SIGPIPE, SIG_IGN)is gone; Darwin's descriptor-scopedF_SETNOSIGPIPEis applied to just the one pipe descriptor that can legitimately hitEPIPE.TextFilterCommandDiscoveryre-scans its directory on every call, considers only executable regular files, and disambiguates colliding display names with the real filename.TextFilterCoordinator(App target) runs a command against a document's active editor: the selection when non-empty, else the whole document. Stale-completion rejection via a captured baseline; editing-assist-fidelity viaEditorTextSystem.applyExternalReplacement; per-tab task ownership with window-close cancellation and same-tab supersession; no stale-alert on a cancelled/superseded task, and norunModal()fallback for a gone origin window.CommandPaletteModel+CommandPaletteView+CommandPalettePanel): a small floating panel, fully keyboard-operable, combining app commands with discovered text filters.WindowCoordinator; auto-dismisses when its origin window closes; row availability (including text-filter availability) is re-checked live at invocation time, not frozen at open time.NSApp.keyWindow— including, as of this pass, Save's own destination fallback (finding [EPIC-04] EditorCore: NSTextView + TextKit 2 representable, performance baseline #5:WorkspaceModel.requiresDestinationToSaveletsWindowController.saveDocumentFromExplicitOrigin()route an untitled/unavailable-backed document to the explicit-origin destination flow instead ofsave()'s own ambient one) and the no-document-origin case (finding [EPIC-05] Tree-sitter highlighting engine + theme system #6: New Tab/Open…/Open Folder… are now hidden, not merely no-ops, when the palette has no live document window as its origin — each previously fell back toNSApp.keyWindow, the palette panel itself, deeper in its own async chain).Export…is not a palette command at all (removed, not given the same treatment).planning/extension-api-design.md(issue [EPIC-14] Extension points: first-party contribution seam + user text-filter commands #15 deliverable 4): design-only; nothing in it ships in 1.0.Residual limitations (tracked, not silently dropped)
setsid()/setpgid()to leave its process group is outside this feature's containment contract by explicit design (third-pass finding [EPIC-06] Markdown engine: swift-markdown parse actor, debounce, front matter, source-range index #7) — not a gap being tracked for a future fix, a deliberately accepted boundary for trusted, user-installed local scripts.Commands run and observed outcomes (third remediation, current head)
-enableCodeCoverage NOis required forbuild-for-testing/test-without-buildingon this toolchain (Xcode's test action instruments this repo's pure-C SwiftPM package targets with-fprofile-instr-generateregardless of the scheme's own coverage setting, and the profiling runtime never links into them — no coverage report is consumed anywhere in this pipeline).-parallel-testing-enabled NOremains required for this suite's load-sensitive tests.A deliberate scope decision from this pass, stated plainly: finding #3 (checked spawn setup) suggested a full syscall-mocking seam for fault injection. That was assessed and not built — most of the ~10 individual POSIX calls essentially never fail outside OOM, and two of the realistic failure paths (a nonexistent working directory, a closed source file descriptor) are covered with real, deterministically-triggerable failures instead, without mocking anything. See §23 for the reasoning in full.
Manual matrix — unverified
epic-14-implementation.md§14 names two on-device journeys this PR cannot execute interactively in a non-interactive session: the palette keystroke path (⌘⇧P, type to filter, arrow/Return to invoke) and the text-filter replacement keystroke-to-undo-step path (select text, run a command, ⌘Z to undo). Do not infer either passed from the unit-test evidence above — a human (or an interactive session) should run through:Export…is absent from the palette; the normal app Export menu item still works unchanged.*/`/underscore — confirm it lands verbatim, not expanded into a Markdown span.setsid()/setpgid()to leave the process group, which is explicitly out of scope (third-pass finding [EPIC-06] Markdown engine: swift-markdown parse actor, debounce, front matter, source-range index #7).Risk and rollback
TextFiltersSPM target (Foundationonly) and new App-target files, plus explicit-target methods onWindowCoordinatorthat the real menu commands now delegate to unchanged.epic-14-implementation.md§10. Coverage:TextFiltersTests/TextFilterProcessGroupTests/TextFilterRunnerAdversarialTests(process/lifecycle/termination, including a real PID-based process-group containment matrix and process-group-identity-safety ordering tests), App-targetTextFilterCoordinatorTests, and App-targetPaletteOriginTargetingTests/CommandPaletteStaleOriginTestsfor the palette-targeting fixes across all three passes.git revertof this PR's commits.Links
1f46609)d40d3ee)planning/epic-14-implementation.md§21/§22 (first/second remediations, historical), §23 (third remediation, current), §20 (initial implementation record), §9 (cancellation-visibility amendment), §6.5/§7.2/§9/§10/§17 Slices 5-8 (binding scope)This PR remains a Draft pending human execution of the manual matrix above, and another independent adversarial pass — nothing in this description or in CI substitutes for either.
🤖 Generated with Claude Code