Address SPDD safe-output spec gaps

actions/setup/js/replace_label.cjs

@@ -33,6 +33,17 @@ const { createCountGatedHandler } = require("./handler_scaffold.cjs");
 const { withRetry, RATE_LIMIT_RETRY_CONFIG } = require("./error_recovery.cjs");
 const { resolveInvocationContext } = require("./invocation_context_helpers.cjs");
 
+const POLICY_REJECTION_ERROR_NAME = "ReplaceLabelPolicyRejectionError";
+const SET_LABELS_RETRY_CONFIG = {
+  ...RATE_LIMIT_RETRY_CONFIG,
+  shouldRetry: error => {
+    const status = error?.response?.status ?? error?.status ?? null;
+    const retryableHttpStatus = typeof status === "number" && status >= 500 && status < 600;
+    const retryableTransportError = status == null && RATE_LIMIT_RETRY_CONFIG.shouldRetry(error);
+    return error?.name !== POLICY_REJECTION_ERROR_NAME && (retryableHttpStatus || retryableTransportError || RATE_LIMIT_RETRY_CONFIG.shouldRetry(error));
+  },
+};
+
 /**
  * Validate a single label against blocked and allowed-list patterns.
  * Uses explicit rejection semantics — does not silently filter or truncate the label name.
@@ -68,23 +79,26 @@ function validateSingleLabel(labelName, allowedPatterns, blockedPatterns, fieldN
 const main = createCountGatedHandler({
   handlerType: HANDLER_TYPE,
   setup: async (config, maxCount, isStaged) => {
-    const blockedPatterns = config.blocked || [];
+    const currentAllowedAdd = () => (Array.isArray(config.allowed_add) ? config.allowed_add : []);
+    const currentAllowedRemove = () => (Array.isArray(config.allowed_remove) ? config.allowed_remove : []);
+    const currentBlockedPatterns = () => (Array.isArray(config.blocked) ? config.blocked : []);
     const requiredLabels = Array.isArray(config.required_labels) ? config.required_labels : [];
     const requiredTitlePrefix = config.required_title_prefix || "";
     const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config);
     const githubClient = await createAuthenticatedGitHubClient(config);
 
     // Config keys use snake_case (set by the Go handler config builder)
-    const configAllowedAdd = Array.isArray(config.allowed_add) ? config.allowed_add : [];
-    const configAllowedRemove = Array.isArray(config.allowed_remove) ? config.allowed_remove : [];
+    const initialAllowedAdd = currentAllowedAdd();
+    const initialAllowedRemove = currentAllowedRemove();
+    const initialBlockedPatterns = currentBlockedPatterns();
     /** @type {{from: string, to: string}[]} */
     const configAllowedTransitions = Array.isArray(config.allowed_transitions) ? config.allowed_transitions : [];
 
     core.info(`Replace label configuration: max=${maxCount}`);
     if (configAllowedTransitions.length > 0) core.info(`Allowed transitions: ${configAllowedTransitions.map(t => `"${t.from}" → "${t.to}"`).join(", ")}`);
-    if (configAllowedAdd.length > 0) core.info(`Allowed labels to add: ${configAllowedAdd.join(", ")}`);
-    if (configAllowedRemove.length > 0) core.info(`Allowed labels to remove: ${configAllowedRemove.join(", ")}`);
-    if (blockedPatterns.length > 0) core.info(`Blocked patterns: ${blockedPatterns.join(", ")}`);
+    if (initialAllowedAdd.length > 0) core.info(`Allowed labels to add: ${initialAllowedAdd.join(", ")}`);
+    if (initialAllowedRemove.length > 0) core.info(`Allowed labels to remove: ${initialAllowedRemove.join(", ")}`);
+    if (initialBlockedPatterns.length > 0) core.info(`Blocked patterns: ${initialBlockedPatterns.join(", ")}`);
     if (requiredLabels.length > 0) core.info(`Required labels (all): ${requiredLabels.join(", ")}`);
     if (requiredTitlePrefix) core.info(`Required title prefix: ${requiredTitlePrefix}`);
     core.info(`Default target repo: ${defaultTargetRepo}`);
@@ -131,14 +145,14 @@ const main = createCountGatedHandler({
       }
 
       // Validate label_to_remove against blocked patterns and allowed-remove list
-      const removeValidation = validateSingleLabel(labelToRemove, configAllowedRemove, blockedPatterns, "label_to_remove");
+      const removeValidation = validateSingleLabel(labelToRemove, initialAllowedRemove, initialBlockedPatterns, "label_to_remove");
       if (!removeValidation.valid) {
         core.warning(`label_to_remove validation failed: ${removeValidation.error}`);
         return { success: false, error: removeValidation.error };
       }
 
       // Validate label_to_add against blocked patterns and allowed-add list
-      const addValidation = validateSingleLabel(labelToAdd, configAllowedAdd, blockedPatterns, "label_to_add");
+      const addValidation = validateSingleLabel(labelToAdd, initialAllowedAdd, initialBlockedPatterns, "label_to_add");
       if (!addValidation.valid) {
         core.warning(`label_to_add validation failed: ${addValidation.error}`);
         return { success: false, error: addValidation.error };
@@ -196,26 +210,42 @@ const main = createCountGatedHandler({
       // Compute the new label set: current labels minus labelToRemove, plus labelToAdd (deduped).
       // If labelToRemove is not on the issue we still proceed — it simply won't appear in the set.
       const currentLabelNames = (item.labels || []).map(/** @param {any} l */ l => (typeof l === "string" ? l : l.name || "")).filter(Boolean);
-      const labelToRemoveIsPresent = currentLabelNames.includes(labelToRemove);
+      let labelToRemoveIsPresent = currentLabelNames.includes(labelToRemove);
       if (!labelToRemoveIsPresent) {
         core.info(`Label "${labelToRemove}" is not present on ${contextType} #${itemNumber} in ${itemRepo} — will only add "${labelToAdd}"`);
       }
-      const newLabelNames = [...new Set([...currentLabelNames.filter(n => n !== labelToRemove), labelToAdd])];
-
-      core.info(`Executing REST setLabels: remove="${labelToRemove}", add="${labelToAdd}" on ${contextType} #${itemNumber} in ${itemRepo}`);
-
-      const beforeState = await fetchIssueState(githubClient, repoParts, itemNumber);
 
       try {
+        let beforeState;
         const { data: updatedLabels } = await withRetry(
-          () =>
-            githubClient.rest.issues.setLabels({
+          async () => {
+            beforeState = await fetchIssueState(githubClient, repoParts, itemNumber);
+            const preWriteRemoveValidation = validateSingleLabel(labelToRemove, currentAllowedRemove(), currentBlockedPatterns(), "label_to_remove");
+            if (!preWriteRemoveValidation.valid) {
+              core.warning(`label_to_remove validation failed before setLabels: ${preWriteRemoveValidation.error}`);
+              const policyError = new Error(preWriteRemoveValidation.error);
+              policyError.name = POLICY_REJECTION_ERROR_NAME;
+              throw policyError;
+            }
+            const preWriteAddValidation = validateSingleLabel(labelToAdd, currentAllowedAdd(), currentBlockedPatterns(), "label_to_add");
+            if (!preWriteAddValidation.valid) {
+              core.warning(`label_to_add validation failed before setLabels: ${preWriteAddValidation.error}`);
+              const policyError = new Error(preWriteAddValidation.error);
+              policyError.name = POLICY_REJECTION_ERROR_NAME;
+              throw policyError;
+            }
+            const beforeWriteLabelNames = normalizeLabelNames(beforeState.labels);
+            labelToRemoveIsPresent = beforeWriteLabelNames.includes(labelToRemove);
+            const newLabelNames = [...new Set([...beforeWriteLabelNames.filter(n => n !== labelToRemove), labelToAdd])];
+
+            return githubClient.rest.issues.setLabels({
               owner: repoParts.owner,
               repo: repoParts.repo,
               issue_number: itemNumber,
               labels: newLabelNames,
-            }),
-          RATE_LIMIT_RETRY_CONFIG,
+            });
+          },
+          SET_LABELS_RETRY_CONFIG,
           `replace_label on ${contextType} #${itemNumber} in ${itemRepo}`
         );
 

actions/setup/js/replace_label.test.cjs

@@ -147,6 +147,41 @@ describe("replace_label", () => {
     expect(result.success).toBe(false);
   });
 
+  it("should reject label_to_add before setLabels when blocklist changes mid-flight", async () => {
+    let setLabelsCalls = 0;
+    let getCalls = 0;
+    const config = { allowed_add: ["done"], blocked: [] };
+    mockGithub.rest.issues.get = async () => {
+      getCalls++;
+      if (getCalls === 2) {
+        config.blocked = ["done"];
+      }
+      return {
+        data: {
+          title: "Test issue title",
+          labels: [
+            { name: "in-progress", node_id: "LA_in_progress_123" },
+            { name: "bug", node_id: "LA_bug_456" },
+          ],
+          node_id: "I_issue_789",
+        },
+      };
+    };
+    mockGithub.rest.issues.setLabels = async () => {
+      setLabelsCalls++;
+      return { data: [] };
+    };
+
+    const handler = await main(config);
+    const result = await handler({ label_to_remove: "in-progress", label_to_add: "done" }, {});
+
+    expect(result.success).toBe(false);
+    expect(result.error).toContain("blocked pattern");
+    expect(getCalls).toBe(2);
+    expect(config.blocked).toEqual(["done"]);
+    expect(setLabelsCalls).toBe(0);
+  });
+
   it("should skip when required-labels filter does not match", async () => {
     const handler = await main({ required_labels: ["approved"] });
     // Issue has "in-progress" and "bug" but not "approved"

pkg/workflow/replace_label_formal_test.go

@@ -570,34 +570,66 @@ func TestFormalStagedMode(t *testing.T) {
 	assert.Equal(t, "true", string(*cfg.Staged))
 }
 
+type formalRunReplaceLabelOpts struct {
+	Staged      bool
+	LabelToAdd  string
+	AllowedAdd  []string
+	Blocked     []string
+	BeforeWrite func() []string
+	OnWrite     func()
+}
+
 // formalRunReplaceLabel models the core execute path of the replace_label handler.
-// onWrite is invoked exactly once when the handler would call the write API
+// OnWrite is invoked exactly once when the handler would call the write API
 // (issues.setLabels). In staged mode the handler must return before reaching
-// onWrite; this is the invariant asserted by TestFormalStagedMode_NoWriteAPI.
-func formalRunReplaceLabel(staged bool, onWrite func()) formalReplaceLabelOutcome {
-	if staged {
+// OnWrite; this is the invariant asserted by TestFormalStagedMode_NoWriteAPI.
+func formalRunReplaceLabel(opts formalRunReplaceLabelOpts) formalReplaceLabelOutcome {
+	if opts.Staged {
 		return formalReplaceLabelOutcome{Success: true, Staged: true}
 	}
-	onWrite()
+	blocked := opts.Blocked
+	if opts.BeforeWrite != nil {
+		blocked = opts.BeforeWrite()
+	}
+	if err := formalValidateSingleLabel(opts.LabelToAdd, opts.AllowedAdd, blocked, "label_to_add"); err != nil {
+		return formalReplaceLabelOutcome{Success: false}
+	}
+	opts.OnWrite()
 	return formalReplaceLabelOutcome{Success: true}
 }
 
 func TestFormalStagedMode_NoWriteAPI(t *testing.T) {
 	writeCalls := 0
-	outcome := formalRunReplaceLabel(true, func() { writeCalls++ })
+	outcome := formalRunReplaceLabel(formalRunReplaceLabelOpts{Staged: true, LabelToAdd: "done", OnWrite: func() { writeCalls++ }})
 	assert.True(t, outcome.Success)
 	assert.True(t, outcome.Staged)
 	assert.Zero(t, writeCalls, "staged mode must not call the write API (setLabels)")
 }
 
 func TestFormalNonStagedMode_InvokesWriteAPI(t *testing.T) {
 	writeCalls := 0
-	outcome := formalRunReplaceLabel(false, func() { writeCalls++ })
+	outcome := formalRunReplaceLabel(formalRunReplaceLabelOpts{LabelToAdd: "done", OnWrite: func() { writeCalls++ }})
 	assert.True(t, outcome.Success)
 	assert.False(t, outcome.Staged)
 	assert.Equal(t, 1, writeCalls, "non-staged mode must invoke the write API exactly once")
 }
 
+func TestFormalBlockedLabelAddedViaReplaceLabelMidBlocklistChange_NoWriteAPI(t *testing.T) {
+	writeCalls := 0
+	outcome := formalRunReplaceLabel(formalRunReplaceLabelOpts{
+		LabelToAdd: "done",
+		AllowedAdd: []string{"done"},
+		Blocked:    []string{},
+		BeforeWrite: func() []string {
+			return []string{"done"}
+		},
+		OnWrite: func() { writeCalls++ },
+	})
+
+	assert.False(t, outcome.Success)
+	assert.Zero(t, writeCalls, "label_to_add blocked before setLabels must be rejected without calling the write API")
+}
+
 func TestFormalSingleRESTCall(t *testing.T) {
 	data, err := os.ReadFile(filepath.Join(formalRepoRoot(t), "actions", "setup", "js", "replace_label.cjs"))
 	require.NoError(t, err)

specs/replace-label-spec.md

@@ -457,6 +457,10 @@ In the Octokit client this is `githubClient.rest.issues.setLabels(params)`.
 
 **RL-046**: When the `setLabels` REST call fails (e.g., HTTP 422 for an invalid label name), the implementation MUST log a `core.error()` entry and MUST return `{ success: false, error: <message> }`. For HTTP-level failures the call is all-or-nothing — either all label changes are applied or none are. For HTTP 200 responses, see §7.4 for partial-success response handling.
 
+**RL-046a**: If `setLabels` returns HTTP 404, the implementation MUST treat the request as rejected because the authoritative labeling target or repository is no longer reachable.
+
+**RL-046b**: If `setLabels` returns HTTP 5xx, times out, or fails with a transport error, the implementation MUST treat the condition as transient and retry according to the retry policy in §7.3. If retries are exhausted, the implementation MUST return `{ success: false, error: <message> }` without reporting a successful replacement.
+
 ### 7.3 Rate-Limit Retry Policy
 
 **RL-048**: The `setLabels` REST call (Stage 8) MUST apply the `RATE_LIMIT_RETRY_CONFIG` retry policy from `actions/setup/js/error_recovery.cjs`. This policy covers secondary rate-limit responses (HTTP 403 with Retry-After header) and primary rate-limit responses (HTTP 429).
@@ -486,6 +490,8 @@ Label allowlists and blocklists are the primary mechanism preventing AI agents f
 
 **RL-049**: Allowlist and blocklist evaluation MUST be performed server-side (in the JavaScript handler executing within GitHub Actions), not by the AI agent. Agents MUST NOT be trusted to self-enforce label restrictions.
 
+**RL-049a**: A conforming implementation MUST re-check `label_to_add` against the current server-side allowlist and blocklist state immediately before invoking `PUT /repos/{owner}/{repo}/issues/{issue_number}/labels`. If the label is blocked or no longer allowed at that point, the implementation MUST reject the message without calling the write API.
+
 ### 8.2 Cross-Repository Restrictions
 
 By default, `replace-label` operates on the repository of the triggering workflow. Cross-repository operation is opt-in and must be explicitly declared.
@@ -518,6 +524,12 @@ Staged mode provides a mechanism for operators to audit AI agent label-transitio
 
 **RL-056**: When `staged: true`, the implementation MUST NOT call any write API endpoint. Read-only API calls performed during Stage 5 gate checks MAY proceed in staged mode.
 
+The staged-mode preview SHOULD identify the `label_to_add` value that would be revalidated before the write call so operators can compare previewed transitions against protected-label policy before disabling staged mode.
+
+### 8.7 Sync Notes
+
+The REST failure and retry semantics in [Section 7](#7-error-handling) are mirrored by the `replace_label` outcome-evaluation rules in [`safe-output-outcome-evaluation.md` Section 30](safe-output-outcome-evaluation.md#30-replace_label). Changes to `404`, `5xx`, or `429` handling in either document SHOULD be reviewed against the other document in the same change.
+
 ---
 
 ## 9. Compliance Testing

specs/safe-output-outcome-evaluation.md

@@ -683,8 +683,9 @@ No outcome to evaluate. Skip.
 | `label_to_add` is present on the item AND `label_to_remove` is absent | `accepted` |
 | `label_to_add` is absent from the item | `rejected` |
 | `label_to_add` is present but `label_to_remove` is also still present | `rejected` (partial failure — remove did not apply) |
-| Item not found (`404`) | `rejected` |
-| API transient failure (`5xx`, timeout, transport error) | `pending` |
+| Item not found (`404`) | Outcome evaluation workers **MUST** classify as `rejected` |
+| API transient failure (`5xx`, timeout, transport error) | Outcome evaluation workers **MUST** classify as `pending` |
+| Rate-limit response (`403` exhaustion or `429`) | Outcome evaluation workers **MUST** classify as `pending` and **SHOULD** reschedule using the reset window |
 | `lifecycle` | N/A — `replace_label` has no lifecycle bot-close behavior |
 | `lifecycle_close` | N/A — `replace_label` has no lifecycle bot-close behavior |
 | `ignored` | N/A — label state is always evaluable when the item is accessible; no time-bounded engagement signal applies |
@@ -710,6 +711,8 @@ No outcome to evaluate. Skip.
 3. If the API returns rate-limit responses (`403` exhaustion or `429`), outcome evaluation workers **MUST** classify as `pending` and reschedule evaluation using the reset window.
 4. While any transient API failure condition exists, outcome evaluation workers **MUST NOT** emit `accepted` or `rejected` for label replacement state.
 
+**Sync note:** Keep the API failure safeguards above aligned with [`replace-label-spec.md` Section 7](replace-label-spec.md#7-error-handling), which defines the shared `404`, `5xx`, and `429` REST failure semantics for `replace_label`.
+
 **References:** See [replace-label-spec.md](replace-label-spec.md) for the full definition of the `replace_label` safe-output type, including the message schema, processing model, and REST interface.
 
 ---

specs/safe-outputs-scratchpad-removal.md

@@ -5,5 +5,6 @@ The deprecated [`scratchpad/safe-outputs-specification.md`](../scratchpad/safe-o
 ## Removal checklist
 
 - [ ] Before 2026-09-21, replace references to the deprecated scratchpad specification in doc-site navigation, workflow files, and internal links with the canonical specification.
+- [ ] Owner: SPDD daily rotation. Before 2026-09-21, verify `grep -r "scratchpad/safe-outputs-specification.md" docs/ .github/` returns zero matches outside this removal notice before deleting the scratchpad file.
 - [ ] On or before 2026-09-21, delete `scratchpad/safe-outputs-specification.md`.
 - [ ] Verify that no remaining repository references resolve to the deleted scratchpad path.

specs/security-architecture-spec-summary.md

@@ -4,6 +4,7 @@
 **Version**: 1.0.0  
 **Status**: Candidate Recommendation  
 **Date**: January 29, 2026
+**Last validated**: v1.0.0 / 2026-07-15
 
 ## Overview
 
@@ -313,16 +314,18 @@ The specification documents the **current implementation** in gh-aw version 1.0.
 
 Key implementation files referenced in the specification:
 
-- `pkg/workflow/safe_inputs_parser.go` - Input sanitization
-- `pkg/workflow/safe_outputs_config.go` - Output isolation
+- `pkg/workflow/safe_outputs_parser.go` - Safe-output frontmatter parsing
+- `pkg/workflow/safe_outputs_config_base.go` - Shared safe-output configuration
+- `pkg/workflow/safe_outputs_config_types.go` - Safe-output type configuration
 - `pkg/workflow/engine.go` - Network permissions
 - `pkg/workflow/compiler_safe_outputs.go` - Safe output compilation
 - `pkg/workflow/safe_jobs.go` - Threat detection
 - `pkg/workflow/compiler_types.go` - Core types
-- Actions in `actions/setup/js/*.cjs` and `actions/setup/sh/*.sh`
 
 ### Spec-to-Lock Sync (v1.0.0)
 
+Summary version **1.0.0** was last validated on **2026-07-15**, matching the validation marker in `specs/security-architecture-spec-validation.md`.
+
 Summary version **1.0.0** corresponds to the minimum validated `.lock.yml` compiler behaviors recorded in `specs/security-architecture-spec-validation.md`:
 
 - Activation, agent, detection, and safe output jobs remain separated in compiled workflows

specs/security-architecture-spec-validation.md

@@ -2,6 +2,7 @@
 
 **Document**: Validation of `security-architecture-spec.md` against compiled `.lock.yml` files  
 **Date**: July 6, 2026  
+**Last validated**: v1.0.0 / 2026-07-15
 **Validator**: GitHub Copilot Agent  
 **Scope**: Cross-reference specification requirements with actual implementation