fix(core): a refused source context is a failure, not an unknown write
Issue #44: an OpenCode import carried `role=user, written_by=unknown`. The role/author set now admits it, but the row still never moved: `HyWriteBindings` is built inside `ingest`, and the invocation marker is recorded *before* that call, so the refusal arrived with the marker already set. The classifier checked `hy_invoked` first and parked the task as `unknown` — a state that means "HY may hold a write we cannot confirm" about a request that provably never reached HY. The reason stayed off the row, the task could not be re-driven, and the queue behind it stopped. `hy_memory_write_verification_context_scope` is raised from that construction alone, and `_check_source_context` (which does run after the call) raises `hy_memory_evidence_changed` instead, so the code is unambiguous: nothing was sent. It joins `PRE_SEND_REFUSAL_CODES`, and both classifiers now test that set before the marker: the durable path settles `failed` with the marker cleared so reclaim does not treat a row HY never saw as possibly-sent, and the task path fails with the code instead of marking unknown. Regression covers both directions: a pre-call refusal must end `failed` with `hy_invoked=0`, and a post-call refusal (`authority_invalid`) must still be `unknown` — the fix narrows the unknown set, it does not widen the failure set.
This commit is contained in:
1 parent
dc0ba49fbe
commit
0df84c086c
3 files changed
+124
-5
No files matched your search
@@ -174,6 +174,22 @@ def _report_stale_known_reasons(task_id: str, reasons: Mapping[str, int]) -> Non
|
||||
|
||||
MEMORY_INGEST_POLICY_VERSION = "hy-native-v23"
|
||||
UNIFIED_HY_AGENT_ID = "one-memory"
|
||||
#: Deterministic refusals raised before the payload reaches HY, so nothing was
|
||||
#: sent even though the invocation marker is already recorded by then: the
|
||||
#: write bindings are built inside `ingest` ahead of the call, and the input
|
||||
#: window is checked while the request is frozen.
|
||||
#:
|
||||
#: They are terminal rather than unknown. `unknown` here means "HY may hold a
|
||||
#: write we cannot confirm", and that is exactly what did not happen — parking
|
||||
#: the task on it hid the reason and stopped the queue behind it (issue #44).
|
||||
#: Kept in step with `durable_queue.PRE_SEND_REFUSAL_CODES`, which the durable
|
||||
#: write path applies to the same codes.
|
||||
PRE_SEND_REFUSAL_CODES = frozenset(
|
||||
{
|
||||
"hy_memory_input_exceeds_window",
|
||||
"hy_memory_write_verification_context_scope",
|
||||
}
|
||||
)
|
||||
_BACKFILL_BATCH_SIZE = 8
|
||||
_BACKFILL_DRAIN_BUDGET_SECONDS = 330.0
|
||||
_HISTORY_MERGE_TURNS = 8
|
||||
@@ -555,6 +571,16 @@ class MemoryTaskRunner:
|
||||
"hy_memory_model_timeout",
|
||||
}
|
||||
safe_terminal_after_invocation = {"hy_memory_import_no_readable_output"}
|
||||
if code in PRE_SEND_REFUSAL_CODES:
|
||||
# Raised before the call, so the marker only says "about to":
|
||||
# fail with the reason rather than parking on a write HY never
|
||||
# saw. See the constant and issue #44.
|
||||
return await in_the_background(
|
||||
self.tasks.fail,
|
||||
task_id,
|
||||
runner_token=runner_token,
|
||||
error_code=code,
|
||||
)
|
||||
if hy_invoked and (
|
||||
code == "hy_memory_evidence_changed"
|
||||
or (
|
||||
|
||||
@@ -249,9 +249,17 @@ MODEL_NOT_READY_CODES = frozenset(
|
||||
#: nothing was sent and retrying the same intent changes nothing — a terminal
|
||||
#: `failed`, not `unknown` (which would wrongly imply a possibly-applied
|
||||
#: write) and not a retry (which would fail identically).
|
||||
#:
|
||||
#: `hy_memory_write_verification_context_scope` belongs here for the same
|
||||
#: reason: `HyWriteBindings` is built inside `ingest` *before* the payload
|
||||
#: reaches HY, and it is the only thing that raises this code. The durable
|
||||
#: marker is already set by then, so classifying it as `unknown` claimed a
|
||||
#: write may have landed when HY was never called — a sticky row that blocked
|
||||
#: the queue behind it and left no reason to act on. See issue #44.
|
||||
PRE_SEND_REFUSAL_CODES = frozenset(
|
||||
{
|
||||
"hy_memory_input_exceeds_window",
|
||||
"hy_memory_write_verification_context_scope",
|
||||
}
|
||||
)
|
||||
|
||||
@@ -1985,7 +1993,17 @@ async def execute_job(
|
||||
# - transient model/provider trouble -> waiting_for_model (parked).
|
||||
# Everything else may have crossed into HY, and HY has no idempotency
|
||||
# key, so it is `unknown` rather than a blind retry.
|
||||
if hy_invoked:
|
||||
if code in PRE_SEND_REFUSAL_CODES:
|
||||
# Provably before the bridge, even though the marker is already
|
||||
# set: the write bindings are built — and the input window
|
||||
# checked — before the payload reaches HY, and this code is raised
|
||||
# from that construction alone. Nothing was sent, so this is
|
||||
# terminal rather than unknown, and the marker is cleared so
|
||||
# reclaim does not treat a row HY never saw as possibly-sent.
|
||||
# Otherwise a refused source context pinned the queue behind it
|
||||
# with no reason shown and no way forward. See issue #44.
|
||||
state, invoked_marker = FAILED, False
|
||||
elif hy_invoked:
|
||||
# Error names do not prove execution phase: permission and
|
||||
# evidence rechecks also run after HY returns. Once the durable
|
||||
# marker is set, every failure is possibly-sent and must stay out
|
||||
@@ -1997,10 +2015,6 @@ async def execute_job(
|
||||
state, invoked_marker = WAITING_FOR_PERMISSION, False
|
||||
elif code in MODEL_NOT_READY_CODES:
|
||||
state, invoked_marker = WAITING_FOR_MODEL, False
|
||||
elif code in PRE_SEND_REFUSAL_CODES:
|
||||
# Provably before the bridge: the frozen input is stale or too
|
||||
# large. Nothing was sent, so this is terminal rather than unknown.
|
||||
state, invoked_marker = FAILED, False
|
||||
elif code == "model_content_transport_request_failed":
|
||||
# The generation or embedding HTTP exchange can fail after HY has
|
||||
# already persisted a node. Treating that as a pre-send failure
|
||||
|
||||
@@ -14,6 +14,7 @@ from datetime import UTC, datetime, timedelta
|
||||
import pytest
|
||||
|
||||
from one_memory.database_access import main_database
|
||||
from one_memory.hy_memory_bridge import HyMemoryBridgeError
|
||||
from one_memory.memory_server.contracts import BatchIdentity, WriteRequest
|
||||
from one_memory.memory_server.durable_queue import (
|
||||
CANCELLED,
|
||||
@@ -4814,6 +4815,84 @@ def test_readmit_paths_never_requeue_a_possibly_sent_row(tmp_path) -> None:
|
||||
assert DurableWriteDrain(queue, execute=_never_execute).claim(limit=5) == ()
|
||||
|
||||
|
||||
def test_a_refused_source_context_fails_instead_of_parking_as_unknown(
|
||||
tmp_path,
|
||||
) -> None:
|
||||
"""A refusal raised before the call must not read as a possibly-sent write.
|
||||
|
||||
Issue #44: an OpenCode import carried `role=user, written_by=unknown`, so
|
||||
`HyWriteBindings` — built inside `ingest`, ahead of the payload reaching HY
|
||||
— refused the batch. The invocation marker is recorded before that call, so
|
||||
the row was parked as `unknown`: a state that says "HY may hold a write we
|
||||
cannot confirm" about a request that was never sent. It kept the reason off
|
||||
the row and stopped the queue behind it.
|
||||
"""
|
||||
path = tmp_path / "one-memory.sqlite3"
|
||||
queue = DurableWriteQueue(path)
|
||||
record = queue.submit(_request(key="refused-scope", source="opencode"))
|
||||
claimer = DurableWriteDrain(queue, execute=_never_execute, owner="refused-owner")
|
||||
assert claimer.claim(limit=1) == (record.job_id,)
|
||||
|
||||
asyncio.run(
|
||||
execute_job(
|
||||
record.job_id,
|
||||
queue=queue,
|
||||
supervisor=_Recorder(
|
||||
error=HyMemoryBridgeError("hy_memory_write_verification_context_scope"),
|
||||
invoke_hy=True,
|
||||
),
|
||||
evidence=_EvidenceStore({"e1"}),
|
||||
owner="refused-owner",
|
||||
)
|
||||
)
|
||||
|
||||
with sqlite3.connect(path.as_uri() + "?mode=ro", uri=True) as db:
|
||||
state, invoked, last_error = db.execute(
|
||||
"SELECT state, hy_invoked, last_error FROM memory_write_jobs "
|
||||
"WHERE job_id=?",
|
||||
(record.job_id,),
|
||||
).fetchone()
|
||||
|
||||
assert state == FAILED
|
||||
# Nothing was sent, so reclaim must not treat this as possibly-sent.
|
||||
assert invoked == 0
|
||||
assert last_error == "hy_memory_write_verification_context_scope"
|
||||
assert DurableWriteDrain(queue, execute=_never_execute).claim(limit=1) == ()
|
||||
|
||||
|
||||
def test_a_failure_after_the_call_still_parks_as_unknown(tmp_path) -> None:
|
||||
"""The fix must not widen: a post-call refusal is still possibly-sent."""
|
||||
path = tmp_path / "one-memory.sqlite3"
|
||||
queue = DurableWriteQueue(path)
|
||||
record = queue.submit(_request(key="after-call", source="opencode"))
|
||||
claimer = DurableWriteDrain(queue, execute=_never_execute, owner="after-call-owner")
|
||||
assert claimer.claim(limit=1) == (record.job_id,)
|
||||
|
||||
asyncio.run(
|
||||
execute_job(
|
||||
record.job_id,
|
||||
queue=queue,
|
||||
supervisor=_Recorder(
|
||||
error=HyMemoryBridgeError(
|
||||
"hy_memory_write_verification_authority_invalid"
|
||||
),
|
||||
invoke_hy=True,
|
||||
),
|
||||
evidence=_EvidenceStore({"e1"}),
|
||||
owner="after-call-owner",
|
||||
)
|
||||
)
|
||||
|
||||
with sqlite3.connect(path.as_uri() + "?mode=ro", uri=True) as db:
|
||||
state, invoked = db.execute(
|
||||
"SELECT state, hy_invoked FROM memory_write_jobs WHERE job_id=?",
|
||||
(record.job_id,),
|
||||
).fetchone()
|
||||
|
||||
assert state == UNKNOWN
|
||||
assert invoked == 1
|
||||
|
||||
|
||||
def test_a_window_holding_only_blocked_rows_advances_instead_of_starving(
|
||||
tmp_path,
|
||||
) -> None:
|
||||
|
||||
Reference in new issue
Block a user