Add e2e scenarios as named gate in ck-commit #40

atanabe · 2026-06-04T02:00:00+02:00

atanabe commented

2026-06-04 02:00:00 +02:00

Add e2e scenarios as named gate in ck-commit

Along the way:

Review ck-files design doc: corrections and review report
Rewrite ck-files design doc to v2: lock every decision
Annotate ckeditor-files.md v2 with implementation drift
added .claude worktrees to the .gitignore
Replace SUSPEND_WINDOW with hash-and-ignore self-write filter

Touches 9 files: .claude/skills/ck-commit.md, .gitignore, Cargo.lock, ckeditor-files-review.md, ckeditor-files.md, ckeditor/Cargo.toml and 3 more.

Add e2e scenarios as named gate in ck-commit Along the way: - Review ck-files design doc: corrections and review report - Rewrite ck-files design doc to v2: lock every decision - Annotate ckeditor-files.md v2 with implementation drift - added .claude worktrees to the .gitignore - Replace SUSPEND_WINDOW with hash-and-ignore self-write filter Touches 9 files: `.claude/skills/ck-commit.md`, `.gitignore`, `Cargo.lock`, `ckeditor-files-review.md`, `ckeditor-files.md`, `ckeditor/Cargo.toml` and 3 more.

ebrandt approved these changes

2026-06-05 14:03:00 +02:00

ebrandt left a comment

A clear improvement. The change in ckeditor/src/files_coordinator.rs is the part I would have argued for.

A clear improvement. The change in `ckeditor/src/files_coordinator.rs` is the part I would have argued for.

ckeditor/src/files_coordinator.rs

					
				@ -655,0 +813,4 @@

				    }

				    #[test]

				    fn consume_self_write_matches_recorded_hash_and_removes_entry() {

consume_self_write_matches_recorded_hash_and_removes_entry reads, decides and writes in one function. The deciding part is the piece I would want to test on its own. Worth splitting?

`consume_self_write_matches_recorded_hash_and_removes_entry` reads, decides and writes in one function. The deciding part is the piece I would want to test on its own. Worth splitting?

ckeditor/tests/scenarios.rs

					
				@ -18,4 +18,5 @@ mod scenarios {

				    pub mod keystrokes_reach_composed_app;

				    pub mod move_file_via_tree;

				    pub mod open_file_via_tree_keys;

				    pub mod self_write_then_external_write_surfaces;

I had to read self_write_then_external_write_surfaces twice. Nothing wrong with it, but one line of comment would save the next person the second read.

I had to read `self_write_then_external_write_surfaces` twice. Nothing wrong with it, but one line of comment would save the next person the second read.

ckeditor/tests/scenarios/self_write_then_external_write_surfaces.rs

					
				@ -0,0 +35,4 @@

				#[test]

				fn scenario_self_write_then_external_write_surfaces() {

				    let dir = tempfile::tempdir().unwrap();

dir unwraps here. If this is reachable from user input it is a panic in front of a person rather than an error they can read. Is it provably there at this point, or should it carry a ??

`dir` unwraps here. If this is reachable from user input it is a panic in front of a person rather than an error they can read. Is it provably there at this point, or should it carry a `?`?

atanabe commented

2026-06-06 10:39:00 +02:00

Split. The decision in consume_self_write_matches_recorded_hash_and_removes_entry is a pure function now, with its own tests in ckeditor/src/files_coordinator.rs.

Split. The decision in `consume_self_write_matches_recorded_hash_and_removes_entry` is a pure function now, with its own tests in `ckeditor/src/files_coordinator.rs`.

atanabe commented

2026-06-07 02:00:00 +02:00

It is reachable. The function in ckeditor/tests/scenarios/self_write_then_external_write_surfaces.rs returns a Result now and the caller reports it, so the unwrap on dir is gone.

It is reachable. The function in `ckeditor/tests/scenarios/self_write_then_external_write_surfaces.rs` returns a Result now and the caller reports it, so the unwrap on `dir` is gone.

atanabe commented

2026-06-07 02:00:00 +02:00

Comment added on self_write_then_external_write_surfaces in ckeditor/tests/scenarios.rs. It was obvious when I wrote it and it will not be in a month.

Comment added on `self_write_then_external_write_surfaces` in `ckeditor/tests/scenarios.rs`. It was obvious when I wrote it and it will not be in a month.

atanabe added 6 commits

2026-06-08 09:00:00 +02:00

Add e2e scenarios as named gate in ck-commit e3bc1b9f5c

Inserts a dedicated Step 2 "E2E Scenarios (GATE)" that runs
`cargo test -p ckeditor --test scenarios` before the broader
workspace tests. Scenario failures are the highest-signal
regression indicator — a red scenario usually means a real
user-facing behaviour broke, so failing fast and visibly beats
burying the result inside a workspace-test wall of output.

Renumbers the subsequent steps (workspace tests → 3, build → 4,
commit → 5, merge → 6). The worktree-detection note at Step 0
now points at Step 6.

Review ck-files design doc: corrections and review report d5f3a362fa

Verified every file:line citation in ckeditor-files.md against the
code at 8cf4070. Expanded the §1.1 inventory from four to nine
source-file fs call sites, corrected subtle drift in Buffer::hydrate
and apply_workspace_edit descriptions, and flagged three load-bearing
gaps (linter-rule infrastructure, suspend/resume race, behaviour
change for closed-file LSP edits) in ckeditor-files-review.md.

Recommendation in the report: (c) needs author rework before PR 1
can begin — see the report for specifics.

Rewrite ck-files design doc to v2: lock every decision 7996b1731a

Replaces the open-questions v1 with a v2 that answers every flagged
issue from the review so a future agent can implement without re-
litigating design choices.

Changes:
- Decision record in §17 with all 15 locked decisions and their
  rationale (hash-and-ignore over suspend, CK018 AST linter in PR 1,
  canonicalized paths, focused-context-only watchers, 64 MiB cap,
  closed-files-stay-closed behaviour, metadata out of scope, etc.).
- New §7 Linter enforcement: CK018 rule spec, allow-list format,
  three-gate layering per CLAUDE.md.
- New §8 Context integration: focused-context watcher model,
  unmount-before-git-switch ordering so context switches never spam
  reconciliation banners.
- New §9 Extension seams: architecture-smell markers, persistent
  file annotations (per-project/per-context), MCP endpoints, ghost
  patches — each named with the concrete hook point so future
  features plug in without rewriting the file layer.
- §5.3 rewritten around SelfWriteHashes (TTL-bounded SHA-256 set);
  suspend/resume moved to rejected alternatives in §16.
- §6 invariants numbered and each bound to a specific test in §11.
- §11 testing section restructured: unit / coordinator / context /
  linter / manual golden paths. Every invariant has a named test.
- §14 PR sequence updated: PR 1 now includes the CK018 rule
  infrastructure, PR 2 migrates all nine call sites (not four),
  PRs 5-7 split context integration / UI / LSP-routing cleanly.
- All HTML review comments removed.

No behaviour change; doc-only. No baseline movement.

Annotate ckeditor-files.md v2 with implementation drift 99a815f930

PRs 1-8 on master shipped four decisions that differ from the v2
design doc's §17 decision record. The merge of review/ck-files
brought the v2 doc over verbatim; this commit records what actually
shipped so the next author does not have to diff-read seven months
of git log to find out.

New subsection "Decisions overruled by implementation" at the end of
§17 catalogs:

- #6 hash-and-ignore → shipped as 1s time-window suspend
  (files_coordinator.rs:29-37, introduced in 13d002b, hardened in
  578a5b6). The race the review report warned about is live: events
  delivered during the suspension window are filtered out and no
  re-read logic runs at tick boundary. Marked as real drift to fix.

- #8 closed-files-stay-closed → shipped as closed-files-auto-open
  (b3dcb37). Deliberate UX reversal: user must see rename-symbol
  blast radius as dirty tabs. v2 §5.3 prose still describes the old
  choice — marked for prose reconciliation.

- #10 64 MiB file-size cap → not shipped; ck_files::read calls
  std::fs::read_to_string unchecked. Real gap, multi-GB file opens
  OOM the editor. Marked as real drift to fix.

- #13 canonicalized watcher keys → not shipped; raw PathBuf keys.
  Linux-only today so the case-insensitive / symlink aliasing hazard
  is latent. Marked as backlog.

Status preamble at the top of the doc updated to say PRs 1-8 landed,
point at the new subsection as source of truth for what actually
shipped, and note the FileError::AlreadyExists(PathBuf) variant PR 1
added that §3.1 still omits.

No behaviour change; doc-only. No baseline movement.

added .claude worktrees to the .gitignore 50c8d85a3b

Replace SUSPEND_WINDOW with hash-and-ignore self-write filter 386478c5a9

Closes the drift flagged in ckeditor-files.md §17 row #6 and brings the
shipped code back in line with the v2 decision. The 1-second
`SUSPEND_WINDOW` introduced in 13d002b (and hardened in 578a5b6) had
the race the review report warned about: an external writer that
landed inside the suspension window had its Modified event filtered
out by `event_is_suspended` and no re-read logic ran at tick boundary,
so the buffer silently drifted out of sync with disk.

Per §5.3, the coordinator now records a SHA-256 hash of the bytes it
is about to write (with a 3-second TTL, GC'd on every `tick`) and
dispatch reads the on-disk bytes, hashes them, and consumes the event
only when the hash matches a recorded self-write. Foreign writes hash
differently, so they pass through to `handle_modified` unfiltered.
`Removed` and `Renamed` events always deliver — our writes never
produce those variants.

`apply_edits` is now split cleanly: the open-buffer arm does not touch
disk and records no self-write; the closed-file arm reads, applies
edits, hashes the new content, records, and writes. `write_through`
hashes the incoming content before `write_atomic`.

New scenario test `self_write_then_external_write_surfaces.rs` (74
lines) pins the exact race the old model lost: after a `write_through`
with content A, a foreign writer lands content B before the watcher
delivers our self-echo, and the clean buffer auto-reloads to B.

Gate status: lint 0/0, `cargo test --workspace` all green,
`cargo mutants --package ckeditor --file
ckeditor/src/files_coordinator.rs` drives to 21/21 caught (100% kill).
No baseline movement — `sha2` is an external dep so `files_coordinator`
efferent coupling stays at 1.

ebrandt manually merged commit 63a4a80fc3 into master

2026-06-08 09:00:00 +02:00

ebrandt referenced this pull request from a commit

2026-06-08 09:00:00 +02:00

Merge pull request #40 from northgale/feat/040-add-e2e-scenarios-as-named-gate-in-ck-commit

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

northgale/ckeditor!40

Rows
Columns