Ticket loads its comments one time as typed Comment objects #1633

Closed
opened 2026-11-07 21:41:44 +00:00 by jared · 3 comments
Owner

Problem

Ticket#comments sends a new GET /issues/{n}/comments on each call and returns Array<String>. The hosts discard the id, user, and created_at that Forgejo sends. Each comment kind is then found with its own string check, and a claim has no age, so code cannot tell a stale claim from a live one.

Kind Site Check today
Bounced lib/backlog/ticket/claiming.rb:43 body.include?("## Bounced")
Resolution lib/backlog/claim.rb:52, lib/backlog/resolution.rb include?(Resolution::HEADING)
Needs review lib/backlog/ticket.rb request_review include?("Needs review: ...")
Decision sweep failure lib/backlog/decision_sweep/gate_application.rb:31 include?(FAILURE_HEADING)
Start (claim) lib/backlog/ticket/claiming.rb:46 written, never read
# lib/backlog/host/forgejo.rb:45
def comments(number)
  on_repo { |repo| repo.comments.list_for_issue(Integer(number)).to_a }.map(&:body)
end

One ticket claim that bounces sends two comment requests (claim.rb:29 and tracker.rb:58).

Reproduce

n/a: design change; the Problem cites each site.

Approach

class Ticket
  def comments    = @comments ||= @tracker.comments(number).map { Comment.build(it) }
  def claims      = comments.grep(Comment::Claim)
  def resolutions = comments.grep(Comment::Resolution)
  def claimed?    = comments.grep(Comment::ClaimEvent).last.is_a?(Comment::Claim)
end
  • Each host returns comment records with id, author, created_at, and body. GET /issues/{n}/comments has no paging (server spec: only since and before), so one request returns all comments.
  • Comment.build reads the type from one data block in the body, parsed with YAML.safe_load. A comment with an unknown kind becomes Comment::Note.
## Start
probe-7 claimed this ticket.

<!-- os-backlog
kind: claim
agent_id: probe-7
session_id: 241a8fe6-1aa7-44b4-b9ff-4b38d4bc9515
implementation_id: 261
-->
  • Old comments have no data block. Read both formats (the data block, else the heading). Write only the new format.
  • Comment::Claim carries agent_id, session_id, implementation_id (when os-sdlc runs the work), and claimed_at, which is the comment created_at. The caller decides what to do with an existing claim. The code has no staleness rule.
  • Comment::Claim is the comment value. Rename the checkout procedure Backlog::Claim (lib/backlog/claim.rb) to make the name free.
  • save! adds the posted comments to @comments or clears the cache, so the duplicate-post protection in tracker.rb:58 still holds.
  • Known limit: claim unless claimed? is not atomic. No lock: a survey of 3,063 comments found no double claim.

Required work: none beyond the acceptance criteria.

Orchestrator work: the data block is vocabulary that other code parses; run /os-adr:create before the build.

Rejected: staleness checks that look up the session or implementation; no double claim has been seen, so the code would handle a case that does not occur.

Acceptance criteria

  1. Given a ticket, when code reads comments two times in one command, then the host gets one comments request.
  2. Given an old ## Resolution comment with no data block, when the ticket loads, then ticket.resolutions contains it.
  3. Given a claim made with ticket claim --agent-id probe-7, when the ticket loads, then ticket.claims.last has agent_id "probe-7", a session_id, and a claimed_at equal to the comment created_at.
  4. Given a ticket with a claim comment and then a bounce comment, when the ticket loads, then claimed? is false.
  5. Given the os-backlog lib, when it is searched, then no comment kind is found with include? or start_with? on a heading.

Owner sketch

module Backlog::Comment
  def self.build(record)
end

class Backlog::Comment::Claim
  def agent_id; def session_id; def implementation_id; def claimed_at
end

class Backlog::Ticket
  def comments; def claims; def resolutions; def claimed?
end

Origin

  • Trigger: review of #335 on 2026-11-07; a survey of 3,063 jared/cc-os comments found 0 claim comments, so #335 closes and this ticket replaces it
  • Improvised this session: none
  • Chain: no claim age and repeated comment requests ← comments reduced to strings at the host ← DESIGN (lib/backlog/host/forgejo.rb:45, lib/backlog/ticket.rb comments)
  • Root candidate: none found; searched comments, Comment, claim, claimed, Resolution, Bounced, include?, created_at, host/forgejo comments, typed comment objects
  • Where: plugins/os-backlog/lib/backlog/ticket.rb, plugins/os-backlog/lib/backlog/host/forgejo.rb
  • Session: 241a8fe6-1aa7-44b4-b9ff-4b38d4bc9515
  • Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/241a8fe6-1aa7-44b4-b9ff-4b38d4bc9515.jsonl

VERDICT: CREATE

## Problem `Ticket#comments` sends a new `GET /issues/{n}/comments` on each call and returns `Array<String>`. The hosts discard the `id`, `user`, and `created_at` that Forgejo sends. Each comment kind is then found with its own string check, and a claim has no age, so code cannot tell a stale claim from a live one. | Kind | Site | Check today | | --- | --- | --- | | Bounced | `lib/backlog/ticket/claiming.rb:43` | `body.include?("## Bounced")` | | Resolution | `lib/backlog/claim.rb:52`, `lib/backlog/resolution.rb` | `include?(Resolution::HEADING)` | | Needs review | `lib/backlog/ticket.rb` `request_review` | `include?("Needs review: ...")` | | Decision sweep failure | `lib/backlog/decision_sweep/gate_application.rb:31` | `include?(FAILURE_HEADING)` | | Start (claim) | `lib/backlog/ticket/claiming.rb:46` | written, never read | ```ruby # lib/backlog/host/forgejo.rb:45 def comments(number) on_repo { |repo| repo.comments.list_for_issue(Integer(number)).to_a }.map(&:body) end ``` One `ticket claim` that bounces sends two comment requests (`claim.rb:29` and `tracker.rb:58`). ## Reproduce n/a: design change; the Problem cites each site. ## Approach ```ruby class Ticket def comments = @comments ||= @tracker.comments(number).map { Comment.build(it) } def claims = comments.grep(Comment::Claim) def resolutions = comments.grep(Comment::Resolution) def claimed? = comments.grep(Comment::ClaimEvent).last.is_a?(Comment::Claim) end ``` - Each host returns comment records with `id`, `author`, `created_at`, and `body`. `GET /issues/{n}/comments` has no paging (server spec: only `since` and `before`), so one request returns all comments. - `Comment.build` reads the type from one data block in the body, parsed with `YAML.safe_load`. A comment with an unknown kind becomes `Comment::Note`. ```markdown ## Start probe-7 claimed this ticket. <!-- os-backlog kind: claim agent_id: probe-7 session_id: 241a8fe6-1aa7-44b4-b9ff-4b38d4bc9515 implementation_id: 261 --> ``` - Old comments have no data block. Read both formats (the data block, else the heading). Write only the new format. - `Comment::Claim` carries `agent_id`, `session_id`, `implementation_id` (when os-sdlc runs the work), and `claimed_at`, which is the comment `created_at`. The caller decides what to do with an existing claim. The code has no staleness rule. - `Comment::Claim` is the comment value. Rename the checkout procedure `Backlog::Claim` (`lib/backlog/claim.rb`) to make the name free. - `save!` adds the posted comments to `@comments` or clears the cache, so the duplicate-post protection in `tracker.rb:58` still holds. - Known limit: `claim unless claimed?` is not atomic. No lock: a survey of 3,063 comments found no double claim. Required work: none beyond the acceptance criteria. Orchestrator work: the data block is vocabulary that other code parses; run `/os-adr:create` before the build. Rejected: staleness checks that look up the session or implementation; no double claim has been seen, so the code would handle a case that does not occur. ## Acceptance criteria 1. Given a ticket, when code reads `comments` two times in one command, then the host gets one comments request. 2. Given an old `## Resolution` comment with no data block, when the ticket loads, then `ticket.resolutions` contains it. 3. Given a claim made with `ticket claim --agent-id probe-7`, when the ticket loads, then `ticket.claims.last` has `agent_id` "probe-7", a `session_id`, and a `claimed_at` equal to the comment `created_at`. 4. Given a ticket with a claim comment and then a bounce comment, when the ticket loads, then `claimed?` is false. 5. Given the os-backlog lib, when it is searched, then no comment kind is found with `include?` or `start_with?` on a heading. ## Owner sketch ```ruby module Backlog::Comment def self.build(record) end class Backlog::Comment::Claim def agent_id; def session_id; def implementation_id; def claimed_at end class Backlog::Ticket def comments; def claims; def resolutions; def claimed? end ``` ## Origin - Trigger: review of #335 on 2026-11-07; a survey of 3,063 jared/cc-os comments found 0 claim comments, so #335 closes and this ticket replaces it - Improvised this session: none - Chain: no claim age and repeated comment requests ← comments reduced to strings at the host ← DESIGN (`lib/backlog/host/forgejo.rb:45`, `lib/backlog/ticket.rb` `comments`) - Root candidate: none found; searched comments, Comment, claim, claimed, Resolution, Bounced, include?, created_at, host/forgejo comments, typed comment objects - Where: plugins/os-backlog/lib/backlog/ticket.rb, plugins/os-backlog/lib/backlog/host/forgejo.rb - Session: 241a8fe6-1aa7-44b4-b9ff-4b38d4bc9515 - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/241a8fe6-1aa7-44b4-b9ff-4b38d4bc9515.jsonl VERDICT: CREATE
Author
Owner

ADR-0308 records the comment data block format (docs/adr/0308-os-backlog-comments-that-code-reads-carry-one-machine-data-block.md). Orchestrator work done.

ADR-0308 records the comment data block format (docs/adr/0308-os-backlog-comments-that-code-reads-carry-one-machine-data-block.md). Orchestrator work done.
Author
Owner

Start: os-sdlc implement on branch ticket-1633 (session 241a8fe6-1aa7-44b4-b9ff-4b38d4bc9515).

Start: os-sdlc implement on branch ticket-1633 (session 241a8fe6-1aa7-44b4-b9ff-4b38d4bc9515).
Author
Owner

Resolution

Done: Ticket#comments loads one time per ticket as typed Comment objects (Claim, Bounce, Resolution, Review, GateFailure, Note) from host Comment::Records with id, author, created_at (Time). All writers append the ADR-0308 data block; kind detection is by class with a legacy-heading fallback. Claims carry agent_id and session_id (CLAUDE_CODE_SESSION_ID). Backlog::Claim renamed Backlog::Pickup. Commit 0255bcc0 on main.

Evidence: os-backlog suite 551 runs 0 failures (3 runs in worktree, 1 on main); os-sdlc suite 3510 runs 0 failures; rubocop total 35 = main baseline, none in touched files; grep finds no heading check for a comment kind in lib (only Ticket::Body body checks); live read of #335 from Forgejo gives Note/Note/Resolution with author jared and Time created_at; one design audit, 1 must-fix + 4 should-fix fixed. Pipeline run 262 escalated (120) and was hand-finished, then abandoned.

Follow-ups: Dropped: a writer for Claim#implementation_id (no os-sdlc caller of ticket claim yet; reader returns nil until one exists). Dropped: moving Backlog::Pickup to a Checkout root (audit note only; no defect). Already filed outside this repo: jared/forgejo-api#3 (repo-wide comment list stops after page 1).

## Resolution **Done:** Ticket#comments loads one time per ticket as typed Comment objects (Claim, Bounce, Resolution, Review, GateFailure, Note) from host Comment::Records with id, author, created_at (Time). All writers append the ADR-0308 data block; kind detection is by class with a legacy-heading fallback. Claims carry agent_id and session_id (CLAUDE_CODE_SESSION_ID). Backlog::Claim renamed Backlog::Pickup. Commit 0255bcc0 on main. **Evidence:** os-backlog suite 551 runs 0 failures (3 runs in worktree, 1 on main); os-sdlc suite 3510 runs 0 failures; rubocop total 35 = main baseline, none in touched files; grep finds no heading check for a comment kind in lib (only Ticket::Body body checks); live read of #335 from Forgejo gives Note/Note/Resolution with author jared and Time created_at; one design audit, 1 must-fix + 4 should-fix fixed. Pipeline run 262 escalated (120) and was hand-finished, then abandoned. **Follow-ups:** Dropped: a writer for Claim#implementation_id (no os-sdlc caller of ticket claim yet; reader returns nil until one exists). Dropped: moving Backlog::Pickup to a Checkout root (audit note only; no defect). Already filed outside this repo: jared/forgejo-api#3 (repo-wide comment list stops after page 1). <!-- os-backlog kind: resolution -->
jared closed this issue 2026-11-07 22:18:29 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
jared/cc-os#1633
No description provided.