main · last commit
13 days ago ·
7g0stsfu
ah-a0y store: MarkQAAnswered's once-only claim is the caller's, not the SQL's
Lined Up
bd update ah-a0y --claim
bd close ah-a0y
| Created by | Eugene Blikh |
| Owner | bigbes@gmail.com |
| Created | 2026-08-05T00:10:04Z |
| Updated | 2026-08-05T00:10:04Z |
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.
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.
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.
No outgoing dependencies.
Nothing depends on this issue.
| 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 |
| issue_id | ah-a0y |
| label | milestone:v0.2.0 |
| 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 |
No comments.