Conversation
ADR-0001 §5 c4 requires Phase B (Commands) to have an HTTP-layer fixture that pins the conflict path on the canonical PUT route. Phase B shipped before §5 was authored (RFC #761 / ADR landed in #769) and only had the happy-path CRUD test — a future regression that wrote body.content and then returned the 409 envelope would have gone undetected. Mirror ``TestUpdateSkill.test_mtime_conflict`` for Commands: PUT with a stale mtime_ns sentinel ("0"), assert 409 + status == "aborted", and **also** assert the canonical file's bytes are unchanged. The content assertion is what actually pins ADR §5 c4's no-write semantics; status labeling alone is satisfied even when the guard runs after the write. The 409 path itself (context_commands.py:241-250) already exists in production code — this is purely test-coverage backfill. Verified: mutation-tested by moving ``atomic_write_text`` above the guard; test failed on the content assertion as expected. Reverted before commit. Refs: #772, #761 (parent RFC), #769 (ADR §5) Co-Authored-By: Claude <[email protected]>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
TestUpdateSkill.test_mtime_conflict(tests/test_web_routes_context.py:222-231) for Commands, satisfying ADR-0001 §5 c4 (docs/adr/0001-context-gateway-sync-policies.md§5 c4).packages/memtomem/src/memtomem/web/routes/context_commands.py:241-250) already exists in production — this is purely test-coverage backfill. No production code changes.What §5 c4 actually requires
§5 c4 pins conflict semantics, not just the response label. A pure
status_code == 409+status == "aborted"assertion would still pass aregression that wrote
body.contentand then returned the abort envelope.This PR adds two assertions on the conflict path:
r.status_code == 409anddata["status"] == "aborted"— pins the soft-abort response shape (verbatim parity with Skills).cmd_file.read_text() == _CMD_CONTENT— pins the no-write semantics that §5 c4 actually targets.Mutation validation
Before commit, I verified the new test is regression-tight by mutating
context_commands.py:241-250to performatomic_write_textbefore themtime-guard. The status-code assertion still passed (409 was still
returned), but the content assertion failed — confirming the test catches
the very class of regression §5 c4 is designed to prevent. The mutation
was reverted before the commit.
Sibling-test parity observation (not in scope)
The Skills counterpart at
test_web_routes_context.py:222-231onlyasserts the response label, not the no-write semantics. That's a
hygiene gap of the same shape as #772 but for Phase A (Skills); it is
not a regression of any current behavior and is out of scope here.
Worth tracking as a follow-up if the team wants strict parity across
all three phases.
Test plan
uv run pytest -m "not ollama" packages/memtomem/tests/test_web_routes_context.py— 57/57 greenuv run ruff check packages/memtomem/tests/test_web_routes_context.pyuv run ruff format --check packages/memtomem/tests/test_web_routes_context.pyRefs: #772, #761 (parent RFC), #769 (ADR §5)
🤖 Generated with Claude Code