markdown · 9188 bytes Raw Blame History

Issues, comments, labels, milestones

S21 ships the issues subsystem: issues + threaded comments, labels, milestones, the polymorphic timeline event log, and the cross-reference index. The same tables are reused by S22 for pull requests via the issue_kind discriminator.

Schema (migration 0022)

repo_issue_counter   — per-repo monotonic numbering (issues + PRs share)
issues               — the row; kind discriminator covers PR rows in S22
issue_comments       — per-issue threaded comments
issue_assignees      — many-to-many issue ↔ user
labels               — repo-scoped labels
issue_labels         — many-to-many issue ↔ label
milestones           — repo-scoped milestones
issue_events         — generic timeline events (closed, locked, labeled, …)
issue_references     — cross-reference index (comment / issue / commit → issue)

The kind column on issues is in from day one ('issue' | 'pr') so S22's PR rows are first-class without a schema split. PR-specific fields land in a pull_requests table keyed on issues.id.

Per-repo numbering

repo_issue_counter (repo_id, next_number) is allocated lazily at first issue creation and on every repo create (S16's repos.Create calls EnsureRepoIssueCounter inside the create tx). Allocation is one UPDATE … RETURNING inside the same tx as the issue insert:

UPDATE repo_issue_counter
SET next_number = next_number + 1
WHERE repo_id = $1
RETURNING (next_number - 1)::bigint AS allocated;

Postgres serializes concurrent updaters on the row lock, so concurrent issues.Create calls on the same repo end up with distinct numbers. The race test (TestCreate_ConcurrentRaceForUniqueNumbers) fans out 8 goroutines and asserts no duplicates.

Default labels

Repo create seeds the GitHub-aligned set:

bug · d73a4a · Something isn't working
documentation · 0075ca · Improvements or additions to documentation
duplicate · cfd3d7 · This issue or pull request already exists
enhancement · a2eeef · New feature or request
good first issue · 7057ff · Good for newcomers
help wanted · 008672 · Extra attention is needed
invalid · e4e669 · This doesn't seem right
question · d876e3 · Further information is requested
wontfix · ffffff · This will not be worked on

Seeding runs inside the repo-create transaction via issues.SeedDefaultLabels(ctx, tx, repoID). It swallows unique-violations so re-runs are no-ops; backfill for repos created before S21 is a one-off SQL INSERT … ON CONFLICT DO NOTHING.

Routes

Route Auth
GET /{owner}/{repo}/issues?state=open|closed public
GET /{owner}/{repo}/issues/{number} public
GET /{owner}/{repo}/labels public
GET /{owner}/{repo}/milestones public
GET /{owner}/{repo}/issues/new RequireUser
POST /{owner}/{repo}/issues RequireUser
POST /{owner}/{repo}/issues/{number}/comments RequireUser
POST /{owner}/{repo}/issues/{number}/state RequireUser
POST /{owner}/{repo}/issues/{number}/lock RequireUser
POST /{owner}/{repo}/issues/{number}/labels RequireUser
POST /{owner}/{repo}/issues/{number}/milestone RequireUser
POST /{owner}/{repo}/issues/{number}/assignees RequireUser
POST /{owner}/{repo}/labels + /{id}/update + /{id}/delete RequireUser
POST /{owner}/{repo}/milestones + /{id}/update RequireUser
POST /{owner}/{repo}/milestones/{id}/state + /{id}/delete RequireUser

Public reads still pass through policy.Can(ActionIssueRead) so private repos return the existence-leak 404. RequireUser on writes gives anonymous browsers a /login redirect instead of the same 404. Issue creation and commenting follow GitHub's public-participation model: any logged-in user may open or comment on issues in a public repo, while private repos require read access.

The issue page is capability-driven:

  • The comment box appears only when policy.Can(issue:comment) allows and the issue is not locked, unless the viewer is triage+.
  • Close/reopen appears for triage+ users and for the issue author.
  • Labels, assignees, milestones, and lock controls appear only for the matching triage-level policy action. Public participants should not see forms that only lead to 403s.

Cross-reference indexing

internal/issues/references.go::insertReferencesFromBody parses #N and owner/repo#N from the comment / issue body inside the creating transaction. Each parsed reference produces:

  1. an issue_references row pointing source → target,
  2. a referenced event emitted on the target issue's timeline.

The cross-repo regex runs first; matched ranges are stripped before the same-repo regex sweeps so alice/proj#3 doesn't also trip the #3 matcher. Self-references skip silently. Unknown owners / repos / issue numbers are best-effort dropped — malformed references shouldn't fail the create.

Commit-message references (S14's push:process) call into the same helper with srcKind = "commit_message" and srcObjectID = push_event_id once that worker job ships.

Markdown + sanitization

Issue bodies and comments use the existing S17 markdown pipeline (internal/repos/markdown.RenderHTML): Goldmark for parsing, bluemonday's UGCPolicy for sanitization. Rendered HTML is cached on body_html_cached; md_pipeline_version lets the worker re-render on pipeline upgrades (post-MVP).

<script> tags are stripped — the sanitization test (TestCreate_RendersHTMLAndSanitizesScripts) injects one and asserts the cached HTML doesn't contain <script while still rendering **bold** as <strong>bold</strong>.

Locked issues

SetLock flips locked + emits a locked/unlocked event. Comments hit the locked gate inside AddComment: non-collaborators get ErrIssueLocked. The IsCollab flag is the caller's responsibility; the web handler resolves it via policy.HasRoleAtLeast(..., RoleTriage), so owners, direct collaborators, and team-derived triage+ roles can comment through a lock.

Rate limit

Comment creation hits the existing throttle bucket (scope=issue_comment, identifier=user:N) at 20 per hour. Issue creation isn't independently rate-limited; the broader repo_create bucket and account anti-abuse measures cover the spam-floor case for now.

Timeline events

issue_events is intentionally polymorphic: kind is a free string, meta is a jsonb blob, and ref_target_id is a denormalized pointer for events that target another issue (referenced, marked_as_duplicate_of, etc.). Known kinds today:

closed, reopened, locked, unlocked,
labeled, unlabeled,
assigned, unassigned,
milestoned, demilestoned,
referenced

meta always carries an empty '{}'::jsonb when no metadata is applicable — Go's nil byte slice would bind as SQL NULL and trip the NOT NULL constraint, so the orchestrator passes []byte("{}") explicitly.

Tests

File What it covers
internal/issues/references_test.go regex behaviour for #N and owner/repo#N
internal/issues/issues_test.go::Sequential… per-repo number allocation
internal/issues/issues_test.go::ConcurrentRace… 8-goroutine fan-out, no duplicates
internal/issues/issues_test.go::Sanitizes… XSS + markdown render
internal/issues/issues_test.go::LockedRejects… locked-issue gate for non-collab
internal/issues/issues_test.go::EmitsEvent close → reopen event ordering
internal/issues/issues_test.go::CrossRef… #N in body emits referenced on target

Deferred

  • Mention notificationsextractMentions is in place, but the notifications surface (S29-area) hasn't shipped, so parsed @username tokens currently don't emit anything.
  • Commit-message references — the helper accepts srcKind = "commit_message" already; wiring lives with S14's worker job (push:process).
  • Cross-org references — the cross-repo regex resolves only user-owned repos; org owners arrive in S31.
  • Required signing / status checks on comments — out of scope.
  • Reactions, edit history, comment edits across users — post-MVP UI polish (S30+).
  • Server-side label / assignee / milestone filtering — current list query filters by state + kind only; richer filtering is the perf-pass item in S36.
  • Issue templates / forms — post-MVP.
  • Reactions — post-MVP (own table; doesn't touch the schema here).
View source
1 # Issues, comments, labels, milestones
2
3 S21 ships the issues subsystem: issues + threaded comments, labels,
4 milestones, the polymorphic timeline event log, and the cross-reference
5 index. The same tables are reused by S22 for pull requests via the
6 `issue_kind` discriminator.
7
8 ## Schema (migration 0022)
9
10 ```
11 repo_issue_counter — per-repo monotonic numbering (issues + PRs share)
12 issues — the row; kind discriminator covers PR rows in S22
13 issue_comments — per-issue threaded comments
14 issue_assignees — many-to-many issue ↔ user
15 labels — repo-scoped labels
16 issue_labels — many-to-many issue ↔ label
17 milestones — repo-scoped milestones
18 issue_events — generic timeline events (closed, locked, labeled, …)
19 issue_references — cross-reference index (comment / issue / commit → issue)
20 ```
21
22 The `kind` column on `issues` is in from day one (`'issue' | 'pr'`) so
23 S22's PR rows are first-class without a schema split. PR-specific
24 fields land in a `pull_requests` table keyed on `issues.id`.
25
26 ### Per-repo numbering
27
28 `repo_issue_counter (repo_id, next_number)` is allocated lazily at first
29 issue creation and on every repo create (S16's `repos.Create` calls
30 `EnsureRepoIssueCounter` inside the create tx). Allocation is one
31 `UPDATE … RETURNING` inside the same tx as the issue insert:
32
33 ```sql
34 UPDATE repo_issue_counter
35 SET next_number = next_number + 1
36 WHERE repo_id = $1
37 RETURNING (next_number - 1)::bigint AS allocated;
38 ```
39
40 Postgres serializes concurrent updaters on the row lock, so concurrent
41 `issues.Create` calls on the same repo end up with distinct numbers.
42 The race test (`TestCreate_ConcurrentRaceForUniqueNumbers`) fans out 8
43 goroutines and asserts no duplicates.
44
45 ### Default labels
46
47 Repo create seeds the GitHub-aligned set:
48
49 ```
50 bug · d73a4a · Something isn't working
51 documentation · 0075ca · Improvements or additions to documentation
52 duplicate · cfd3d7 · This issue or pull request already exists
53 enhancement · a2eeef · New feature or request
54 good first issue · 7057ff · Good for newcomers
55 help wanted · 008672 · Extra attention is needed
56 invalid · e4e669 · This doesn't seem right
57 question · d876e3 · Further information is requested
58 wontfix · ffffff · This will not be worked on
59 ```
60
61 Seeding runs inside the repo-create transaction via
62 `issues.SeedDefaultLabels(ctx, tx, repoID)`. It swallows unique-violations
63 so re-runs are no-ops; backfill for repos created before S21 is a one-off
64 SQL `INSERT … ON CONFLICT DO NOTHING`.
65
66 ## Routes
67
68 | Route | Auth |
69 | -------------------------------------------------------------- | ------------- |
70 | `GET /{owner}/{repo}/issues?state=open\|closed` | public |
71 | `GET /{owner}/{repo}/issues/{number}` | public |
72 | `GET /{owner}/{repo}/labels` | public |
73 | `GET /{owner}/{repo}/milestones` | public |
74 | `GET /{owner}/{repo}/issues/new` | RequireUser |
75 | `POST /{owner}/{repo}/issues` | RequireUser |
76 | `POST /{owner}/{repo}/issues/{number}/comments` | RequireUser |
77 | `POST /{owner}/{repo}/issues/{number}/state` | RequireUser |
78 | `POST /{owner}/{repo}/issues/{number}/lock` | RequireUser |
79 | `POST /{owner}/{repo}/issues/{number}/labels` | RequireUser |
80 | `POST /{owner}/{repo}/issues/{number}/milestone` | RequireUser |
81 | `POST /{owner}/{repo}/issues/{number}/assignees` | RequireUser |
82 | `POST /{owner}/{repo}/labels` + `/{id}/update` + `/{id}/delete` | RequireUser |
83 | `POST /{owner}/{repo}/milestones` + `/{id}/update` | RequireUser |
84 | `POST /{owner}/{repo}/milestones/{id}/state` + `/{id}/delete` | RequireUser |
85
86 Public reads still pass through `policy.Can(ActionIssueRead)` so
87 private repos return the existence-leak 404. RequireUser on writes
88 gives anonymous browsers a `/login` redirect instead of the same 404.
89 Issue creation and commenting follow GitHub's public-participation
90 model: any logged-in user may open or comment on issues in a public
91 repo, while private repos require `read` access.
92
93 The issue page is capability-driven:
94
95 - The comment box appears only when `policy.Can(issue:comment)` allows
96 and the issue is not locked, unless the viewer is triage+.
97 - Close/reopen appears for triage+ users and for the issue author.
98 - Labels, assignees, milestones, and lock controls appear only for the
99 matching triage-level policy action. Public participants should not
100 see forms that only lead to 403s.
101
102 ## Cross-reference indexing
103
104 `internal/issues/references.go::insertReferencesFromBody` parses
105 `#N` and `owner/repo#N` from the comment / issue body inside the
106 creating transaction. Each parsed reference produces:
107
108 1. an `issue_references` row pointing source → target,
109 2. a `referenced` event emitted on the *target* issue's timeline.
110
111 The cross-repo regex runs first; matched ranges are stripped before
112 the same-repo regex sweeps so `alice/proj#3` doesn't also trip the
113 `#3` matcher. Self-references skip silently. Unknown owners / repos /
114 issue numbers are best-effort dropped — malformed references shouldn't
115 fail the create.
116
117 Commit-message references (S14's `push:process`) call into the same
118 helper with `srcKind = "commit_message"` and `srcObjectID = push_event_id`
119 once that worker job ships.
120
121 ## Markdown + sanitization
122
123 Issue bodies and comments use the existing S17 markdown pipeline
124 (`internal/repos/markdown.RenderHTML`): Goldmark for parsing,
125 bluemonday's `UGCPolicy` for sanitization. Rendered HTML is cached on
126 `body_html_cached`; `md_pipeline_version` lets the worker re-render on
127 pipeline upgrades (post-MVP).
128
129 `<script>` tags are stripped — the sanitization test
130 (`TestCreate_RendersHTMLAndSanitizesScripts`) injects one and asserts
131 the cached HTML doesn't contain `<script` while still rendering
132 `**bold**` as `<strong>bold</strong>`.
133
134 ## Locked issues
135
136 `SetLock` flips `locked` + emits a `locked`/`unlocked` event. Comments
137 hit the locked gate inside `AddComment`: non-collaborators get
138 `ErrIssueLocked`. The `IsCollab` flag is the caller's responsibility;
139 the web handler resolves it via `policy.HasRoleAtLeast(..., RoleTriage)`,
140 so owners, direct collaborators, and team-derived triage+ roles can
141 comment through a lock.
142
143 ## Rate limit
144
145 Comment creation hits the existing throttle bucket
146 (`scope=issue_comment`, `identifier=user:N`) at **20 per hour**.
147 Issue creation isn't independently rate-limited; the broader
148 `repo_create` bucket and account anti-abuse measures cover the
149 spam-floor case for now.
150
151 ## Timeline events
152
153 `issue_events` is intentionally polymorphic: `kind` is a free string,
154 `meta` is a `jsonb` blob, and `ref_target_id` is a denormalized
155 pointer for events that target another issue (`referenced`,
156 `marked_as_duplicate_of`, etc.). Known kinds today:
157
158 ```
159 closed, reopened, locked, unlocked,
160 labeled, unlabeled,
161 assigned, unassigned,
162 milestoned, demilestoned,
163 referenced
164 ```
165
166 `meta` always carries an empty `'{}'::jsonb` when no metadata is
167 applicable — Go's `nil` byte slice would bind as SQL NULL and trip
168 the NOT NULL constraint, so the orchestrator passes `[]byte("{}")`
169 explicitly.
170
171 ## Tests
172
173 | File | What it covers |
174 | ------------------------------------------------- | -------------------------------------- |
175 | `internal/issues/references_test.go` | regex behaviour for #N and owner/repo#N |
176 | `internal/issues/issues_test.go::Sequential…` | per-repo number allocation |
177 | `internal/issues/issues_test.go::ConcurrentRace…` | 8-goroutine fan-out, no duplicates |
178 | `internal/issues/issues_test.go::Sanitizes…` | XSS + markdown render |
179 | `internal/issues/issues_test.go::LockedRejects…` | locked-issue gate for non-collab |
180 | `internal/issues/issues_test.go::EmitsEvent` | close → reopen event ordering |
181 | `internal/issues/issues_test.go::CrossRef…` | #N in body emits `referenced` on target |
182
183 ## Deferred
184
185 - **Mention notifications** — `extractMentions` is in place, but the
186 notifications surface (S29-area) hasn't shipped, so parsed `@username`
187 tokens currently don't emit anything.
188 - **Commit-message references** — the helper accepts
189 `srcKind = "commit_message"` already; wiring lives with S14's worker
190 job (push:process).
191 - **Cross-org references** — the cross-repo regex resolves only
192 user-owned repos; org owners arrive in S31.
193 - **Required signing / status checks on comments** — out of scope.
194 - **Reactions, edit history, comment edits across users** — post-MVP UI
195 polish (S30+).
196 - **Server-side label / assignee / milestone filtering** — current
197 list query filters by state + kind only; richer filtering is the
198 perf-pass item in S36.
199 - **Issue templates / forms** — post-MVP.
200 - **Reactions** — post-MVP (own table; doesn't touch the schema here).