diff --git a/LABELS.md b/LABELS.md index af7ccf3..0f6acd4 100644 --- a/LABELS.md +++ b/LABELS.md @@ -175,9 +175,11 @@ take. It is hand-set: the machine never sets `attention`, never assigns anyone to receive one, and never decides that one has been answered — the assignee's removal is the only ack. It writes the label in exactly one place, the derived `claimed` → `post-merge` transition below, and nowhere -else; where it reads the flag it reads it to diagnose, and a diagnosis is a -comment that leaves the label alone. No reconciler enforces the assignee -requirement either: an unassigned flag may be reported, never repaired. +else; where it reads the flag it reads it to diagnose. The PR sweep comments +when `attention` is put on a pull request, and the issue sweep comments when +it is put on an issue with no assignee. Both diagnoses leave the label and +assignees alone; the machine never infers the claim issue, decides that the +demand was answered, or repairs either malformed shape. An `attention` issue without an assignee is therefore a board bug, not a demand; anyone may assign it or remove the flag. It never composes with `post-merge`, whose released claim has no assignee to answer the demand. The diff --git a/TRIAGE.md b/TRIAGE.md index 75e1b6d..e2cbe53 100644 --- a/TRIAGE.md +++ b/TRIAGE.md @@ -68,7 +68,10 @@ is the failure this whole flow exists to prevent. agreement is reached, record the ruling as a decision in one comment, remove the label, and return the issue to its flow in that same comment; when that ruling or any directive or answered builder question delivers - the assignee's next move in prose, set `attention` in the same comment. + the assignee's next move in prose, set `attention` in the same comment on + the assigned issue that owns the claim — never on the pull request, even + when the comment lives there. An unassigned issue is a board bug, not a + demand; repair the board rather than setting `attention`. This is not a substitute for minting work or for `needs-ruling`. 4. **Decline.** Real idea, wrong repo or wrong time. Say why plainly, link where it belongs if anywhere, close. A refusal with reasons is a good diff --git a/actions/issueflow-reconcile/issueflow-reconcile.sh b/actions/issueflow-reconcile/issueflow-reconcile.sh index 329a3b6..99a323f 100644 --- a/actions/issueflow-reconcile/issueflow-reconcile.sh +++ b/actions/issueflow-reconcile/issueflow-reconcile.sh @@ -27,6 +27,9 @@ TRIAGE_ACTORS=() # The needs-ruling invariants (#52) — one implementation for both surfaces. # shellcheck source=lib/ruling.sh . "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../../lib/ruling.sh" +# The attention target invariants (#232) — diagnosis only, both surfaces. +# shellcheck source=lib/attention.sh +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../../lib/attention.sh" # The guarded read and its reason line (#101, #247) — one implementation for # both surfaces. # shellcheck source=lib/read.sh @@ -511,6 +514,7 @@ reconcile_issue() { local n="$1" decision refs cross_refs states age created assignees open_pr=false label owners local merged_ref_pr="" transition_marker="" transition_handled=false parsed_set="" parse_marker="" local unchecked="" remove_claimed=claimed + local attention_active=true attention_suppression="" decision="$(queue_decision <<<"$ISSUE_LABELS")" case "$decision" in ADD_NEEDS_TRIAGE) @@ -554,6 +558,7 @@ The merge releases the claim; no builder owes a draft. Triage owes completion in --remove-label "$remove_claimed" --add-label post-merge >/dev/null fi log "#$n: merged Refs PR -> post-merge; claim released" + attention_active=false else created="$(jq -r '.created_at' <<<"$ISSUE_JSON")" guarded_read age last_issue_activity "$n" "$created" \ @@ -585,6 +590,7 @@ The merge releases the claim; no builder owes a draft. Triage owes completion in fi log "#$n: stale claim reclaimed -> ready" ;; esac + [ "$decision" != FLAG_UNASSIGNED ] || attention_suppression=claimed-unassigned if has_issue_label offsite; then local timeline if timeline="$(offsite_timeline "$n")"; then @@ -605,6 +611,7 @@ The merge releases the claim; no builder owes a draft. Triage owes completion in 'This `post-merge` issue has an assignee or `attention`. The sweep will not undo hand-set intent; triage must clear the invalid composition or move the issue back into buildable queue state.' log "#$n: assigned or attention-bearing post-merge issue flagged" fi + attention_suppression=post-merge-assigned elif has_issue_label blocked; then refs="$(blocked_references <<<"$(jq -r '.body // ""' <<<"$ISSUE_JSON")")" cross_refs="$(blocked_cross_references <<<"$(jq -r '.body // ""' <<<"$ISSUE_JSON")")" @@ -662,6 +669,14 @@ itself, so a parse unchanged since the last echo never re-posts.*" >/dev/null fi fi + # The flag composes with every build queue state, but requires an assignee. + # Existing post-merge/claimed diagnostics take precedence so one board bug + # draws one comment (#232 D5); the shared helper still logs the suppression. + if [ "$attention_active" = true ] && has_issue_label attention; then + [ -n "${assignees:-}" ] || assignees="$(jq '.assignees | length' <<<"$ISSUE_JSON")" + reconcile_attention "$n" issue "$assignees" "$attention_suppression" + fi + # ---- the ruling invariants (#52), on any queue state ---- # The flag composes with the queue labels (#50 D8), so this runs after the # queue branches rather than inside one of them. The FLAG_CONFLICT return diff --git a/actions/labels-reconcile/labels-reconcile.sh b/actions/labels-reconcile/labels-reconcile.sh index 97ada8a..a2ef589 100755 --- a/actions/labels-reconcile/labels-reconcile.sh +++ b/actions/labels-reconcile/labels-reconcile.sh @@ -74,6 +74,9 @@ SELF_WORKFLOW="${SELF_WORKFLOW:-${GITHUB_WORKFLOW:-}}" # The needs-ruling invariants (#52) — one implementation for both surfaces. # shellcheck source=lib/ruling.sh . "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../../lib/ruling.sh" +# The attention target invariants (#232) — diagnosis only, both surfaces. +# shellcheck source=lib/attention.sh +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../../lib/attention.sh" # The guarded read and its reason line (#101) — one implementation for both # surfaces. read_failure_reason lived here until the issue surface needed the # identical rule (#247); a second copy of it is the failure lib/ruling.sh's @@ -921,6 +924,12 @@ reconcile_pr() { # $1 = PR number; relies on the globals set from its fetch if has_label needs-ruling; then reconcile_ruling "$n" "$last_activity_epoch" "$NOW" fi + + # `attention` belongs on the assigned issue that owns the claim, never on + # a pull request (#232). Behind the label gate so ordinary PRs pay no read. + if has_label attention; then + reconcile_attention "$n" pr "$(jq '.assignees | length' <<<"$PR_JSON")" "" + fi } main() { diff --git a/changelog.d/230.md b/changelog.d/230.md new file mode 100644 index 0000000..159ce2b --- /dev/null +++ b/changelog.d/230.md @@ -0,0 +1,5 @@ +### Fixed + +- Triage now puts `attention` on the assigned issue that owns a claim, never + on its pull request, and treats an unassigned issue as a board bug rather + than a demand (#230). diff --git a/changelog.d/232.md b/changelog.d/232.md new file mode 100644 index 0000000..6785136 --- /dev/null +++ b/changelog.d/232.md @@ -0,0 +1,5 @@ +### Added + +- The label and issue-flow sweeps now comment once per episode when + `attention` targets a pull request or an unassigned issue, without + retargeting the demand or changing labels or assignees (#232). diff --git a/lib/attention.sh b/lib/attention.sh new file mode 100644 index 0000000..736493a --- /dev/null +++ b/lib/attention.sh @@ -0,0 +1,97 @@ +#!/usr/bin/env bash +# lib/attention.sh — the `attention` target invariants (#232, epic #229). +# +# Both reconcilers source this file. Pure decisions sit above the divider; +# the impure orchestrator below reads the current label episode and comments +# through the sourcing script's run()/log(). The machine diagnoses only: it +# never sets, clears, retargets or assigns anything on the strength of these +# checks (#229 D2). + +ATTENTION_MARKER_PREFIX='\n' "$ATTENTION_MARKER_PREFIX" "$1" +} + +# --------------------------------------------------------------------------- +# The impure orchestrator. Called only behind a has-attention gate. +# Needs REPO; uses the caller's run() and log(). +# --------------------------------------------------------------------------- + +reconcile_attention() { # $1 item, $2 pr|issue, $3 assignees, $4 suppression + local n="$1" surface="$2" assignees="$3" suppression="${4:-}" + local target comment labeled_events labeled_at marker comments body + : "${REPO:?reconcile_attention: REPO is required}" + + target="$(attention_target_decision "$surface" "$assignees")" + comment="$(attention_comment_decision "$target" "$suppression")" + [ "$comment" != KEEP ] || return 0 + + if [ "$comment" = SUPPRESS ]; then + log "#$n: malformed attention detected; comment suppressed by $suppression precedence" + return 0 + fi + + if ! labeled_events="$(gh api --paginate "repos/$REPO/issues/$n/timeline" \ + --jq '.[] | select(.event == "labeled" and .label.name == "attention") + | .created_at' 2>/dev/null)"; then + log "#$n: attention timeline unreadable — no verdict invented this pass" + return 0 + fi + if [ -z "$labeled_events" ]; then + log "#$n: attention flag has no visible labeled event — no verdict invented this pass" + return 0 + fi + labeled_at="$(attention_newest_flag <<<"$labeled_events")" + marker="$(attention_episode_marker "$labeled_at")" + + if ! comments="$(gh api --paginate "repos/$REPO/issues/$n/comments" \ + --jq '.[].body // ""' 2>/dev/null)"; then + log "#$n: attention comments unreadable — no verdict invented this pass" + return 0 + fi + grep -qF "$marker" <<<"$comments" && return 0 + + case "$target" in + MALFORMED_PR) + body="$marker +This pull request carries \`attention\`, but that label is issue-only. Put +the label on the assigned issue that owns the claim. The sweep cannot infer +which issue that is, so it reports the malformed target without removing or +retargeting the label." ;; + MALFORMED_UNASSIGNED) + body="$marker +This issue carries \`attention\` but has no assignee to receive the demand. +Assign the intended builder or remove the flag. The sweep reports the board +bug without assigning anyone or changing the label." ;; + esac + run gh issue comment "$n" -R "$REPO" --body "$body" >/dev/null + log "#$n: malformed attention ($surface) — commented; no label or assignee changed" +} diff --git a/test/attention.test.sh b/test/attention.test.sh new file mode 100644 index 0000000..410c572 --- /dev/null +++ b/test/attention.test.sh @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +set -u + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +# shellcheck source=test/harness.sh +source "$ROOT/test/harness.sh" +# shellcheck source=lib/attention.sh +source "$ROOT/lib/attention.sh" + +check "attention on an unassigned PR is malformed" 0 "MALFORMED_PR" \ + attention_target_decision pr 0 +check "attention on an assigned PR is still malformed" 0 "MALFORMED_PR" \ + attention_target_decision pr 1 +check "attention on an unassigned issue is malformed" 0 "MALFORMED_UNASSIGNED" \ + attention_target_decision issue 0 +check "attention on an assigned issue is healthy" 0 "KEEP" \ + attention_target_decision issue 1 +check "an unknown surface is rejected" 2 "" attention_target_decision discussion 0 + +check "a malformed PR target is commented" 0 "POST" \ + attention_comment_decision MALFORMED_PR "" +check "an unassigned issue target is commented without precedence" 0 "POST" \ + attention_comment_decision MALFORMED_UNASSIGNED "" +check "claimed-unassigned precedence suppresses the second comment" 0 "SUPPRESS" \ + attention_comment_decision MALFORMED_UNASSIGNED claimed-unassigned +check "post-merge-assigned precedence suppresses the second comment" 0 "SUPPRESS" \ + attention_comment_decision MALFORMED_UNASSIGNED post-merge-assigned +check "a healthy target stays silent" 0 "KEEP" \ + attention_comment_decision KEEP "" + +check "the newest labeled event defines the episode" 0 "2026-08-03T12:00:00Z" \ + attention_newest_flag <<'EOF' +2026-08-03T10:00:00Z +2026-08-03T12:00:00Z +2026-08-03T11:00:00Z +EOF +check "the marker names the label episode" 0 \ + '' \ + attention_episode_marker 2026-08-03T12:00:00Z + +# Diagnosis is the only write this library may own. Pin the absence of every +# label/assignee mutation spelling so a later refactor cannot quietly turn a +# report into a repair (#229 D2). +check "the attention library contains no issue/PR edit mutation" 1 "" \ + grep -E 'gh (issue|pr) edit|--(add|remove)-(label|assignee)' "$ROOT/lib/attention.sh" + +summary diff --git a/test/issueflow-reconcile.test.sh b/test/issueflow-reconcile.test.sh index e609f03..ae82aa3 100644 --- a/test/issueflow-reconcile.test.sh +++ b/test/issueflow-reconcile.test.sh @@ -462,6 +462,95 @@ issue_probe() { # $1 issue, $2 labels, $3 assignees, $4 false|closing|refs, $5 m tfix() { printf '%s/repos_owner_repo_issues_%s_timeline.json' "$TMP" "$1"; } cfix() { printf '%s/repos_owner_repo_issues_%s_comments.json' "$TMP" "$1"; } +# -- malformed attention targets are diagnosed, never repaired (#232) ------- +attention_episode() { # $1 issue, $2 labeled timestamp + jq -n --arg at "$2" \ + '[{"event":"labeled","label":{"name":"attention"},"actor":{"login":"setter"},"created_at":$at}]' \ + >"$(tfix "$1")" + printf '[]\n' >"$(cfix "$1")" +} + +: >"$TMP/issue-edits" +attention_edits_before="$(wc -l <"$TMP/issue-edits")" +for n in 61 62 63; do + attention_episode "$n" "$(iso_at $((INOW - 60)))" +done +# The assignment event is the claim clock's own activity fact. The live issue +# has since lost its assignee, which is exactly the claimed-unassigned shape. +jq --arg at "$(iso_at $((INOW - 60)))" \ + '. + [{"event":"assigned","created_at":$at}]' \ + "$(tfix 63)" >"$(tfix 63).tmp" && mv "$(tfix 63).tmp" "$(tfix 63)" +printf '{"state":"open"}\n' >"$TMP/repos_owner_repo_issues_999.json" + +issue_probe 61 $'ready\nattention' 0 >/dev/null +check "unassigned attention under ready is diagnosed once" 0 "1" \ + grep -cF '' "$TMP/posted-63" +check "the suppressed attention detection remains in the log" 0 "1" \ + grep -cF 'comment suppressed by claimed-unassigned precedence' <<<"$claimed_attention" + +attention_episode 64 "$(iso_at $((INOW - 60)))" +issue_probe 64 $'ready\nattention' 1 >/dev/null +check "assigned attention under ready is healthy" 1 "" test -f "$TMP/posted-64" +attention_episode 65 "$(iso_at $((INOW - 60)))" +issue_probe 65 $'claimed\nattention' 1 true >/dev/null +check "assigned attention under claimed is healthy" 1 "" test -f "$TMP/posted-65" +attention_episode 66 "$(iso_at $((INOW - 60)))" +issue_probe 66 $'blocked\nattention' 1 false "" 'Blocked by #999' >/dev/null +check "assigned attention under blocked is healthy" 1 "" test -f "$TMP/posted-66" + +attention_episode 67 "$(iso_at $((INOW - 60)))" +post_merge_attention="$(issue_probe 67 $'post-merge\nattention' 0)" +check "post-merge precedence leaves exactly its existing comment" 0 "1" \ + grep -c -- '^----$' "$TMP/posted-67" +check "post-merge's existing diagnostic wins" 0 "1" \ + grep -cF '' "$TMP/posted-67" +check "the post-merge suppression remains in the log" 0 "1" \ + grep -cF 'comment suppressed by post-merge-assigned precedence' <<<"$post_merge_attention" + +attention_episode 68 "$(iso_at $((INOW - 120)))" +issue_probe 68 $'ready\nattention' 0 >/dev/null +issue_probe 68 $'ready\nattention' 0 >/dev/null +check "two sweeps in one malformed episode post once" 0 "1" \ + grep -cF '