diff --git a/actions/setup/js/evaluate_outcomes.cjs b/actions/setup/js/evaluate_outcomes.cjs index 52be0fc3089..a98a8501348 100644 --- a/actions/setup/js/evaluate_outcomes.cjs +++ b/actions/setup/js/evaluate_outcomes.cjs @@ -455,7 +455,7 @@ function evaluateAddComment(item, itemRepo, timestamp, out, apiGet, nowMs) { * @returns {EvalResult} */ function evaluateAddLabels(item, itemRepo, timestamp, out, apiGet, nowMs) { - const num = parseIssueNumberFromURL(item.url || ""); + const num = getItemNumber(item); if (!num) { out.result = "unknown"; out.detail = "unknown: issue number not found"; @@ -463,8 +463,9 @@ function evaluateAddLabels(item, itemRepo, timestamp, out, apiGet, nowMs) { return out; } - const hasLabelsBefore = Object.hasOwn(item, "labelsBefore") && Array.isArray(item.labelsBefore); - const labelsBefore = hasLabelsBefore ? normalizeLabels(item.labelsBefore) : []; + const persistedBefore = item.before_state?.labels ?? item.labelsBefore; + const hasLabelsBefore = Array.isArray(persistedBefore); + const labelsBefore = hasLabelsBefore ? normalizeLabels(persistedBefore) : []; const labelsAdded = normalizeLabels(item.labelsAdded); const fallbackLabels = normalizeLabels(item.labels); const effectiveLabelsAdded = labelsAdded.length > 0 ? labelsAdded : fallbackLabels; @@ -534,6 +535,170 @@ function evaluateAddLabels(item, itemRepo, timestamp, out, apiGet, nowMs) { return out; } +/** + * Evaluate `close_issue`. + * @param {object} item + * @param {string} defaultRepo + * @param {(endpoint: string) => any} api + * @param {number} nowMs + * @returns {EvalResult} + */ +function evaluateCloseIssue(item, defaultRepo, api = ghAPI, nowMs = Date.now()) { + const repo = getItemRepo(item, defaultRepo); + const number = getItemNumber(item); + const timestamp = item.timestamp || ""; + /** @type {EvalResult} */ + const out = { + result: "unknown", + outcome_status: "unknown", + evidence_strength: "weak", + signal: "unknown", + detail: "", + resolution_sec: null, + pending_age_sec: null, + review_comments: null, + changed_files: null, + additions: null, + deletions: null, + reactions_total: null, + reactions_positive: null, + reactions_negative: null, + comments: null, + zero_touch: false, + }; + + if (!repo || !number) { + out.detail = "missing issue reference"; + return out; + } + + const issue = api(`repos/${repo}/issues/${number}`); + if (!issue || typeof issue.state !== "string") { + out.detail = "api error"; + setPendingAge(out, timestamp, nowMs); + return out; + } + + out.comments = typeof issue.comments === "number" ? issue.comments : null; + if (issue.reactions && typeof issue.reactions === "object") { + const summary = summarizeReactions(issue.reactions); + out.reactions_total = summary.total; + out.reactions_positive = summary.positive; + out.reactions_negative = summary.negative; + } + + if (issue.state === "closed") { + out.result = "accepted"; + out.outcome_status = "accepted"; + out.evidence_strength = "strong"; + out.signal = "closed"; + out.detail = "closed"; + if (issue.created_at && issue.closed_at) { + out.resolution_sec = secondsBetween(issue.created_at, issue.closed_at); + } + return out; + } + + out.result = "rejected"; + out.outcome_status = "rejected"; + out.evidence_strength = "strong"; + out.signal = "not_closed"; + out.detail = "not_closed"; + return out; +} + +/** + * Evaluate `close_pull_request`. + * @param {object} item + * @param {string} defaultRepo + * @param {(endpoint: string) => any} api + * @param {number} nowMs + * @returns {EvalResult} + */ +function evaluateClosePullRequest(item, defaultRepo, api = ghAPI, nowMs = Date.now()) { + const repo = getItemRepo(item, defaultRepo); + const number = getItemNumber(item); + const timestamp = item.timestamp || ""; + /** @type {EvalResult} */ + const out = { + result: "unknown", + outcome_status: "unknown", + evidence_strength: "weak", + signal: "unknown", + detail: "", + resolution_sec: null, + pending_age_sec: null, + review_comments: null, + changed_files: null, + additions: null, + deletions: null, + reactions_total: null, + reactions_positive: null, + reactions_negative: null, + comments: null, + zero_touch: false, + }; + + if (!repo || !number) { + out.detail = "missing pull request reference"; + return out; + } + + const pullRequest = api(`repos/${repo}/pulls/${number}`); + if (!pullRequest || typeof pullRequest.state !== "string") { + out.detail = "api error"; + setPendingAge(out, timestamp, nowMs); + return out; + } + + out.review_comments = typeof pullRequest.review_comments === "number" ? pullRequest.review_comments : null; + out.changed_files = typeof pullRequest.changed_files === "number" ? pullRequest.changed_files : null; + out.additions = typeof pullRequest.additions === "number" ? pullRequest.additions : null; + out.deletions = typeof pullRequest.deletions === "number" ? pullRequest.deletions : null; + out.comments = typeof pullRequest.comments === "number" ? pullRequest.comments : null; + if (pullRequest.reactions && typeof pullRequest.reactions === "object") { + const summary = summarizeReactions(pullRequest.reactions); + out.reactions_total = summary.total; + out.reactions_positive = summary.positive; + out.reactions_negative = summary.negative; + } + + // A merged PR is rejected because close_pull_request verifies that the PR + // remained closed without being merged. Merging is a different terminal + // state than closing, so it invalidates the close outcome. + if (pullRequest.merged === true || pullRequest.merged_at != null) { + out.result = "rejected"; + out.outcome_status = "rejected"; + out.evidence_strength = "strong"; + out.signal = "closed_by_merge"; + out.detail = "merged"; + if (pullRequest.created_at && pullRequest.merged_at) { + out.resolution_sec = secondsBetween(pullRequest.created_at, pullRequest.merged_at); + } + return out; + } + // Accepted means the PR is closed and unmerged, which is the durable + // terminal state that close_pull_request validates. + if (pullRequest.state === "closed") { + out.result = "accepted"; + out.outcome_status = "accepted"; + out.evidence_strength = "strong"; + out.signal = "closed"; + out.detail = "closed"; + if (pullRequest.created_at && pullRequest.closed_at) { + out.resolution_sec = secondsBetween(pullRequest.created_at, pullRequest.closed_at); + } + return out; + } + + out.result = "rejected"; + out.outcome_status = "rejected"; + out.evidence_strength = "strong"; + out.signal = "not_closed"; + out.detail = "not_closed"; + return out; +} + /** * Normalize legacy result/detail pairs into the shared outcome model. * @param {string} result @@ -596,9 +761,18 @@ function normalizeOutcome(result, detail) { if (result === "accepted" && normalizedDetail.startsWith("merged")) { return { outcome_status: "accepted", evidence_strength: "strong", signal: "merged" }; } + if (result === "accepted" && normalizedDetail === "closed") { + return { outcome_status: "accepted", evidence_strength: "strong", signal: "closed" }; + } if (result === "rejected" && normalizedDetail === "closed") { return { outcome_status: "rejected", evidence_strength: "strong", signal: "closed" }; } + if (result === "rejected" && normalizedDetail === "merged") { + return { outcome_status: "rejected", evidence_strength: "strong", signal: "closed_by_merge" }; + } + if (result === "rejected" && normalizedDetail === "not_closed") { + return { outcome_status: "rejected", evidence_strength: "strong", signal: "not_closed" }; + } if (result === "pending" && normalizedDetail === "open") { return { outcome_status: "pending", evidence_strength: "medium", signal: "open" }; } @@ -1187,6 +1361,21 @@ function evaluateItem(item, defaultRepo, apiOrOptions) { if (type === "update_pull_request") { return evaluateUpdatePullRequest(item, defaultRepo, ghAPIFn); } + if (type === "add_labels") { + return evaluateAddLabels(item, itemRepo, timestamp, out, ghAPIFn, nowMs); + } + if (type === "close_issue") { + return evaluateCloseIssue(item, defaultRepo, ghAPIFn, nowMs); + } + if (type === "close_pull_request") { + return evaluateClosePullRequest(item, defaultRepo, ghAPIFn, nowMs); + } + if (type === "create_issue") { + return evaluateCreateIssue(item, itemRepo, timestamp, out, ghAPIFn, nowMs); + } + if (type === "add_comment") { + return evaluateAddComment(item, itemRepo, timestamp, out, ghAPIFn, nowMs); + } if (!url) { if (item.type === "add_reviewer") { @@ -1206,15 +1395,6 @@ function evaluateItem(item, defaultRepo, apiOrOptions) { if (type === "submit_pull_request_review") { return evaluateSubmitPullRequestReview(item, defaultRepo, ghAPIFn); } - if (type === "create_issue") { - return evaluateCreateIssue(item, itemRepo, timestamp, out, ghAPIFn, nowMs); - } - if (type === "add_comment") { - return evaluateAddComment(item, itemRepo, timestamp, out, ghAPIFn, nowMs); - } - if (type === "add_labels") { - return evaluateAddLabels(item, itemRepo, timestamp, out, ghAPIFn, nowMs); - } // Issues / issue-comments const issueMatch = url.match(/\/(?:issues|pull)\/(\d+)/); @@ -1944,6 +2124,8 @@ module.exports = { evaluateCreateIssue, evaluateAddComment, evaluateAddLabels, + evaluateCloseIssue, + evaluateClosePullRequest, evaluateCreatePullRequestOutcome, evaluatePushToPullRequestBranchOutcome, normalizeOutcome, diff --git a/actions/setup/js/evaluate_outcomes.test.cjs b/actions/setup/js/evaluate_outcomes.test.cjs index 72487ce817a..010f1b95bf1 100644 --- a/actions/setup/js/evaluate_outcomes.test.cjs +++ b/actions/setup/js/evaluate_outcomes.test.cjs @@ -142,7 +142,8 @@ describe("evaluate_outcomes type-specific evaluators", () => { it("evaluates add_labels retention using persisted before-state labels", () => { const item = { type: "add_labels", - url: "https://github.com/acme/repo/issues/42", + number: 42, + repo: "acme/repo", timestamp: "2026-05-25T00:00:00Z", labelsBefore: ["bug"], labelsAdded: ["bug", "triage"], @@ -164,6 +165,192 @@ describe("evaluate_outcomes type-specific evaluators", () => { expect(evaluateItem({ ...item, labelsBefore: [] }, "acme/repo", { ghAPI: retained, nowMs: Date.parse("2026-05-27T00:00:00Z") }).detail).toBe("accepted:strong"); }); + it("evaluates add_labels retention using production before_state.labels manifest shape", () => { + // Production manifest from add_labels.cjs stores labels in before_state.labels, not labelsBefore. + const item = { + type: "add_labels", + number: 42, + repo: "acme/repo", + timestamp: "2026-05-25T00:00:00Z", + before_state: { labels: ["bug"] }, + labelsAdded: ["bug", "triage"], + }; + + const retained = createAPIStub({ + "repos/acme/repo/issues/42/labels": [{ name: "bug" }, { name: "triage" }], + }); + expect(evaluateItem(item, "acme/repo", { ghAPI: retained, nowMs: Date.parse("2026-05-27T00:00:00Z") }).detail).toBe("accepted:strong"); + }); + + it("returns unknown for add_labels when the item number is missing", () => { + const missingNumber = evaluateItem( + { + type: "add_labels", + repo: "acme/repo", + timestamp: "2026-05-25T00:00:00Z", + labelsBefore: ["bug"], + labelsAdded: ["bug", "triage"], + }, + "acme/repo", + { + ghAPI: createAPIStub({ + "repos/acme/repo/issues/42/labels": [{ name: "bug" }, { name: "triage" }], + }), + nowMs: Date.parse("2026-05-27T00:00:00Z"), + } + ); + expect(missingNumber.detail).toBe("unknown: issue number not found"); + }); + + it("evaluates close_issue using persisted repo/number metadata", () => { + const item = { + type: "close_issue", + number: 77, + repo: "acme/repo", + timestamp: "2026-05-26T00:00:00Z", + }; + + const closed = createAPIStub({ + "repos/acme/repo/issues/77": { + state: "closed", + comments: 1, + reactions: { total_count: 2 }, + created_at: "2026-05-26T00:00:00Z", + closed_at: "2026-05-26T00:10:00Z", + }, + }); + expect(evaluateItem(item, "acme/repo", { ghAPI: closed })).toMatchObject({ + result: "accepted", + outcome_status: "accepted", + evidence_strength: "strong", + signal: "closed", + detail: "closed", + }); + + const open = createAPIStub({ + "repos/acme/repo/issues/77": { state: "open", comments: 1, reactions: { total_count: 0 } }, + }); + expect(evaluateItem(item, "acme/repo", { ghAPI: open })).toMatchObject({ + result: "rejected", + outcome_status: "rejected", + evidence_strength: "strong", + signal: "not_closed", + detail: "not_closed", + }); + }); + + it("returns unknown for close_issue when the item reference is missing", () => { + const missingRef = evaluateItem({ type: "close_issue", timestamp: "2026-05-26T00:00:00Z" }, "acme/repo", { ghAPI: createAPIStub({}), nowMs: Date.parse("2026-05-27T00:00:00Z") }); + expect(missingRef.result).toBe("unknown"); + expect(missingRef.detail).toBe("missing issue reference"); + }); + + it("returns unknown for close_issue on API error and sets pending_age_sec", () => { + const nowMs = Date.parse("2026-05-27T00:00:00Z"); + const apiError = evaluateItem({ type: "close_issue", number: 77, repo: "acme/repo", timestamp: "2026-05-26T00:00:00Z" }, "acme/repo", { ghAPI: createAPIStub({}), nowMs }); + expect(apiError.result).toBe("unknown"); + expect(apiError.detail).toBe("api error"); + expect(typeof apiError.pending_age_sec).toBe("number"); + }); + + it("evaluates close_pull_request using persisted repo/number metadata", () => { + const item = { + type: "close_pull_request", + number: 55, + repo: "acme/repo", + timestamp: "2026-05-26T00:00:00Z", + }; + + const closed = createAPIStub({ + "repos/acme/repo/pulls/55": { + state: "closed", + merged: false, + review_comments: 0, + changed_files: 1, + additions: 3, + deletions: 1, + comments: 0, + created_at: "2026-05-26T00:00:00Z", + closed_at: "2026-05-26T00:10:00Z", + }, + }); + expect(evaluateItem(item, "acme/repo", { ghAPI: closed })).toMatchObject({ + result: "accepted", + outcome_status: "accepted", + evidence_strength: "strong", + signal: "closed", + detail: "closed", + }); + + const merged = createAPIStub({ + "repos/acme/repo/pulls/55": { + state: "closed", + merged: true, + review_comments: 0, + changed_files: 1, + additions: 3, + deletions: 1, + comments: 0, + created_at: "2026-05-26T00:00:00Z", + merged_at: "2026-05-26T00:10:00Z", + }, + }); + expect(evaluateItem(item, "acme/repo", { ghAPI: merged })).toMatchObject({ + result: "rejected", + outcome_status: "rejected", + evidence_strength: "strong", + signal: "closed_by_merge", + detail: "merged", + }); + + const open = createAPIStub({ + "repos/acme/repo/pulls/55": { state: "open", merged: false, review_comments: 0 }, + }); + expect(evaluateItem(item, "acme/repo", { ghAPI: open })).toMatchObject({ + result: "rejected", + outcome_status: "rejected", + evidence_strength: "strong", + signal: "not_closed", + detail: "not_closed", + }); + }); + + it("detects merged PR via merged_at when merged flag is absent", () => { + const item = { + type: "close_pull_request", + number: 55, + repo: "acme/repo", + timestamp: "2026-05-26T00:00:00Z", + }; + const mergedAtOnly = createAPIStub({ + "repos/acme/repo/pulls/55": { + state: "closed", + merged: false, + merged_at: "2026-05-26T00:10:00Z", + created_at: "2026-05-26T00:00:00Z", + }, + }); + expect(evaluateItem(item, "acme/repo", { ghAPI: mergedAtOnly })).toMatchObject({ + result: "rejected", + signal: "closed_by_merge", + detail: "merged", + }); + }); + + it("returns unknown for close_pull_request when the item reference is missing", () => { + const missingRef = evaluateItem({ type: "close_pull_request", timestamp: "2026-05-26T00:00:00Z" }, "acme/repo", { ghAPI: createAPIStub({}), nowMs: Date.parse("2026-05-27T00:00:00Z") }); + expect(missingRef.result).toBe("unknown"); + expect(missingRef.detail).toBe("missing pull request reference"); + }); + + it("returns unknown for close_pull_request on API error and sets pending_age_sec", () => { + const nowMs = Date.parse("2026-05-27T00:00:00Z"); + const apiError = evaluateItem({ type: "close_pull_request", number: 55, repo: "acme/repo", timestamp: "2026-05-26T00:00:00Z" }, "acme/repo", { ghAPI: createAPIStub({}), nowMs }); + expect(apiError.result).toBe("unknown"); + expect(apiError.detail).toBe("api error"); + expect(typeof apiError.pending_age_sec).toBe("number"); + }); + it("evaluates update_issue retained and reverted states from persisted execution metadata", () => { const retained = evaluateItem( { diff --git a/pkg/cli/outcome_eval.go b/pkg/cli/outcome_eval.go index fcd9908aacd..d2af77bcd92 100644 --- a/pkg/cli/outcome_eval.go +++ b/pkg/cli/outcome_eval.go @@ -92,6 +92,7 @@ var outcomeEvaluators = map[string]outcomeEvaluator{ "update_pull_request": evalUpdatePullRequest, "add_comment": evalAddComment, "add_labels": evalAddLabels, + "replace_label": evalReplaceLabel, "assign_to_agent": evalAssignToAgent, "close_issue": evalCloseSticky, "close_pull_request": evalCloseSticky, diff --git a/pkg/cli/outcome_eval_label.go b/pkg/cli/outcome_eval_label.go index 21be3ab8a0b..0dd4fe65348 100644 --- a/pkg/cli/outcome_eval_label.go +++ b/pkg/cli/outcome_eval_label.go @@ -2,12 +2,139 @@ package cli import ( "fmt" + "slices" "github.com/github/gh-aw/pkg/logger" ) var outcomeEvalLabelLog = logger.New("cli:outcome_eval_label") +// evalReplaceLabel checks whether the label delta applied at execution time is +// still intact in the current issue state. It computes the set of labels added +// and removed by the replacement and verifies only that delta, ignoring any +// unrelated labels that may have been added or removed since execution. +func evalReplaceLabel(item CreatedItemReport, repoOverride string) OutcomeReport { + repo := resolveItemRepo(item, repoOverride) + num := resolveItemNumber(item) + outcomeEvalLabelLog.Printf("Evaluating replace_label outcome: repo=%s, number=%d", repo, num) + report := OutcomeReport{ + Type: item.Type, + ObjectURL: item.URL, + ObjectNumber: num, + Repo: repo, + } + if num == 0 || repo == "" || item.BeforeState == nil || item.AfterState == nil { + report.Result = OutcomeUnknown + report.Detail = "missing execution state" + report.OutcomeEvaluation = OutcomeEvaluation{ + OutcomeStatus: OutcomeStatusUnknown, + EvidenceStrength: EvidenceNone, + Signal: "missing_execution_state", + } + return report + } + + beforeLabels := mutableStringSlice(item.BeforeState["labels"]) + afterLabels := mutableStringSlice(item.AfterState["labels"]) + + // Compute the replacement delta: labels added and labels removed. + added := labelSetDiff(afterLabels, beforeLabels) + removed := labelSetDiff(beforeLabels, afterLabels) + + if len(added) == 0 && len(removed) == 0 { + report.Result = OutcomeUnknown + report.Detail = "no label delta" + report.OutcomeEvaluation = OutcomeEvaluation{ + OutcomeStatus: OutcomeStatusUnknown, + EvidenceStrength: EvidenceNone, + Signal: "no_state_delta", + } + return report + } + + currentState, _, err := extractCurrentIssueUpdateState(repo, num) + if err != nil { + report.Result = OutcomeError + report.EvalError = err.Error() + return report + } + currentLabels := mutableStringSlice(currentState["labels"]) + + // The replacement is retained when all added labels are still present and + // all removed labels are still absent, regardless of any other label changes. + addedRetained := labelSetContainsAll(currentLabels, added) + removedStillAbsent := !labelSetContainsAny(currentLabels, removed) + + if addedRetained && removedStillAbsent { + report.Result = OutcomeAccepted + report.Detail = "label replacement retained" + report.OutcomeEvaluation = OutcomeEvaluation{ + OutcomeStatus: OutcomeStatusAccepted, + EvidenceStrength: EvidenceMedium, + Signal: "state_retained", + } + return report + } + + // Reverted: all added labels are gone and all removed labels are back. + addedReverted := !labelSetContainsAny(currentLabels, added) + removedBack := labelSetContainsAll(currentLabels, removed) + if addedReverted && removedBack { + report.Result = OutcomeRejected + report.Detail = "label replacement reverted" + report.OutcomeEvaluation = OutcomeEvaluation{ + OutcomeStatus: OutcomeStatusRejected, + EvidenceStrength: EvidenceStrong, + Signal: "state_reverted", + } + return report + } + + report.Result = OutcomeRejected + report.Detail = "label replacement replaced" + report.OutcomeEvaluation = OutcomeEvaluation{ + OutcomeStatus: OutcomeStatusRejected, + EvidenceStrength: EvidenceStrong, + Signal: "state_replaced", + } + return report +} + +// labelSetDiff returns the elements of a that are not in b. +// Both slices must be sorted (as produced by mutableStringSlice). +// Uses binary search for O(n log m) performance. +func labelSetDiff(a, b []string) []string { + var out []string + for _, v := range a { + if _, found := slices.BinarySearch(b, v); !found { + out = append(out, v) + } + } + return out +} + +// labelSetContainsAll reports whether current contains every element of want. +// Both slices must be sorted. Uses binary search for O(n log m) performance. +func labelSetContainsAll(current, want []string) bool { + for _, v := range want { + if _, found := slices.BinarySearch(current, v); !found { + return false + } + } + return true +} + +// labelSetContainsAny reports whether current contains at least one element of want. +// Both slices must be sorted. Uses binary search for O(n log m) performance. +func labelSetContainsAny(current, want []string) bool { + for _, v := range want { + if _, found := slices.BinarySearch(current, v); found { + return true + } + } + return false +} + // evalAddLabels checks whether labels added by the workflow are still present. func evalAddLabels(item CreatedItemReport, repoOverride string) OutcomeReport { repo := resolveItemRepo(item, repoOverride) diff --git a/pkg/cli/outcome_eval_update.go b/pkg/cli/outcome_eval_update.go index d4fcf559e50..4a888e06d7b 100644 --- a/pkg/cli/outcome_eval_update.go +++ b/pkg/cli/outcome_eval_update.go @@ -155,6 +155,8 @@ func mutableTrackedFields(itemType string) []string { return []string{"title", "body_hash", "state", "labels", "assignees"} case "update_pull_request": return []string{"title", "body_hash", "state", "base", "draft", "head_sha"} + case "replace_label": + return []string{"labels"} default: return nil } diff --git a/pkg/cli/outcome_eval_update_test.go b/pkg/cli/outcome_eval_update_test.go index fb80f225b9a..914854ea18d 100644 --- a/pkg/cli/outcome_eval_update_test.go +++ b/pkg/cli/outcome_eval_update_test.go @@ -191,3 +191,105 @@ func TestEvalRetainedUpdateMissingExecutionStateUsesEvidenceNone(t *testing.T) { assert.Equal(t, EvidenceNone, report.EvidenceStrength) assert.Equal(t, "missing_execution_state", report.Signal) } + +func TestEvalReplaceLabelRetained(t *testing.T) { + old := outcomeUpdateGHAPIGet + t.Cleanup(func() { + outcomeUpdateGHAPIGet = old + }) + outcomeUpdateGHAPIGet = func(endpoint string, repo string) (map[string]any, error) { + return map[string]any{ + "labels": []any{ + map[string]any{"name": "triage"}, + map[string]any{"name": "done"}, + }, + }, nil + } + + report := evalReplaceLabel(CreatedItemReport{ + Type: "replace_label", + Number: 12, + Repo: "owner/repo", + BeforeState: map[string]any{ + "labels": []any{"triage", "in-progress"}, + }, + AfterState: map[string]any{ + "labels": []any{"triage", "done"}, + }, + }, "owner/repo") + + assert.Equal(t, OutcomeAccepted, report.Result) + assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus) + assert.Equal(t, EvidenceMedium, report.EvidenceStrength) + assert.Equal(t, "state_retained", report.Signal) +} + +func TestEvalReplaceLabelRetainedWithExtraLabel(t *testing.T) { + // An unrelated label added after execution must not cause state_replaced. + // Before: [in-progress], After: [done], Current: [done, security] + // Delta: added=[done], removed=[in-progress] + // "done" is still present, "in-progress" still absent → accepted. + old := outcomeUpdateGHAPIGet + t.Cleanup(func() { + outcomeUpdateGHAPIGet = old + }) + outcomeUpdateGHAPIGet = func(endpoint string, repo string) (map[string]any, error) { + return map[string]any{ + "labels": []any{ + map[string]any{"name": "done"}, + map[string]any{"name": "security"}, + }, + }, nil + } + + report := evalReplaceLabel(CreatedItemReport{ + Type: "replace_label", + Number: 12, + Repo: "owner/repo", + BeforeState: map[string]any{ + "labels": []any{"in-progress"}, + }, + AfterState: map[string]any{ + "labels": []any{"done"}, + }, + }, "owner/repo") + + assert.Equal(t, OutcomeAccepted, report.Result) + assert.Equal(t, OutcomeStatusAccepted, report.OutcomeStatus) + assert.Equal(t, EvidenceMedium, report.EvidenceStrength) + assert.Equal(t, "state_retained", report.Signal) +} + +func TestEvalReplaceLabelReverted(t *testing.T) { + // Before: [in-progress], After: [done], Current: [in-progress] + // Delta: added=[done], removed=[in-progress] + // "done" absent, "in-progress" back → reverted. + old := outcomeUpdateGHAPIGet + t.Cleanup(func() { + outcomeUpdateGHAPIGet = old + }) + outcomeUpdateGHAPIGet = func(endpoint string, repo string) (map[string]any, error) { + return map[string]any{ + "labels": []any{ + map[string]any{"name": "in-progress"}, + }, + }, nil + } + + report := evalReplaceLabel(CreatedItemReport{ + Type: "replace_label", + Number: 12, + Repo: "owner/repo", + BeforeState: map[string]any{ + "labels": []any{"in-progress"}, + }, + AfterState: map[string]any{ + "labels": []any{"done"}, + }, + }, "owner/repo") + + assert.Equal(t, OutcomeRejected, report.Result) + assert.Equal(t, OutcomeStatusRejected, report.OutcomeStatus) + assert.Equal(t, EvidenceStrong, report.EvidenceStrength) + assert.Equal(t, "state_reverted", report.Signal) +} diff --git a/specs/intent-attribution-agent-governance.md b/specs/intent-attribution-agent-governance.md index 5184edce5df..187d77399c6 100644 --- a/specs/intent-attribution-agent-governance.md +++ b/specs/intent-attribution-agent-governance.md @@ -92,6 +92,33 @@ These capabilities remain supported. In this specification, they are treated as an early implementation of **intent attribution**, not as proof of business impact. +### Safeguards + +When governance policy resolution fails because `.github/objective-mapping.json`, +`.github/intent-policy.json`, or equivalent policy inputs are missing, malformed, +or produce no deterministic match, the implementation MUST fail closed to the +safest policy defined in **Fail-Closed Behavior** above. + +An implementation MUST NOT silently reuse a stale cached policy decision when the +current repository policy inputs cannot be resolved. The failure reason and the +fallback to safe policy decision SHOULD be recorded in execution provenance. + +### Sync Notes + +Until repositories migrate fully to `.github/intent-policy.json`, `.github/objective-mapping.json` +remains the authoritative label-to-intent source for attribution fallback and drift detection. +This specification defines the precedence and sync expectations for that migration, but does +not assign a mandatory repository-by-repository completion deadline. +For this document, "fully migrate" means all active intent keys and governance rules are +defined in `.github/intent-policy.json`, and runtime authorization no longer depends on +reading `.github/objective-mapping.json` except for explicit backward-compatibility paths. + +Implementations SHOULD detect drift by comparing the active repository governance inputs — +the compiled rule set from `.github/intent-policy.json` when present, otherwise the effective +label-to-intent selectors derived from `.github/objective-mapping.json` — during validation +or CI. Keys present in one source but not the other SHOULD surface as a sync warning or +compliance failure so attribution and authorization stay aligned. + ## Product boundary The system can establish: diff --git a/specs/intent-attribution-compliance/README.md b/specs/intent-attribution-compliance/README.md new file mode 100644 index 00000000000..1861b9462a1 --- /dev/null +++ b/specs/intent-attribution-compliance/README.md @@ -0,0 +1,26 @@ +# Intent Attribution Compliance Fixtures + +This directory defines the minimum compliance scenarios for the +[Intent Attribution & Agent Governance Specification](../intent-attribution-agent-governance.md). + +## Required scenarios + +| Scenario | Expected result | Spec references | +|---|---|---| +| Explicit intent wins over linked issues | Attribution uses explicit metadata as the sole source | §RFC 2119 Norms → Attribution-Resolution Order | +| Ambiguous root issue set | Attribution is `status: "ambiguous"` with `source: "closing_issue"` | §RFC 2119 Norms → Ambiguous-Root Handling | +| Unlinked pull request fails closed | Governance resolves to the safest policy (`propose_only`, no writes, approval required) | §RFC 2119 Norms → Fail-Closed Behavior | + +## Fixture guidance + +Each future fixture in this directory SHOULD record: + +- the input artifact shape (explicit metadata, linked issues, labels) +- the expected attribution source and status +- the expected compiled execution policy when attribution is missing or ambiguous + +The minimum fixture set for conformance claims is: + +1. `explicit-intent-wins` +2. `ambiguous-root-closing-issues` +3. `unlinked-pr-fail-closed` diff --git a/specs/otel-observability-spec.md b/specs/otel-observability-spec.md index 990caf4fc2f..bbe4e2f38ab 100644 --- a/specs/otel-observability-spec.md +++ b/specs/otel-observability-spec.md @@ -863,6 +863,17 @@ The following test IDs are stubs for Level 1 (Stable Configuration and Export) c | **T-OT-006** | Local mirrors | The runtime helper writes each exported span as a raw OTLP/HTTP JSON line with a `resourceSpans` key to `/tmp/gh-aw/otel.jsonl`; the file format MUST NOT be an envelope-only summary. | | **T-OT-007** | Compiler config | `observability.otlp.headers` entries are emitted as `OTEL_EXPORTER_OTLP_HEADERS` in `key=value,key=value` format and are masked in diagnostics, job summaries, and artifacts. | +### 17.1.2 Concrete Span-Attribute Contract Cases + +At minimum, the automated compliance suite SHOULD include the following +attribute-level checks mapped directly to Section 10 requirements: + +| Test ID | Attribute requirement | Expected value | Verification method | +|---------|------------------------|----------------|---------------------| +| **T-OT-008** | §10.2 `gh-aw.job.name` on built-in job spans | The setup span includes `gh-aw.job.name` equal to the GitHub Actions job name | Decode `/tmp/gh-aw/otel.jsonl` or captured OTLP payloads and assert the setup span attribute value exactly matches the workflow job name | +| **T-OT-009** | §10.3 `gen_ai.system` on built-in agent spans | A known engine emits a non-empty normalized provider/system value | Run a workflow with a built-in agent span, decode exported spans, and assert `gen_ai.system` is present whenever the engine mapping is known | +| **T-OT-010** | §13.3 `gh-aw.outcome.type` on outcome-evaluation spans | Outcome-evaluation spans emit the safe-output type being evaluated | Execute the outcome collector, inspect the `gh-aw.outcome.evaluate` span, and assert `gh-aw.outcome.type` matches the evaluated manifest item type | + ### 17.2 Optional Extension Tests @@ -885,6 +896,10 @@ The following implementation areas are authoritative for version 0.4.0 compatibi Any change that alters a listed compatibility surface MUST update the corresponding tests in the same change. +Outcome-evaluation span contracts SHOULD stay aligned with the governance and attribution +schema defined in `specs/intent-attribution-agent-governance.md`, especially when intent +context is added to outcome spans or links. + --- ## 18. References diff --git a/specs/replace-label-compliance/README.md b/specs/replace-label-compliance/README.md index 232bc956ed3..c0fae81f5b9 100644 --- a/specs/replace-label-compliance/README.md +++ b/specs/replace-label-compliance/README.md @@ -12,6 +12,7 @@ requirements defined in §4 of the specification. | Filename | Scenario | Spec Coverage | |---|---|---| | `rl-001-glob-semantics.yaml` | Glob pattern matching for `allowed-add`, `allowed-remove`, and `blocked` follows gobwas/glob semantics | RL-001, T-RL-020–T-RL-023 | +| `rl-002-allowlist-enforcement.yaml` | Non-empty allowlists enforce matches while empty allowlists permit any non-blocked label | RL-002, T-RL-021b, T-RL-022b, T-RL-025 | | `rl-003-blocklist-ordering.yaml` | Blocklist evaluation occurs before allowlist evaluation (security boundary) | RL-003, T-RL-023–T-RL-024 | ## Fixture Schema @@ -57,6 +58,9 @@ The following test IDs defined in the replace-label specification map to these f |---------|---------|-------------| | T-RL-020 | `rl-001-glob-semantics.yaml` | Star glob matches label name substring | | T-RL-021 | `rl-001-glob-semantics.yaml` | Exact pattern matches only exact name | +| T-RL-021b | `rl-002-allowlist-enforcement.yaml` | Non-empty allowed-add accepts a matching label | | T-RL-022 | `rl-001-glob-semantics.yaml` | Character class pattern matches correctly | +| T-RL-022b | `rl-002-allowlist-enforcement.yaml` | Non-empty allowed-add rejects a non-matching label | | T-RL-023 | `rl-001-glob-semantics.yaml`, `rl-003-blocklist-ordering.yaml` | Glob pattern rejects non-matching label; blocked label rejected even when allowed | | T-RL-024 | `rl-003-blocklist-ordering.yaml` | Blocked label rejected even with wildcard allowed-add | +| T-RL-025 | `rl-002-allowlist-enforcement.yaml` | Empty allowed-remove permits any non-blocked label | diff --git a/specs/replace-label-compliance/rl-002-allowlist-enforcement.yaml b/specs/replace-label-compliance/rl-002-allowlist-enforcement.yaml new file mode 100644 index 00000000000..722d4ca2782 --- /dev/null +++ b/specs/replace-label-compliance/rl-002-allowlist-enforcement.yaml @@ -0,0 +1,81 @@ +# RL-002 Allowlist Enforcement -- Compliance Fixture +# Tests: T-RL-021, T-RL-022, T-RL-025 +# Spec: §4.1 Allowlist Enforcement (RL-002) + +fixture_id: "rl-002-allowlist-enforcement" +description: > + When an allowlist is non-empty, a label name MUST match at least one configured + pattern before the operation is allowed. Empty or absent allowlists permit any + label name for that direction (RL-002). + +spec_refs: + - "RL-002 — Non-empty allowlists MUST enforce at least one matching pattern" + - "RL-002 — Empty allowlists permit any label name" + - "§4.1 — allowed-add and allowed-remove allowlist evaluation" + - "T-RL-021 — label_to_add matching allowed-add is accepted" + - "T-RL-022 — label_to_add not matching allowed-add is rejected" + - "T-RL-025 — label_to_remove is accepted when allowed-remove is empty" + +scenarios: + - scenario_id: "rl-002-allowed-add-match" + description: "A non-empty allowed-add list permits an exact label match." + input: + safe_output_config: + allowed-add: + - "done" + allowed-remove: [] + blocked: [] + message: + label_to_add: "done" + label_to_remove: "" + expected: + decision: allow + error_code: null + reason: "" + + - scenario_id: "rl-002-allowed-add-miss" + description: "A non-empty allowed-add list rejects a label that matches no pattern." + input: + safe_output_config: + allowed-add: + - "done" + allowed-remove: [] + blocked: [] + message: + label_to_add: "wontfix" + label_to_remove: "" + expected: + decision: deny + error_code: -32002 + reason: "label not in allowed-add list" + + - scenario_id: "rl-002-empty-allowed-remove-permits" + description: "An empty allowed-remove list permits removing any non-blocked label." + input: + safe_output_config: + allowed-add: [] + allowed-remove: [] + blocked: [] + message: + label_to_add: "" + label_to_remove: "state-in-progress" + expected: + decision: allow + error_code: null + reason: "" + + - scenario_id: "rl-002-allowed-remove-miss" + description: "A non-empty allowed-remove list rejects removal when no pattern matches." + input: + safe_output_config: + allowed-add: [] + allowed-remove: + - "state-*" + blocked: [] + message: + label_to_add: "" + label_to_remove: "bug" + expected: + decision: deny + error_code: -32002 + reason: "label not in allowed-remove list" diff --git a/specs/replace-label-spec.md b/specs/replace-label-spec.md index c67c57ff097..8d12683d714 100644 --- a/specs/replace-label-spec.md +++ b/specs/replace-label-spec.md @@ -534,6 +534,10 @@ The test suite for `replace-label` spans two layers: #### 9.2.3 Label Validation Tests +The normative compliance fixtures for the allowlist and blocklist edge cases in +this subsection live in `specs/replace-label-compliance/rl-002-allowlist-enforcement.yaml` +and `specs/replace-label-compliance/rl-003-blocklist-ordering.yaml`. + - **T-RL-020**: Verify that `label_to_add` is accepted when `allowed-add` is empty. - **T-RL-021**: Verify that `label_to_add` matching a pattern in `allowed-add` is accepted. - **T-RL-022**: Verify that `label_to_add` not matching any pattern in a non-empty `allowed-add` is rejected. diff --git a/specs/safe-output-outcome-evaluation.md b/specs/safe-output-outcome-evaluation.md index 17a825bacad..73351a2748e 100644 --- a/specs/safe-output-outcome-evaluation.md +++ b/specs/safe-output-outcome-evaluation.md @@ -111,7 +111,7 @@ Rows marked `evalGenericSticky` fallback are generic existence checks, not type- | `link_sub_issue` | `evalGenericSticky` fallback | sub-issue link target exists | | `hide_comment` | `evalHideComment` | none yet | | `assign_milestone` | `evalAssignMilestone` | milestone still set | -| `replace_label` | `evalGenericSticky` fallback | label target still exists | +| `replace_label` | `evalReplaceLabel` | label replacement retained | | `update_project` | `evalGenericSticky` fallback | object still exists | | `update_release` | `evalGenericSticky` fallback | object still exists | | `noop` | explicit skip | skipped | @@ -122,12 +122,12 @@ Rows marked `evalGenericSticky` fallback are generic existence checks, not type- | `create_pull_request` | implemented | `pkg/workflow/safe_outputs_config.go`, `pkg/workflow/compiler_safe_outputs.go`, `pkg/cli/outcome_eval.go` (`evalCreatePullRequest`) | `actions/setup/js/safe_outputs_handlers.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (PR-specific path) | | `create_issue` | implemented | `pkg/workflow/safe_outputs_config.go`, `pkg/workflow/compiler_safe_outputs.go`, `pkg/cli/outcome_eval.go` (`evalCreateIssue`) | `actions/setup/js/safe_outputs_handlers.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (issue-specific path) | | `add_comment` | implemented | `pkg/workflow/safe_outputs_dispatch.go`, `pkg/cli/outcome_eval.go` (`evalAddComment`) | `actions/setup/js/safe_outputs_handlers.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (issue-comment URL path) | -| `add_labels` | partial | `pkg/workflow/safe_outputs_allowed_labels_validation.go`, `pkg/cli/outcome_eval.go` (`evalAddLabels`) | `actions/setup/js/safe_outputs_handlers.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | +| `add_labels` | partial | `pkg/workflow/safe_outputs_allowed_labels_validation.go`, `pkg/cli/outcome_eval.go` (`evalAddLabels`) | `actions/setup/js/safe_outputs_handlers.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (`evaluateAddLabels`) | | `add_reviewer` | implemented | `pkg/workflow/add_reviewer.go`, `pkg/workflow/safe_outputs_config.go`, `pkg/cli/outcome_eval_review.go` | `actions/setup/js/add_reviewer.cjs`, `actions/setup/js/evaluate_outcomes.cjs` | | `update_issue` | implemented | `pkg/workflow/safe_outputs_config.go`, `pkg/cli/outcome_eval_update.go` | `actions/setup/js/update_issue.cjs`, `actions/setup/js/evaluate_outcomes.cjs` | | `update_pull_request` | implemented | `pkg/workflow/safe_outputs_config.go`, `pkg/cli/outcome_eval_update.go` | `actions/setup/js/update_pull_request.cjs`, `actions/setup/js/evaluate_outcomes.cjs` | -| `close_issue` | partial | `pkg/workflow/safe_outputs_dispatch.go`, `pkg/cli/outcome_eval.go` (`evalCloseSticky`) | `actions/setup/js/close_issue.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | -| `close_pull_request` | partial | `pkg/workflow/safe_outputs_dispatch.go`, `pkg/cli/outcome_eval.go` (`evalCloseSticky`) | `actions/setup/js/close_pull_request.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | +| `close_issue` | partial | `pkg/workflow/safe_outputs_dispatch.go`, `pkg/cli/outcome_eval.go` (`evalCloseSticky`) | `actions/setup/js/close_issue.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (`evaluateCloseIssue`) | +| `close_pull_request` | partial | `pkg/workflow/safe_outputs_dispatch.go`, `pkg/cli/outcome_eval.go` (`evalCloseSticky`) | `actions/setup/js/close_pull_request.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (`evaluateClosePullRequest`) | | `close_discussion` | partial | `pkg/workflow/safe_outputs_dispatch.go`, `pkg/cli/outcome_eval.go` (`evalCloseDiscussion`) | `actions/setup/js/close_discussion.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | | `create_discussion` | partial | `pkg/workflow/safe_outputs_dispatch.go`, `pkg/cli/outcome_eval.go` (`evalCreateDiscussion`) | `actions/setup/js/create_discussion.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | | `update_discussion` | partial | `pkg/workflow/safe_outputs_config.go`, `pkg/cli/outcome_eval_workflow.go` (`evalUpdateDiscussion`) | `actions/setup/js/update_discussion.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | @@ -144,7 +144,7 @@ Rows marked `evalGenericSticky` fallback are generic existence checks, not type- | `link_sub_issue` | not-started | `pkg/workflow/link_sub_issue.go`, `pkg/cli/outcome_eval.go` (`evalGenericSticky` fallback) | `actions/setup/js/link_sub_issue.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | | `hide_comment` | partial | `pkg/workflow/hide_comment.go`, `pkg/cli/outcome_eval.go` (`evalHideComment`) | `actions/setup/js/hide_comment.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | | `assign_milestone` | partial | `pkg/workflow/assign_milestone.go`, `pkg/cli/outcome_eval.go` (`evalAssignMilestone`) | `actions/setup/js/assign_milestone.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | -| `replace_label` | not-started | `pkg/workflow/replace_label.go`, `pkg/cli/outcome_eval.go` (`evalGenericSticky` fallback) | `actions/setup/js/replace_label.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | +| `replace_label` | partial | `pkg/workflow/replace_label.go`, `pkg/cli/outcome_eval.go` (`evalReplaceLabel`) | `actions/setup/js/replace_label.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | | `update_project` | not-started | `pkg/workflow/update_project.go`, `pkg/cli/outcome_eval.go` (`evalGenericSticky` fallback) | `actions/setup/js/update_project.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | | `update_release` | not-started | `pkg/workflow/safe_outputs_config.go`, `pkg/cli/outcome_eval.go` (`evalGenericSticky` fallback) | `actions/setup/js/update_release.cjs`, `actions/setup/js/evaluate_outcomes.cjs` (generic fallback) | | `noop` | implemented | `pkg/cli/outcome_eval.go` (explicit skip in `EvaluateOutcomes`) | `actions/setup/js/evaluate_outcomes.cjs` (`NOOP_TYPES`) |