~bigbes/agents-dev · parade

main · last commit 13 days ago · 7g0stsfu

← Back to the parade

ah-a0y store: MarkQAAnswered's once-only claim is the caller's, not the SQL's Lined Up

status: open P4 bug milestone:v0.2.0
bd update ah-a0y --claim bd close ah-a0y
Created byEugene Blikh
Ownerbigbes@gmail.com
Created2026-08-05T00:10:04Z
Updated2026-08-05T00:10:04Z
Description
Found while refreshing SPEC §15/§7 (ah-e5l).

internal/store/store.go MarkQAAnswered's docstring says it stamps answered_at 'making the answer delivery once-only'. The SQL is an unconditional 'UPDATE yonote_qa SET answered_at = ? WHERE task_id = ?' with no 'AND answered_at IS NULL'. The once-only property actually comes from the CALLER: internal/reconcile/qa.go:385 guards with 'if q == nil || q.AnsweredAt != nil { ... }'. SPEC §6 repeats the same overstatement.

Low severity today — the single reconcile goroutine makes the caller-side guard sufficient — but a second caller (a retry path, a manual re-deliver, the mid-run tools channel) would silently break it and post a duplicate Yonote reply under the bot identity.
Design
Deciding between the two fixes is the actual work, and they are not equivalent.

(a) Make the SQL the enforcement point: add 'AND answered_at IS NULL'. This is stronger — a future second caller cannot break it — but it collides with the method's OTHER documented contract: 'Idempotent: re-marking refreshes the stamp on the same row; a missing row is an error (the caller just read it)'. With the guard added, a re-mark affects 0 rows, which the existing n==0 branch reports as 'no qa row' — wrong, the row exists. So (a) also requires deciding what a re-mark means (a no-op returning nil is the natural once-only semantics) and distinguishing 'already answered' from 'row absent', e.g. with a follow-up existence probe or RETURNING.

(b) Fix the docstring (and SPEC §6) to state the real contract: the stamp is a RECORD, and once-only comes from the caller's read-then-guard.

(a) is the better end state. Do it in a change that also touches reconcile/qa.go, so the caller's guard and the store's guarantee are decided together — this was deliberately not done on 2026-08-05 because internal/reconcile was mid-edit by another agent.
Acceptance criteria
MarkQAAnswered's doc comment, SPEC §6 and the SQL all describe the same contract. If the SQL becomes the enforcement point, a second MarkQAAnswered call on an answered row is a no-op that does not report a missing row, and a genuinely missing row still errors. Covered by a store test.

Depends on

No outgoing dependencies.

Depended on by

Nothing depends on this issue.

No comments.

  • Eugene Blikh created the issue · 2026-08-05T03:10:03Z
  • Eugene Blikh added label milestone:v0.2.0 · 2026-08-05T03:24:04Z
Stored rows — what this pane was built from, as read
issues 1 row
id ah-a0y
content_hash 1f451cf42cd3f15fe24298bebe9e07a8aa2bcd7c5ec4d3bcd9e538a26d33a3fb
title store: MarkQAAnswered's once-only claim is the caller's, not the SQL's
description Found while refreshing SPEC §15/§7 (ah-e5l). internal/store/store.go MarkQAAnswered's docstring says it stamps answered_at 'making the answer delivery once-only'. The SQL is an unconditional 'UPDATE yonote_qa SET answered_at = ? WHERE task_id = ?' with no 'AND answered_at IS NULL'. The once-only property actually comes from the CALLER: internal/reconcile/qa.go:385 guards with 'if q == nil || q.AnsweredAt != nil { ... }'. SPEC §6 repeats the same overstatement. Low severity today — the single reconcile goroutine makes the caller-side guard sufficient — but a second caller (a retry path, a manual re-deliver, the mid-run tools channel) would silently break it and post a duplicate Yonote reply under the bot identity.
design Deciding between the two fixes is the actual work, and they are not equivalent. (a) Make the SQL the enforcement point: add 'AND answered_at IS NULL'. This is stronger — a future second caller cannot break it — but it collides with the method's OTHER documented contract: 'Idempotent: re-marking refreshes the stamp on the same row; a missing row is an error (the caller just read it)'. With the guard added, a re-mark affects 0 rows, which the existing n==0 branch reports as 'no qa row' — wrong, the row exists. So (a) also requires deciding what a re-mark means (a no-op returning nil is the natural once-only semantics) and distinguishing 'already answered' from 'row absent', e.g. with a follow-up existence probe or RETURNING. (b) Fix the docstring (and SPEC §6) to state the real contract: the stamp is a RECORD, and once-only comes from the caller's read-then-guard. (a) is the better end state. Do it in a change that also touches reconcile/qa.go, so the caller's guard and the store's guarantee are decided together — this was deliberately not done on 2026-08-05 because internal/reconcile was mid-edit by another agent.
acceptance_criteria MarkQAAnswered's doc comment, SPEC §6 and the SQL all describe the same contract. If the SQL becomes the enforcement point, a second MarkQAAnswered call on an answered row is a no-op that does not report a missing row, and a genuinely missing row still errors. Covered by a store test.
notes
status open
priority 4
issue_type bug
assignee NULL
estimated_minutes NULL
created_at 2026-08-05T00:10:04Z
created_by Eugene Blikh
owner bigbes@gmail.com
updated_at 2026-08-05T00:10:04Z
closed_at NULL
closed_by_session
external_ref NULL
spec_id
compaction_level 0
compacted_at NULL
compacted_at_commit NULL
original_size NULL
sender
ephemeral 0
wisp_type
pinned 0
is_template 0
mol_type
work_type
source_system
metadata �{}
source_repo
close_reason
event_kind
actor
target
payload
await_type
await_id
timeout_ns 0
waiters
hook_bead
role_bead
agent_state
last_activity NULL
role_type
rig
due_at NULL
defer_until NULL
no_history 0
started_at NULL
is_blocked 0
labels 1 row
issue_id ah-a0y
label milestone:v0.2.0
events 2 rows
id 019fcf41-7ede-760e-bcf7-8b75f5439616
issue_id ah-a0y
event_type created
actor Eugene Blikh
old_value
new_value
comment NULL
created_at 2026-08-05T03:10:03Z
id 019fcf4e-5147-78b2-a8f8-6caeea964bf4
issue_id ah-a0y
event_type label_added
actor Eugene Blikh
old_value NULL
new_value NULL
comment Added label: milestone:v0.2.0
created_at 2026-08-05T03:24:04Z