fix(web/ctx): echo current mtime_ns on Settings resolve 409 abort#782
Merged
fix(web/ctx): echo current mtime_ns on Settings resolve 409 abort#782
Conversation
Settings ``POST /api/context/settings/resolve`` returned only ``status``/``reason`` on stale-mtime abort, while Skills/Commands/Agents all echo the current ``st_mtime_ns`` in their 409 envelopes (PRs #776/#777/#779/#781). This was a parity gap, not an intentional contract: the route already had the old ``mtime_ns`` and the recheck already re-stat()'d the target, so adding the echo is a one-line behavioral fix that lets clients refresh local state without an extra round-trip. Tighten ``test_resolve_route_returns_soft_abort_on_stale_mtime`` to the same triplet the sibling routes pin: * response status == "aborted" * envelope ``mtime_ns`` == current ``st_mtime_ns`` * no-write content, asserted symmetrically: "echo user" still present AND "echo canonical" absent (catches a regression that wrote before returning aborted, which a status/reason-only assertion would miss). Mutation-validated both new assertions against the production code before committing — dropping the envelope echo raises ``KeyError`` on ``body["mtime_ns"]``; reordering ``_write_json`` above the recheck flips the "echo user" present check. 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
st_mtime_nsin its 409 abort envelope, matching the Skills/Commands/Agents contract. This lets clients refresh local state on conflict without an extra round-trip.status+ envelopemtime_nsecho + no-write content (asserted symmetrically with positive marker + negative!=)._write_jsonabove the recheck both flip the test red.Why
Codex review on the three-phase parity series (#776/#777/#779/#781) flagged Settings as the lone surface that was not asserting the strict triplet. The reviewer's inspection of
settings_sync.py:262confirmed it was a real envelope-shape gap — the recheck already re-stat()'d the target, so adding the echo is one line. Tightening the test alone (without the route fix) would have papered over an asymmetric API surface.status: "aborted"mtime_nsechoCompatibility
mtime_nsis additive in the response. The only known consumer (settings-hooks-watchdog.js:123–139) readsresult.statusandresult.reason— adding a new key cannot break it.Test plan
uv run pytest packages/memtomem/tests/test_context_settings.py— 37 passedtest_web_routes_context*.py+test_context_settings.py— 104 passeduv run ruff check+uv run ruff format --check— cleanKeyError: 'mtime_ns'✓ caughtassert 'echo user' in on_diskfails ✓ caught🤖 Generated with Claude Code