Skip to content

Coordinate JSON config writes and reject stale undo - #93

Closed
teamleaderleo wants to merge 5 commits into
tact-84-config-prerequisitesfrom
tact-84-cooperative-config
Closed

teamleaderleo wants to merge 5 commits into
tact-84-config-prerequisitesfrom
tact-84-cooperative-config

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 20, 2026 •

Copy link
Copy Markdown
Owner

Summary

A helper paused after reading cmux.json can overwrite a completed native Settings edit when it resumes. Two production-code fixtures reproduce lost same-key and disjoint-key edits. This change puts native/helper mutations inside one nonblocking cooperative writer boundary and adds single-path conditional undo: a newer user choice causes a conflict instead of restoration.

  • Stable resolved-target flock sidecar, fresh reads, source/target/inode rechecks, existing lossless editor and atomic publication. Busy writers return explicit conflicts.
  • Native in-memory receipts and private helper receipts (--receipt, undo); preview/revision preconditions; unset and explicit defaults remain distinct. No whole-file restore.
  • Existing Computer Use Settings models use the fully validated receipt path; helper mutations use the same canonical Q validator. Legacy native set/reset retain their syntax-only validation contract. Q currently omits the real app.devWindowDisplay catalog key, so this PR deliberately does not claim universal native semantic enforcement.
  • Localized recovery guidance, persisted-versus-runtime-unobserved output, and the bounded contract in docs/config-transactions.md.

Tracks teamleaderleo/Tact#84. This is a first slice, not issue closure.

Prerequisites / review base

This focused fork PR targets tact-84-config-prerequisites at e531059e2bfac5a0b4b6bf35c16e5796c350fab3, composed from initial upstream main b334a7deeb0ff65bc9ee4d1c6a2bf7d454795110 plus:

Both remain open and were rechecked after integrating the P repair. The composition resolves their helper conflict by validating the parsed lossless candidate before publication. Promote/rebase the two transaction commits onto upstream after those prerequisites settle; do not merge the prerequisite composition as an unrelated upstream transaction patch.

Exact guarantee and boundary

Participating native stores/helper writers to one resolved target preserve disjoint edits or return a conflict. Conditional undo compares current raw value and target with the installed result while holding the same lock. Arbitrary editors, older/nonparticipating writers, hard-link aliases, and non-equivalent network filesystem locks are outside this guarantee. A preflight check cannot close the external-editor race before rename. Value-based undo intentionally does not detect ABA. Existing native first-duplicate/Python last-duplicate policies remain unchanged; the shared ordinary-setting flow assumes unique keys. Disk persistence is not runtime application.

Testing

Current composition: c6c25b98e02ede0907ec9e730abdff1325ea959c. The P successor adds its native cache regression and fixes Python removal across duplicate ancestors. The native merge retained #84’s equivalent persistedRoot implementation. Python tests were executed again after integration. The 39 native and 4 model results below are historical working-tree executions before this integration, not new-head certification; the newly added P native regression has not been executed locally.

  • Red commit: da2ff76a1b712f2d6fa6e8944d48e782923f320a; swift test --package-path Packages/macOS/CmuxSettings --filter JSONConfigTransactionTests produced exactly two lost-edit failures on the original P/Q composition (before the P repair). Cached sequential stores and external-before-watcher invalidation already passed.
  • Fix: 9775b1a036a23c64390e4e54edc13359bf21584e.
  • swift test --package-path Packages/macOS/CmuxSettings --filter 'JSONConfig(Store|Transaction)': 39 tests pass, including original JSONC/encoding/BOM/duplicate/symlink/malformed cases and new stale undo, pin/inheritance, and semantic rejection cases.
  • swift test --package-path Packages/macOS/CmuxSettingsUI --filter JSONValueModel: 4 tests pass, including the actual Settings model preserving a user choice against undo and surfacing full-candidate rejection.
  • python3 -m unittest discover -s tests -p 'test_cmux_settings_*.py': 25 tests pass on the updated composition. Controlled validation seams isolate the interleaving and generic lossless fixtures; these tests do not claim canonical semantic coverage.
  • python3 scripts/localization_catalog.py check: 7 catalogs / 9 locales, zero parity errors. Portable helper recovery translations also cover all nine locales, with a Japanese behavior test.
  • git diff --check: pass.
  • Tagged managed app build and production CLI doctor verification: pending coordinated native slot (refactor: extract session persistence and autosave lifecycle #79 → refactor: extract main window registry and context ownership #80 → fix: recover control socket after pathname loss #84). No app launch, live config mutation, or runtime/TCC application claim.

Demo Video

No visual layout changed. The deterministic production-code interleaving and actual Settings-model tests are the current behavior evidence. No live-user video or dogfood is claimed.

Checklist

  • Tested the change locally with owned temporary fixtures
  • Added failing-before/passing-after behavioral coverage
  • Updated the contract and helper guidance
  • No iOS connectivity/lifecycle/workspace/terminal behavior changed; soak coverage is unaffected
  • Tagged app build / production CLI validation complete
  • Requested additional bot reviews (not triggered without second-model review opt-in)
  • All review comments resolved

@teamleaderleo

Copy link
Copy Markdown
Owner Author

Fresh prerequisite audit: manaflow-ai#13218 still has two review findings at included head 89e9eea55820e9224352256e0183ac62a25029eb.

  • Native cache root: composed fix: recover control socket after pathname loss #84 already assigns the parsed persistedRoot after publication (behavior commit 9775b1a036a23c64390e4e54edc13359bf21584e). Standalone P still needs its own repair. This update is source confirmation, not a fresh native test run.
  • Python duplicate-ancestor removal: reproduced against the actual inherited editor. Removing app.appearance from {"app":{"appearance":"hidden"},"app":{"appearance":"dark"}} produces {"app":{"appearance":"hidden"},"app":{}}. The effective value is absent, but a later removal of the effective ancestor can expose the hidden value. This remains unresolved in the included prerequisite.

The documented cross-language receipt flow is restricted to ordinary unique-key settings; this audit does not widen that guarantee. Requested P repair ownership through the campaign coordinator. #84 owns only its composed transaction branch and will integrate/recheck the repaired prerequisite once assigned and available. No branch or production code changes, native build, or live-config mutation occurred during this audit.

@teamleaderleo

Copy link
Copy Markdown
Owner Author

Integrated the repaired lossless prerequisite deliberately; P's owning branch was not edited here.

The native conflict was equivalent writtenRoot/persistedRoot naming; retained #84's existing parsed-written-root cache implementation. P's native cache agreement regression is now included. The Python editor now removes the leaf across duplicate ancestors, addressing the previously reproduced shadowed-value finding.

Executed on this updated composition: python3 -m unittest discover -s tests -p 'test_cmux_settings_*.py' — 25 tests pass (private execution receipt bc9d87a31f4ce413); git diff --check passes. No native compile or test run was started during integration. Prior 39 native store + 4 model passes are historical working-tree evidence; the new P native test has not been executed locally on this head. Native/CLI gates remain explicit in the PR.

Queue coordination now hands #84's eventual release to #79's brief manaflow-ai#13217 symlink regression run, then to the optional host benchmark; #85 withdrew its local reservation. All mutations here were scoped source/Git changes and temporary fixtures, with no live config or app launch.

@teamleaderleo

Copy link
Copy Markdown
Owner Author

Superseded by the actual upstream cmux review: manaflow-ai#13254. The composed-base comparison remains linked there for reviewing the narrow transaction patch while its prerequisites are open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant