Skip to content

Fix safe-outputs body:null parsing to enable field updates#15129

Merged
pelikhan merged 4 commits intomainfrom
copilot/fix-safe-outputs-max-config
Feb 12, 2026
Merged

Fix safe-outputs body:null parsing to enable field updates#15129
pelikhan merged 4 commits intomainfrom
copilot/fix-safe-outputs-max-config

Conversation

Copy link
Contributor

Copilot AI commented Feb 12, 2026

parseUpdateEntityBoolField returned nil for body: null configurations, causing the field to be treated as disabled. The parser now treats YAML null values as explicit enablement (true) in FieldParsingBoolValue mode.

Changes

  • Core logic: Added explicit nil check in FieldParsingBoolValue case to return &true for backward compatibility
  • Test updates: Updated TestUpdateIssueBodyNullBackwardCompatibility and TestParseUpdateEntityBoolField to expect non-nil pointer
  • Documentation: Clarified nil-as-true behavior in function and usage comments

Behavior

safe-outputs:
  update-issue:
    body: true   # enabled
    body: false  # disabled
    body:        # enabled (treated as true)

Applies to body/footer fields in update-issue, update-pull-request, update-discussion, and update-release.

Technical Details

case FieldParsingBoolValue:
    if boolVal, ok := val.(bool); ok {
        return &boolVal
    }
    if val == nil {
        trueVal := true
        return &trueVal  // Backward compatibility
    }
    return nil  // Invalid non-bool values
Original prompt

This section details on the original issue you should resolve

<issue_title>[CI Failure Doctor] Safe outputs max config misinterprets update-issue body null</issue_title>
<issue_description># 🏥 CI Failure Investigation - Run github/gh-aw#35059

Summary

The test job in run 21943609711 failed because the safe-outputs max configuration test expected the update-issue body field to be marked as updatable even when it was configured as body: null in the frontmatter map.

Failure Details

Root Cause Analysis

parseUpdateEntityBoolField returned nil whenever a bool field was present but not a bool (e.g., body: null). The safe-outputs max config test is verifying that a complete update-issue configuration marks the body as updatable when body: null, so the nil pointer prevented the test from seeing the field as enabled.

Failed Jobs and Errors

  • test: TestSafeOutputsMaxConfiguration/Complete_configuration_with_all_options
    • safe_outputs_max_test.go:200: Expected UpdateIssues.Body to be non-nil (updatable)
  • test: TestSafeOutputsMaxConfiguration
    • The suite fails because the same expectation bubbles up during the aggregate configuration checks.

Investigation Findings

  • FieldParsingBoolValue now immediately returns a pointer to true when the incoming value is nil, so body: null explicitly enables the field instead of yielding nil.
  • TestUpdateIssueBodyNull was updated to assert the new behavior so that the logic now checks that null bodies default to true while still allowing body: false to disable the field.
  • Local re-runs of the package were blocked because go requires >=1.25.0 but the runner cannot download the new toolchain from proxy.golang.org.

Recommended Actions

  • Update parseUpdateEntityBoolField to treat nil values as allowances for update-entity bool fields so safe-outputs configs mark those fields as updatable.
  • Adjust TestUpdateIssueBodyNull to expect that body: null yields a non-nil pointer that defaults to true.
  • Document that null bool values in safe-outputs configs now behave as explicit true to avoid future test mismatches.

Prevention Strategies

Add regression coverage for other bool fields parsed via FieldParsingBoolValue so that any future change to the helper cannot accidentally treat null as nil again. Update the docs/reference frontmatter guidance to call out this behavior explicitly.

AI Team Self-Improvement

When writing tests or helpers for safe-outputs bool flags, treat null the same as an explicit true (i.e., the agent may rely on defaults) and document that behavior in the comment above FieldParsingBoolValue.

Historical Context

No identical failure was found in prior investigations; this regression was first observed in run github/gh-aw#35059 (this investigation).

AI generated by CI Failure Doctor

To add this workflow in your repository, run gh aw add githubnext/agentics/workflows/ci-doctor.md@ea350161ad5dcc9624cf510f134c6a9e39a6f94d. See usage guide.

  • expires on Feb 13, 2026, 11:04 AM UTC

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits February 12, 2026 11:19
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix safe outputs max config for update-issue body interpretation Fix safe-outputs body:null parsing to enable field updates Feb 12, 2026
Copilot AI requested a review from pelikhan February 12, 2026 11:26
@pelikhan pelikhan marked this pull request as ready for review February 12, 2026 11:29
Copilot AI review requested due to automatic review settings February 12, 2026 11:29
@pelikhan pelikhan merged commit 8566eb1 into main Feb 12, 2026
159 checks passed
@pelikhan pelikhan deleted the copilot/fix-safe-outputs-max-config branch February 12, 2026 11:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the parsing of body: null configurations in safe-outputs update entity handlers. Previously, null values were parsed as nil pointers, which relied on downstream default handling. Now, null values are explicitly treated as true during parsing, maintaining backward compatibility while making the behavior more explicit and consistent.

Changes:

  • Modified parseUpdateEntityBoolField to treat null values as true in FieldParsingBoolValue mode for backward compatibility
  • Updated test expectations to verify that null values result in non-nil pointers set to true
  • Regenerated lock files to reflect consistent handler configurations with explicit allow_body: true

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/workflow/update_entity_helpers.go Added nil check in FieldParsingBoolValue case to return &true instead of nil; updated documentation to clarify nil-as-true behavior
pkg/workflow/update_entity_helpers_test.go Updated test to expect non-nil pointer with true value when parsing nil in BoolValue mode
pkg/workflow/update_issue_test.go Updated backward compatibility test to verify nil values parse as &true
pkg/workflow/data/action_pins.json Added actions/setup-go@v5 pin discovered during recompilation
.github/workflows/ci-doctor.lock.yml Added explicit allow_body: true in handler config
.github/workflows/sub-issue-closer.lock.yml Added explicit allow_body: true in handler config
.github/workflows/workflow-health-manager.lock.yml Added explicit allow_body: true in handler config
Comments suppressed due to low confidence (1)

pkg/workflow/update_entity_helpers.go:206

  • The comment incorrectly states that FieldParsingBoolValue is "Used by body/footer fields in all update entities". This is inaccurate:
  • update-issue: body, footer ✓
  • update-pull-request: title, body (not just body/footer)
  • update-discussion: footer only (body uses KeyExistence)
  • update-release: footer only (no body field)

The comment should accurately describe which fields use this mode.

	// FieldParsingBoolValue mode: Field's boolean value determines if it can be updated.
	// Special case: nil values are treated as true for backward compatibility.
	// Used by body/footer fields in all update entities.
	FieldParsingBoolValue

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +43 to 55
// 2. **Bool Value Mode** (for body/footer fields in all entities):
// Fields are enabled based on explicit boolean values.
// Special case: null values are treated as true for backward compatibility:
// ```yaml
// update-issue:
// body: true # Explicitly enable body updates
// body: false # Explicitly disable body updates
// body: null # Treated as true (backward compatibility)
// body: # Same as null, treated as true
// update-pull-request:
// title: true # Can update title
// body: false # Cannot update body
// title: true # Can update title
// body: false # Cannot update body
// ```
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation incorrectly states that FieldParsingBoolValue mode is used for "body/footer fields in all entities". However, this is not accurate:

  • update-issue uses it for: body, footer (correct)
  • update-pull-request uses it for: title, body (not just body/footer)
  • update-discussion uses it for: footer only (body uses KeyExistence mode)
  • update-release uses it for: footer only (no body field)

The documentation should be updated to accurately reflect which fields use this mode, or use a more general description like "certain update fields that support explicit true/false configuration".

This issue also appears on line 203 of the same file.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI Failure Doctor] Safe outputs max config misinterprets update-issue body null

3 participants