amadan

oficina / docs Public

Task 3 fix round 1: test the verifier's actual mismatch case, fix a stale comment

Review finding 1 (important): TestVerifySnapshotAcceptsItsOwnAndRejectsGarbage
covered return codes 0 and ERR_DECODE but never return code 1 — a
validly-decodable snapshot of a genuinely different document, the
verifier's primary purpose. Extends the test with a second handle
carrying a different fixture (simple.bin vs. hello) and asserts its
snapshot verifies false, nil against the first handle; the two
fixtures are in fact distinguishable, so no assertion needed weakening.

Review finding 2 (minor, promoted): the doc comment above
ErrUpdateLogFull still said compaction was "unsupported ... this ABI
doesn't have yet", contradicting both the corrected error message
right below it and the maxRetainedUpdates comment above — ABI v4's
export exists; only the host-side wiring is missing. Corrected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 60c4412b284c47349dd63670bafd2f2558d00ab1 parent 9e61570
2 files changed, +41 −6
  • internal/core/core.go +4 −2
  • internal/core/core_test.go +37 −4
diff --git a/internal/core/core.go b/internal/core/core.go
index bc7c7c6..ae312a8 100644
--- a/internal/core/core.go
+++ b/internal/core/core.go
@@ -144,8 +144,10 @@ const MaxRetainedUpdates = maxRetainedUpdates
// ErrUpdateLogFull is returned by Handle.Apply once a document's retained
// update log has reached maxRetainedUpdates. See that constant's doc
-// comment: this is a deliberate, visible failure standing in for
-// compaction support this ABI doesn't have yet, not a bug.
+// comment: the ABI has exported a snapshot/compaction primitive since
+// v4; what's still missing is this package calling it and a caller
+// persisting the result. This is a deliberate, visible failure standing
+// in for that host-side wiring, not a bug.
var ErrUpdateLogFull = errors.New("update log exceeds the retained limit; document needs compaction (not yet wired up on the host side) before it can accept further updates")
// Option configures a Runtime. See WithGuestCallTimeout.
diff --git a/internal/core/core_test.go b/internal/core/core_test.go
index 54a58ce..c8cd2be 100644
--- a/internal/core/core_test.go
+++ b/internal/core/core_test.go
@@ -221,11 +221,16 @@ func TestSnapshotRoundTripsThroughAFreshHandle(t *testing.T) {
}
}
-// TestVerifySnapshotAcceptsItsOwnAndRejectsGarbage exercises the two
+// TestVerifySnapshotAcceptsItsOwnAndRejectsGarbage exercises all three
// outcomes VerifySnapshot returns without an error: a document's own
-// snapshot verifies true, and bytes that are not a decodable update at
-// all (ERR_DECODE) come back false with no error — a "no" answer, not a
-// failure of the call.
+// snapshot verifies true; a validly-decodable snapshot of a genuinely
+// different document verifies false (return code 1 — the verifier's
+// primary purpose, distinct from "not even a decodable update"); and
+// bytes that are not a decodable update at all (ERR_DECODE) also come
+// back false with no error — a "no" answer, not a failure of the call.
+// The middle case and the garbage case are different failures of the
+// candidate and must not be conflated: only the garbage case may ever
+// legitimately share its outcome with a rejected-but-valid document.
func TestVerifySnapshotAcceptsItsOwnAndRejectsGarbage(t *testing.T) {
ctx := context.Background()
rt, err := core.New(ctx)
@@ -258,6 +263,34 @@ func TestVerifySnapshotAcceptsItsOwnAndRejectsGarbage(t *testing.T) {
t.Error("a document refused its own snapshot")
}
+ // A genuinely different document's snapshot: a real, validly-decodable
+ // update, but not a snapshot of h. This is not "simple" (also a
+ // fixtures/vectors/fragment .bin, used elsewhere in this file via
+ // os.ReadFile) reused as a second, unrelated document's content — the
+ // point is that decode succeeds and the structural comparison still
+ // says no.
+ other, err := rt.NewDoc(ctx, "verify-test-other", 2)
+ if err != nil {
+ t.Fatalf("NewDoc other: %v", err)
+ }
+ defer other.Close(ctx)
+
+ otherUpdate, err := os.ReadFile("../../fixtures/vectors/fragment/simple.bin")
+ if err != nil {
+ t.Fatalf("read fixture: %v", err)
+ }
+ if err := other.Apply(ctx, otherUpdate); err != nil {
+ t.Fatalf("Apply other: %v", err)
+ }
+ otherSnap, err := other.Snapshot(ctx)
+ if err != nil {
+ t.Fatalf("Snapshot other: %v", err)
+ }
+
+ if ok, err := h.VerifySnapshot(ctx, otherSnap); err != nil || ok {
+ t.Errorf("a different document's snapshot verified against h: ok=%v err=%v", ok, err)
+ }
+
if ok, err := h.VerifySnapshot(ctx, []byte{0xff, 0x00}); err != nil || ok {
t.Errorf("garbage verified: ok=%v err=%v", ok, err)
}