Feature 02 task 2: Store helpers: transactions, ULIDs and time conventions #11

Merged

approver_agent merged 4 commits from task/store-helpers_120dd5b6-871b-42c1-9b44-481509932b15 into feature/b9c21d67-bf72-411a-901d-77e57352d398-sqlite-data-layer-and-schema 2026-10-03 08:54:56 -04:00

Conversation 4 Commits 4 Files Changed 8

+422

dev_agent commented 2026-10-03 08:39:29 -04:00

Member

Copy Link

Copy Source

Implements task 120dd5b6 (Feature 02, task 2 of 4).

Done-condition: internal/store provides WithTx(ctx, fn) that commits on nil and rolls back on error or panic (re-panicking); a monotonic ULID generator (oklog/ulid/v2); a UTC RFC 3339 timestamp type that scans/values to TEXT; and a LocalDate type (YYYY-MM-DD) that rejects invalid dates. Tests cover commit, rollback on error, rollback on panic, ULID ordering, and timestamp/date round-trips through SQLite. Go quality floor passes.

Evidence (developer-run, to be re-verified at review): gofmt -l . empty; go vet ./... clean; go test -race -count=1 ./... ok (internal/store ~1.12s); CGO_ENABLED=0 go build ./... ok; make test ok. Only approved dependency github.com/oklog/ulid/v2 v2.1.2 added.

Base: feature/b9c21d67-...-sqlite-data-layer-and-schema (e9ee4e7).

Implements task 120dd5b6 (Feature 02, task 2 of 4). Done-condition: internal/store provides WithTx(ctx, fn) that commits on nil and rolls back on error or panic (re-panicking); a monotonic ULID generator (oklog/ulid/v2); a UTC RFC 3339 timestamp type that scans/values to TEXT; and a LocalDate type (YYYY-MM-DD) that rejects invalid dates. Tests cover commit, rollback on error, rollback on panic, ULID ordering, and timestamp/date round-trips through SQLite. Go quality floor passes. Evidence (developer-run, to be re-verified at review): gofmt -l . empty; go vet ./... clean; go test -race -count=1 ./... ok (internal/store ~1.12s); CGO_ENABLED=0 go build ./... ok; make test ok. Only approved dependency github.com/oklog/ulid/v2 v2.1.2 added. Base: feature/b9c21d67-...-sqlite-data-layer-and-schema (e9ee4e7).

dev_agent added 1 commit 2026-10-03 08:39:29 -04:00

Add store helpers: transactions, ULIDs and time conventions ... 59c3cf259d

- Store.WithTx runs fn on the writer, committing on nil and rolling back
  on error, panic or goroutine exit; panics propagate unchanged.
- NewID returns a monotonic ULID string (oklog/ulid/v2, crypto/rand),
  strictly increasing within the process.
- Timestamp stores UTC RFC 3339 TEXT at fixed millisecond precision.
- LocalDate stores YYYY-MM-DD TEXT and rejects invalid or non-canonical
  dates.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

review_agent reviewed 2026-10-03 08:41:57 -04:00

review_agent left a comment

Member

Copy Link

Copy Source

Independent review — Codex gpt-6-sol, reasoning high

Verdict: CHANGES_REQUIRED
Reviewed SHA: 59c3cf259dfd61907710d2bcf01da4d43ddc826e

{
  "verdict": "CHANGES_REQUIRED",
  "reviewed_sha": "59c3cf259dfd61907710d2bcf01da4d43ddc826e",
  "summary": "Transaction, ULID, and date behavior and their named tests check out. gofmt, go vet, and go test -race passed. The CGO-free build could not start because this read-only environment prevented Go from creating a temporary build directory. Timestamp scanning accepts an invalid RFC 3339 form, so the timestamp done-condition is unmet.",
  "findings": [
    {
      "id": "timestamp-noncanonical-scan",
      "severity": "medium",
      "location": "internal/store/time.go:46",
      "failure_case": "Scan accepts a timestamp with a comma before milliseconds, such as 2026-10-03T12:37:05,123Z, then Value writes it with a period. The input is not RFC 3339 TEXT.",
      "evidence": "Scan relies on time.Parse without checking that formatting the parsed value reproduces the input. Go's fractional-second parser accepts either a comma or a period. TestTimestampRejects does not cover this case.",
      "suggested_correction": "After parsing, require parsed.Format(timestampLayout) == s; add the comma form to the rejection test."
    }
  ]
}

---

Posted unaltered by review_agent. This is transport, not a second review.

## Independent review — Codex `gpt-6-sol`, reasoning high Verdict: **CHANGES_REQUIRED** Reviewed SHA: `59c3cf259dfd61907710d2bcf01da4d43ddc826e` ```json { "verdict": "CHANGES_REQUIRED", "reviewed_sha": "59c3cf259dfd61907710d2bcf01da4d43ddc826e", "summary": "Transaction, ULID, and date behavior and their named tests check out. gofmt, go vet, and go test -race passed. The CGO-free build could not start because this read-only environment prevented Go from creating a temporary build directory. Timestamp scanning accepts an invalid RFC 3339 form, so the timestamp done-condition is unmet.", "findings": [ { "id": "timestamp-noncanonical-scan", "severity": "medium", "location": "internal/store/time.go:46", "failure_case": "Scan accepts a timestamp with a comma before milliseconds, such as 2026-10-03T12:37:05,123Z, then Value writes it with a period. The input is not RFC 3339 TEXT.", "evidence": "Scan relies on time.Parse without checking that formatting the parsed value reproduces the input. Go's fractional-second parser accepts either a comma or a period. TestTimestampRejects does not cover this case.", "suggested_correction": "After parsing, require parsed.Format(timestampLayout) == s; add the comma form to the rejection test." } ] } ``` --- Posted unaltered by `review_agent`. This is transport, not a second review.

dev_agent added 1 commit 2026-10-03 08:42:36 -04:00

Reject non-canonical timestamps in Timestamp.Scan ... 5304887ce9

time.Parse accepts a comma before fractional seconds, so Scan took
"2026-10-03T12:37:05,123Z" and Value then wrote it back with a period.
Scan now requires the parsed instant to re-format byte-identically to
its input, and the rejection test covers the comma form.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

review_agent reviewed 2026-10-03 08:45:23 -04:00

review_agent left a comment

Member

Copy Link

Copy Source

Independent review — Codex gpt-6-sol, reasoning high

Verdict: CHANGES_REQUIRED
Reviewed SHA: 5304887ce9a37065015dd5a3ae60cc1855e1fc32

{
  "verdict": "CHANGES_REQUIRED",
  "reviewed_sha": "5304887ce9a37065015dd5a3ae60cc1855e1fc32",
  "summary": "The named transaction, ULID, timestamp, and date tests pass. Formatting, vet, race tests, and module verification pass. The CGO_ENABLED=0 build could not start because Go could not create a temporary directory on the read-only filesystem. One timestamp value can be written but cannot be scanned back.",
  "findings": [
    {
      "id": "F1",
      "severity": "medium",
      "location": "internal/store/time.go:33",
      "failure_case": "A Timestamp constructed with year 10000 writes TEXT that Timestamp.Scan rejects, breaking the required round-trip and UTC RFC 3339 representation.",
      "evidence": "Value checks only IsZero before formatting. Go formats year 10000 as five digits, while Scan parses a four-digit year using timestampLayout.",
      "suggested_correction": "Reject years outside the supported four-digit range in Value and add a test for that case."
    }
  ]
}

---

Posted unaltered by review_agent. This is transport, not a second review.

## Independent review — Codex `gpt-6-sol`, reasoning high Verdict: **CHANGES_REQUIRED** Reviewed SHA: `5304887ce9a37065015dd5a3ae60cc1855e1fc32` ```json { "verdict": "CHANGES_REQUIRED", "reviewed_sha": "5304887ce9a37065015dd5a3ae60cc1855e1fc32", "summary": "The named transaction, ULID, timestamp, and date tests pass. Formatting, vet, race tests, and module verification pass. The CGO_ENABLED=0 build could not start because Go could not create a temporary directory on the read-only filesystem. One timestamp value can be written but cannot be scanned back.", "findings": [ { "id": "F1", "severity": "medium", "location": "internal/store/time.go:33", "failure_case": "A Timestamp constructed with year 10000 writes TEXT that Timestamp.Scan rejects, breaking the required round-trip and UTC RFC 3339 representation.", "evidence": "Value checks only IsZero before formatting. Go formats year 10000 as five digits, while Scan parses a four-digit year using timestampLayout.", "suggested_correction": "Reject years outside the supported four-digit range in Value and add a test for that case." } ] } ``` --- Posted unaltered by `review_agent`. This is transport, not a second review.

dev_agent added 1 commit 2026-10-03 08:46:07 -04:00

Reject Timestamp years outside 0000-9999 in Value ... e99ddec433

The four-digit stored form cannot represent them, so Value would emit
TEXT that Scan rejects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

review_agent reviewed 2026-10-03 08:51:56 -04:00

review_agent left a comment

Member

Copy Link

Copy Source

Independent review — Codex gpt-6-sol, reasoning high

Verdict: CHANGES_REQUIRED
Reviewed SHA: e99ddec4331b988c724b550df01e13696c189602

{
  "verdict": "CHANGES_REQUIRED",
  "reviewed_sha": "e99ddec4331b988c724b550df01e13696c189602",
  "summary": "The named tests pass, as do gofmt and go vet. The CGO-disabled build could not run because the read-only environment prevents Go from creating a temporary work directory. One timestamp scan inconsistency remains.",
  "findings": [
    {
      "id": "F1",
      "severity": "medium",
      "location": "internal/store/time.go:50-58",
      "failure_case": "Scanning \"0001-01-01T00:00:00.000Z\" succeeds and produces a zero Timestamp, but writing that value back fails.",
      "evidence": "Scan parses and assigns the canonical text without checking IsZero; Value explicitly rejects a zero Timestamp at lines 35-37.",
      "suggested_correction": "Reject a parsed zero Timestamp in Scan, consistent with Value, and add a test for this input."
    }
  ]
}

---

Posted unaltered by review_agent. This is transport, not a second review.

## Independent review — Codex `gpt-6-sol`, reasoning high Verdict: **CHANGES_REQUIRED** Reviewed SHA: `e99ddec4331b988c724b550df01e13696c189602` ```json { "verdict": "CHANGES_REQUIRED", "reviewed_sha": "e99ddec4331b988c724b550df01e13696c189602", "summary": "The named tests pass, as do gofmt and go vet. The CGO-disabled build could not run because the read-only environment prevents Go from creating a temporary work directory. One timestamp scan inconsistency remains.", "findings": [ { "id": "F1", "severity": "medium", "location": "internal/store/time.go:50-58", "failure_case": "Scanning \"0001-01-01T00:00:00.000Z\" succeeds and produces a zero Timestamp, but writing that value back fails.", "evidence": "Scan parses and assigns the canonical text without checking IsZero; Value explicitly rejects a zero Timestamp at lines 35-37.", "suggested_correction": "Reject a parsed zero Timestamp in Scan, consistent with Value, and add a test for this input." } ] } ``` --- Posted unaltered by `review_agent`. This is transport, not a second review.

dev_agent added 1 commit 2026-10-03 08:52:32 -04:00

Reject the zero instant in Timestamp.Scan ... 5de7a0df15

Scan accepted "0001-01-01T00:00:00.000Z" and produced a zero Timestamp
that Value then refused to write. Reject it on read, consistent with Value.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

review_agent approved these changes 2026-10-03 08:54:31 -04:00

review_agent left a comment

Member

Copy Link

Copy Source

Independent review — Codex gpt-6-sol, reasoning high

Verdict: APPROVED
Reviewed SHA: 5de7a0df15e208c9306bf7c6b23a56f2878704c8

{
  "verdict": "APPROVED",
  "reviewed_sha": "5de7a0df15e208c9306bf7c6b23a56f2878704c8",
  "summary": "The helpers and tests meet the stated done-condition. The rollback tests verify persisted rows, and the panic test verifies that the same value propagates. gofmt, go vet, race tests, and go mod verify passed. The CGO_ENABLED=0 build could not run because the read-only sandbox prevented Go from creating a temporary build directory.",
  "findings": []
}

---

Posted unaltered by review_agent. This is transport, not a second review.

## Independent review — Codex `gpt-6-sol`, reasoning high Verdict: **APPROVED** Reviewed SHA: `5de7a0df15e208c9306bf7c6b23a56f2878704c8` ```json { "verdict": "APPROVED", "reviewed_sha": "5de7a0df15e208c9306bf7c6b23a56f2878704c8", "summary": "The helpers and tests meet the stated done-condition. The rollback tests verify persisted rows, and the panic test verifies that the same value propagates. gofmt, go vet, race tests, and go mod verify passed. The CGO_ENABLED=0 build could not run because the read-only sandbox prevented Go from creating a temporary build directory.", "findings": [] } ``` --- Posted unaltered by `review_agent`. This is transport, not a second review.

approver_agent merged commit b06d1680a3 into feature/b9c21d67-bf72-411a-901d-77e57352d398-sqlite-data-layer-and-schema 2026-10-03 08:54:56 -04:00

dev_agent referenced this pull request 2026-10-03 09:16:09 -04:00

Feature 02: SQLite Data Layer and Schema #14

Sign in to join this conversation.

Reviewers

No Reviewers

review_agent

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

approver_agent (The Approver) chatgpt claude-ai Claude-Code codex (Codex) dev_agent grok opencode orchestrator-agent overseer (Joshua) review_agent (The Reviewer)

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Maniacs_Mansion/MealQuest#11