feat(logstream): warn at store time on unappliable kind=patch artifacts
Found in the first RFC 003 dogfood: windows-codex stored a patch without its final trailing newline and git apply rejected the diff as corrupt on the receiving side. Content stays verbatim — put_artifact/submit_patch now return an advisory 'warnings' list (missing trailing newline, CRLF line endings) so the producer can fix the handoff while it still owns the diff. CLI 'artifact put' mirrors warnings to stderr. (cherry picked from commit 7c0c9e1362cf20fabc72d61669fe2ea6125f4e3a)
This commit is contained in:
parent
a9f19a2f7b
commit
a50677d0e7
|
|
@ -1382,6 +1382,10 @@ def cmd_artifact(args):
|
|||
print(f"Stored {artifact['id']} kind={artifact['kind']}")
|
||||
print(f" sha256={artifact['sha256']}")
|
||||
print(f" size={artifact['size_bytes']} bytes")
|
||||
# Warnings go to stderr in both modes so `--json | jq` stays
|
||||
# clean while interactive callers still can't miss them.
|
||||
for warning in artifact.get("warnings", []):
|
||||
print(f"Warning: {warning}", file=sys.stderr)
|
||||
elif args.artifact_action == "get":
|
||||
try:
|
||||
artifact = ls.get_artifact(args.artifact_id)
|
||||
|
|
|
|||
|
|
@ -390,6 +390,33 @@ class Logstream:
|
|||
row = conn.execute("SELECT rowid, * FROM events WHERE id = ?", (event_id,)).fetchone()
|
||||
return self._event_dict(row, artifact_ids=list(artifact_ids))
|
||||
|
||||
@staticmethod
|
||||
def _patch_content_warnings(kind: str, content: str) -> list[str]:
|
||||
"""Advisory checks for ``kind=patch`` content. Never mutates content.
|
||||
|
||||
Verbatim storage means we store exactly what the producer sent —
|
||||
but a diff that ``git apply`` will reject as corrupt is a broken
|
||||
handoff, so warn at store time where the producer can still fix it.
|
||||
Found in the wild during the first RFC 003 dogfood: a patch stored
|
||||
without its final trailing newline truncates the last hunk line.
|
||||
"""
|
||||
if kind != "patch":
|
||||
return []
|
||||
warnings = []
|
||||
if not content.endswith("\n"):
|
||||
warnings.append(
|
||||
"patch content does not end with a trailing newline; "
|
||||
"git apply will reject the final hunk as corrupt — "
|
||||
"store the diff byte-exactly including its trailing newline"
|
||||
)
|
||||
if "\r" in content:
|
||||
warnings.append(
|
||||
"patch content contains carriage returns (CRLF line endings); "
|
||||
"git apply often rejects CRLF diffs — generate the diff with "
|
||||
"LF endings"
|
||||
)
|
||||
return warnings
|
||||
|
||||
def put_artifact(
|
||||
self,
|
||||
kind: str,
|
||||
|
|
@ -401,6 +428,9 @@ class Logstream:
|
|||
|
||||
Returns the artifact record without echoing ``content`` back —
|
||||
callers already hold the content; readers use :meth:`get_artifact`.
|
||||
For ``kind=patch``, a ``warnings`` list is included when the diff
|
||||
looks unappliable (missing trailing newline, CRLF endings); the
|
||||
content itself is still stored verbatim.
|
||||
"""
|
||||
if not isinstance(kind, str) or kind not in ARTIFACT_KINDS:
|
||||
allowed = ", ".join(sorted(ARTIFACT_KINDS))
|
||||
|
|
@ -439,7 +469,7 @@ class Logstream:
|
|||
metadata_json,
|
||||
),
|
||||
)
|
||||
return {
|
||||
record = {
|
||||
"id": artifact_id,
|
||||
"kind": kind,
|
||||
"sha256": digest,
|
||||
|
|
@ -447,6 +477,10 @@ class Logstream:
|
|||
"created_by": created_by,
|
||||
"created_at": created_at,
|
||||
}
|
||||
warnings = self._patch_content_warnings(kind, content)
|
||||
if warnings:
|
||||
record["warnings"] = warnings
|
||||
return record
|
||||
|
||||
def ack_event(
|
||||
self,
|
||||
|
|
|
|||
|
|
@ -338,6 +338,49 @@ class TestArtifacts:
|
|||
assert listed[0]["artifact_ids"] == [artifact["id"]]
|
||||
|
||||
|
||||
class TestPatchContentWarnings:
|
||||
"""Advisory guards for unappliable diffs, found in the first dogfood:
|
||||
a patch stored without its trailing newline is rejected by git apply."""
|
||||
|
||||
def test_patch_without_trailing_newline_warns(self, logstream):
|
||||
artifact = logstream.put_artifact(
|
||||
kind="patch",
|
||||
content="diff --git a/x b/x\n+no trailing newline",
|
||||
created_by="windows-codex",
|
||||
)
|
||||
assert any("trailing newline" in w for w in artifact["warnings"])
|
||||
# Content is still stored verbatim — the warning never mutates it.
|
||||
assert logstream.get_artifact(artifact["id"])["content"].endswith("newline")
|
||||
|
||||
def test_patch_with_crlf_warns(self, logstream):
|
||||
artifact = logstream.put_artifact(
|
||||
kind="patch",
|
||||
content="diff --git a/x b/x\r\n+crlf\r\n",
|
||||
created_by="windows-codex",
|
||||
)
|
||||
assert any("carriage returns" in w for w in artifact["warnings"])
|
||||
|
||||
def test_clean_patch_has_no_warnings_key(self, logstream):
|
||||
artifact = logstream.put_artifact(
|
||||
kind="patch", content=TestArtifacts.PATCH, created_by="windows-codex"
|
||||
)
|
||||
assert "warnings" not in artifact
|
||||
|
||||
def test_non_patch_kinds_never_warn(self, logstream):
|
||||
artifact = logstream.put_artifact(
|
||||
kind="log", content="no trailing newline", created_by="windows-codex"
|
||||
)
|
||||
assert "warnings" not in artifact
|
||||
|
||||
def test_submit_patch_propagates_warnings(self, logstream):
|
||||
result = logstream.submit_patch(
|
||||
content="diff --git a/x b/x\n+truncated",
|
||||
from_agent="windows-codex",
|
||||
stream="project/mempalace",
|
||||
)
|
||||
assert any("trailing newline" in w for w in result["artifact"]["warnings"])
|
||||
|
||||
|
||||
# ── Ack ───────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue