MEM01: canonical fenced storage adapter - #1170
Conversation
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| time.sleep(0.025) | ||
| continue | ||
| # PID exists with a different start identity: verified dead owner. | ||
| try: |
There was a problem hiding this comment.
🟠 High rlm/harness.py:451
Stale-lock removal races: the read_bytes() == raw check and self.path.unlink() are not atomic. When two contenders both detect the same dead owner, contender A unlinks the stale lock and acquires a fresh lease between contender B's read_bytes() and unlink(). B then deletes A's live lease, and both acquire the lock from the same snapshot, defeating the CAS fence so one committed update overwrites the other. Consider replacing the read-compare-unlink sequence with an atomic os.unlink (which fails with FileNotFoundError if the lock is already gone) or a compare-and-swap primitive that cannot delete a live lease.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @prime-agent-runtime/src/rlm/harness.py around line 451:
Stale-lock removal races: the `read_bytes() == raw` check and `self.path.unlink()` are not atomic. When two contenders both detect the same dead owner, contender A unlinks the stale lock and acquires a fresh lease between contender B's `read_bytes()` and `unlink()`. B then deletes A's live lease, and both acquire the lock from the same snapshot, defeating the CAS fence so one committed update overwrites the other. Consider replacing the read-compare-unlink sequence with an atomic `os.unlink` (which fails with `FileNotFoundError` if the lock is already gone) or a compare-and-swap primitive that cannot delete a live lease.
| throw new HarnessGenerationConflict(planningSnapshot, actual); | ||
| const candidate: HarnessState = { | ||
| schema: 2, | ||
| generation: actual.generation + 1, |
There was a problem hiding this comment.
🟠 High refinement/refinement.ts:920
A state with generation === 9007199254740991 (MAX_SAFE_INTEGER) passes validateV2 and loads successfully, but saveHarnessState unconditionally increments it to MAX_SAFE_INTEGER + 1, which fails safeInteger validation inside validateV2. This makes a legitimately loaded state permanently unmodifiable — every save attempt throws instead of either rejecting the state at load time or handling generation exhaustion explicitly. Consider rejecting generation === MAX_SAFE_INTEGER in validateV2 so the state is never accepted in the first place, or detecting exhaustion in saveHarnessState and throwing a dedicated error.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/core/refinement/refinement.ts around line 920:
A state with `generation === 9007199254740991` (`MAX_SAFE_INTEGER`) passes `validateV2` and loads successfully, but `saveHarnessState` unconditionally increments it to `MAX_SAFE_INTEGER + 1`, which fails `safeInteger` validation inside `validateV2`. This makes a legitimately loaded state permanently unmodifiable — every save attempt throws instead of either rejecting the state at load time or handling generation exhaustion explicitly. Consider rejecting `generation === MAX_SAFE_INTEGER` in `validateV2` so the state is never accepted in the first place, or detecting exhaustion in `saveHarnessState` and throwing a dedicated error.
| raise ValueError("invalid_json_value") | ||
|
|
||
| # Python's sort is Unicode code point order, the required scalar-value order. | ||
| return (json.dumps(normalize(value), ensure_ascii=False, sort_keys=True, separators=(",", ":"), allow_nan=False) + "\n").encode("utf-8") |
There was a problem hiding this comment.
🟠 High rlm/harness.py:220
_canonical_json crashes with UnicodeEncodeError when a string value contains a lone surrogate such as "\ud800". Validation in _validate_json only checks NFC normalization and does not reject surrogate code points, so a payload like {"x": "\ud800"} passes validation. Then json.dumps(..., ensure_ascii=False).encode("utf-8") raises UnicodeEncodeError because strict UTF-8 cannot encode lone surrogates. Since _decode_v2 only catches ValueError, this uncaught exception bypasses corrupt-state recovery and leaves the harness unusable. Consider rejecting surrogate code points in _validate_json for both nested string values and object keys, matching the shared wire contract.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @prime-agent-runtime/src/rlm/harness.py around line 220:
`_canonical_json` crashes with `UnicodeEncodeError` when a string value contains a lone surrogate such as `"\ud800"`. Validation in `_validate_json` only checks NFC normalization and does not reject surrogate code points, so a payload like `{"x": "\ud800"}` passes validation. Then `json.dumps(..., ensure_ascii=False).encode("utf-8")` raises `UnicodeEncodeError` because strict UTF-8 cannot encode lone surrogates. Since `_decode_v2` only catches `ValueError`, this uncaught exception bypasses corrupt-state recovery and leaves the harness unusable. Consider rejecting surrogate code points in `_validate_json` for both nested string values and object keys, matching the shared wire contract.
| except OSError as exc: | ||
| raise HarnessAtomicWriteUnsupported("owner-only harness root unavailable") from exc | ||
| with _Lease(self.file_path): | ||
| try: |
There was a problem hiding this comment.
🟠 High rlm/harness.py:644
load() catches every ValueError from _read_disk() and calls _recover_locked, which overwrites the live state file with an empty schema-2 document. An unsupported_schema error (from a valid file written by a newer schema) is therefore treated as corruption: the original file is moved aside and all active entries are replaced with empty data, instead of refusing to load the unsupported format. Consider distinguishing unsupported_schema from true corruption so a newer-schema file is rejected rather than wiped.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @prime-agent-runtime/src/rlm/harness.py around line 644:
`load()` catches every `ValueError` from `_read_disk()` and calls `_recover_locked`, which overwrites the live state file with an empty schema-2 document. An `unsupported_schema` error (from a valid file written by a newer schema) is therefore treated as corruption: the original file is moved aside and all active entries are replaced with empty data, instead of refusing to load the unsupported format. Consider distinguishing `unsupported_schema` from true corruption so a newer-schema file is rejected rather than wiped.
|
|
||
|
|
||
| _MAX_SAFE_INTEGER = 9_007_199_254_740_991 | ||
| _RFC3339_MILLIS_Z = __import__("re").compile(r"^\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d\.\d{3}Z$") |
There was a problem hiding this comment.
🟡 Medium rlm/harness.py:186
_RFC3339_MILLIS_Z only validates the digit layout of timestamps, so out-of-range values like 2026-99-99T99:99:99.000Z pass validation in _lock_owner (and _validate_v2). Malformed created_at fields are accepted as valid lock records instead of being rejected as corrupt. Consider parsing the timestamp with datetime.strptime(value, "%Y-%m-%dT%H:%M:%S.%fZ") (and optionally datetime.fromisoformat) to verify the calendar ranges, or document why only the lexical format is enforced.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @prime-agent-runtime/src/rlm/harness.py around line 186:
`_RFC3339_MILLIS_Z` only validates the digit layout of timestamps, so out-of-range values like `2026-99-99T99:99:99.000Z` pass validation in `_lock_owner` (and `_validate_v2`). Malformed `created_at` fields are accepted as valid lock records instead of being rejected as corrupt. Consider parsing the timestamp with `datetime.strptime(value, "%Y-%m-%dT%H:%M:%S.%fZ")` (and optionally `datetime.fromisoformat`) to verify the calendar ranges, or document why only the lexical format is enforced.
| def _legacy_changes(value: Any) -> list[str] | None: | ||
| """Accept the schema-1 changes wire type without language-specific coercion.""" | ||
| if isinstance(value, str): | ||
| return [value] | ||
| if isinstance(value, list) and all(isinstance(change, str) for change in value): | ||
| return value | ||
| return None |
There was a problem hiding this comment.
🟠 High rlm/harness.py:333
_legacy_changes returns schema-1 change strings without NFC normalization. A legacy refinement containing decomposed Unicode (e.g. "e\u0301") loads successfully, but the next mutation calls _validate_v2, which rejects that unchanged value as non_nfc, so the legacy file can never be saved through the normal mutation path. Applying unicodedata.normalize("NFC", change) before returning would fix this.
def _legacy_changes(value: Any) -> list[str] | None:
"""Accept the schema-1 changes wire type without language-specific coercion."""
if isinstance(value, str):
- return [value]
+ return [unicodedata.normalize("NFC", value)]
if isinstance(value, list) and all(isinstance(change, str) for change in value):
- return value
+ return [unicodedata.normalize("NFC", change) for change in value]
return None🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @prime-agent-runtime/src/rlm/harness.py around lines 333-339:
`_legacy_changes` returns schema-1 change strings without NFC normalization. A legacy refinement containing decomposed Unicode (e.g. `"e\u0301"`) loads successfully, but the next mutation calls `_validate_v2`, which rejects that unchanged value as `non_nfc`, so the legacy file can never be saved through the normal mutation path. Applying `unicodedata.normalize("NFC", change)` before returning would fix this.
| if (isRecord(item)) { | ||
| const result: Record<string, unknown> = {}; |
There was a problem hiding this comment.
🟠 High refinement/refinement.ts:394
canonicalBytes builds sorted objects using a plain object literal and bracket assignment, so an allowed own key named __proto__ in metadata, arguments, or reference invokes the inherited prototype setter instead of creating an own property. The key passes validateJson but is silently dropped from the persisted bytes. Because the post-rename verification path canonicalizes the same way, the dropped key goes undetected. Consider constructing the result object with a null prototype (e.g. Object.create(null)) or using Object.defineProperty so __proto__ becomes an own data property.
| if (isRecord(item)) { | |
| const result: Record<string, unknown> = {}; | |
| const result: Record<string, unknown> = Object.create(null); |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/core/refinement/refinement.ts around lines 394-395:
`canonicalBytes` builds sorted objects using a plain object literal and bracket assignment, so an allowed own key named `__proto__` in `metadata`, `arguments`, or `reference` invokes the inherited prototype setter instead of creating an own property. The key passes `validateJson` but is silently dropped from the persisted bytes. Because the post-rename verification path canonicalizes the same way, the dropped key goes undetected. Consider constructing the result object with a null prototype (e.g. `Object.create(null)`) or using `Object.defineProperty` so `__proto__` becomes an own data property.
| const deadline = Date.now() + 2_000; | ||
| while (true) | ||
| try { | ||
| const fd = openSync(lock, "wx", 0o600); |
There was a problem hiding this comment.
🟠 High refinement/refinement.ts:749
withHarnessLease creates the lock file with openSync before writing and syncing its owner record. If writeFileSync or fsyncSync throws, the exception escapes through the closeSync finally but the outer try/finally (which deletes the lock) is never entered because acquisition hasn't completed. This leaves an empty or partial lock file on disk. parseLock rejects such a file, so every subsequent loadHarnessState/saveHarnessState call fails with HarnessLockBusy until the file is manually removed. Consider unlinking the lock file in the inner finally when the write or fsync fails before acquisition succeeds.
Also found in 1 other location(s)
prime-agent-runtime/src/rlm/harness.py:423
_Lease.__enter__leaves the newly created lock file behind when initialization fails afteros.open, such as_write_all/fsyncraising or the mode check raisingHarnessAtomicWriteUnsupported. Because__enter__never completes,__exit__is not called; the lock record still identifies the current live process, so every subsequent load/save waits and raisesHarnessLockBusyuntil that process exits or the file is manually removed.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/core/refinement/refinement.ts around line 749:
`withHarnessLease` creates the lock file with `openSync` before writing and syncing its owner record. If `writeFileSync` or `fsyncSync` throws, the exception escapes through the `closeSync` finally but the outer `try`/`finally` (which deletes the lock) is never entered because acquisition hasn't completed. This leaves an empty or partial lock file on disk. `parseLock` rejects such a file, so every subsequent `loadHarnessState`/`saveHarnessState` call fails with `HarnessLockBusy` until the file is manually removed. Consider unlinking the lock file in the inner `finally` when the write or fsync fails before acquisition succeeds.
Also found in 1 other location(s):
- prime-agent-runtime/src/rlm/harness.py:423 -- `_Lease.__enter__` leaves the newly created lock file behind when initialization fails after `os.open`, such as `_write_all`/`fsync` raising or the mode check raising `HarnessAtomicWriteUnsupported`. Because `__enter__` never completes, `__exit__` is not called; the lock record still identifies the current live process, so every subsequent load/save waits and raises `HarnessLockBusy` until that process exits or the file is manually removed.
|
|
||
|
|
||
| _MAX_SAFE_INTEGER = 9_007_199_254_740_991 | ||
| _RFC3339_MILLIS_Z = __import__("re").compile(r"^\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d\.\d{3}Z$") |
There was a problem hiding this comment.
🟠 High rlm/harness.py:186
_RFC3339_MILLIS_Z uses \d, which in Python matches Unicode decimal digits, so timestamps with non-ASCII digits like ２０２６-０８-１０T００:００:００.０００Z pass validation. Because the shared Node runtime treats \d as ASCII-only, Python can accept and persist lock-owner records that Node rejects, breaking the cross-runtime wire contract. Use explicit [0-9] ranges instead of \d.
| _RFC3339_MILLIS_Z = __import__("re").compile(r"^\d{4}-\d\d-\d\dT\d\d:\d\d:\d\d\.\d{3}Z$") | |
| +_RFC3339_MILLIS_Z = __import__("re").compile(r"^[0-9]{4}-[0-9][0-9]-[0-9][0-9]T[0-9][0-9]:[0-9][0-9]:[0-9][0-9]\.[0-9]{3}Z$") |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @prime-agent-runtime/src/rlm/harness.py around line 186:
`_RFC3339_MILLIS_Z` uses `\d`, which in Python matches Unicode decimal digits, so timestamps with non-ASCII digits like `２０２６-０８-１０T００:００:００.０００Z` pass validation. Because the shared Node runtime treats `\d` as ASCII-only, Python can accept and persist lock-owner records that Node rejects, breaking the cross-runtime wire contract. Use explicit `[0-9]` ranges instead of `\d`.
1 participant
MEM01 — storage adapter
Status: Draft. Source reviewed; remote validation is in progress. Not merge-ready.
Scope / feature
Adds fenced atomic persistence and a canonical harness-state codec, with deterministic legacy migration and cross-runtime fixtures for the coding-agent and Python runtime implementations.
Contracts
No limiter
This change adds no client-side rate limiter, shared semaphore, admission queue, or synthetic local 429 behavior. Independent model requests remain independent.
Rollback
Revert this PR's commit range (or the storage-state feature commits) to return to B00B behavior; this cleanly removes the new persistence/codec adapter and its fixtures.
Evidence retained
The source branch retains review fixes and validation artifacts in commit history and cross-runtime fixtures, including evidence from known failed attempts and their subsequent corrections. Remote validation remains ongoing and is not represented as complete.
Exact provenance
perf/b00b-production-gateat9d9cf28d51490ef06efba738c3fff788463acdff(notmain)perf/mem01-storage-adapterb529552f10c8000fa8cf9bab0b51626d844cd78fNote
Add canonical fenced storage adapter for harness state with CAS, atomic writes, and cross-runtime fixtures
harness.pyandrefinement.ts; concurrent writes now raiseHarnessGenerationConflictinstead of silently overwriting.HarnessAtomicWriteUnsupported..lockfile lease (with nonce and process-identity verification) guards all reads and writes; a busy lock raisesHarnessLockBusy..corruptbackup and the state resets to a clean baseline; callers receiveHarnessRecoveryRequiredwhen corruption is detected at save time.saveHarnessStateno longer returns a path and requires callers to pass a current snapshot for CAS;HarnessState.snapshot()now returnsHarnessSnapshotinstead of a raw dict, breaking existing callers.📊 Macroscope summarized b529552. 5 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.