Harden SPDD specification sync and conformance coverage - #54564
Merged
Merged
Conversation
9 tasks
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] Update sync notes in safe outputs specification
Harden SPDD specification sync and conformance coverage
Aug 21, 2026
Contributor
Contribution ApprovedExcellent work hardening the SPDD specification sync and conformance coverage! This PR addresses all the items from the daily spec review (#54563) in a focused, well-documented manner. Summary
Changes Verified
Ready for merge. No additional work required.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens SPDD specifications and links requirements to conformance coverage.
Changes:
- Clarifies guard-policy precedence and drift-check cadence.
- Adds safe-output implementation mappings.
- Registers CR-06a and T-GH-94 conformance identifiers.
Show a summary per file
| File | Description |
|---|---|
specs/awf-config-sources-spec.md |
Updates cadence, safeguards, and CR-06a traceability. |
specs/awf-config-sources-compliance/README.md |
Registers T-DR-011. |
scratchpad/safe-outputs-specification.md |
Adds synchronization guidance. |
scratchpad/guard-policies-specification.md |
Centralizes evaluation order. |
scratchpad/github-mcp-access-control-specification.md |
Documents deny-list precedence and empty scopes. |
pkg/workflow/awf_config_safeguards_formal_test.go |
Associates CR-06a tests with T-DR-011. |
pkg/workflow/awf_config_conformance_registry_formal_test.go |
Advances registry expectations. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Suppressed comments (1)
pkg/workflow/awf_config_safeguards_formal_test.go:124
- This newly registered T-DR-011 test checks only the upper deadline. The predicate it exercises returns true for an acknowledgement timestamp before
assignedAt, so the fixture does not establish that acknowledgement occurred after assignment. Add the lower-bound check and a regression assertion for a pre-assignment timestamp.
func TestAWFConfigSafeguard_TDR011_EscalationAcknowledgementWindow(t *testing.T) {
assignedFriday := time.Date(2026, 8, 7, 9, 0, 0, 0, time.UTC)
mondayDeadline := time.Date(2026, 8, 10, 9, 0, 0, 0, time.UTC)
assert.True(t, formalEscalationAcknowledgedWithinOneBusinessDay(assignedFriday, mondayDeadline))
assert.False(t, formalEscalationAcknowledgedWithinOneBusinessDay(assignedFriday, mondayDeadline.Add(time.Nanosecond)))
- Files reviewed: 7/7 changed files
- Comments generated: 6
- Review effort level: Balanced
|
|
||
| The MCP Gateway MUST evaluate access in this order: | ||
|
|
||
| 1. `lockdown: true` takes absolute precedence and denies the invocation; `allowed-repos` and `min-integrity` MUST NOT be evaluated. |
| A scheduled GitHub Actions workflow in `github/gh-aw` SHOULD automate this procedure. The workflow SHOULD: | ||
|
|
||
| - Run on a weekly schedule and on pull requests that touch AWF config handling. | ||
| - Run on a daily schedule and on pull requests that touch AWF config handling. |
| ``` | ||
| pkg/workflow/awf_config_drift_test.go — DriftRecord schema validation and usage (T-DR-001 through T-DR-010; T-DR-005: TestDriftRecord_TDR005_NoAdditionalProperties) | ||
| pkg/workflow/awf_config_safeguards_formal_test.go — unavailable-source safeguards (T-DR-SAFE-001 through T-DR-SAFE-004) | ||
| pkg/workflow/awf_config_safeguards_formal_test.go — unavailable-source safeguards (T-DR-SAFE-001 through T-DR-SAFE-004) and CR-06a escalation-owner acknowledgement (T-DR-011) |
| | T-DR-008 | §7.5.1 — corrective PR embeds records | The corrective PR description MUST embed the full `DriftRecord` list as JSON. | `pkg/workflow/awf_config_drift_test.go` | | ||
| | T-DR-009 | §7.5.1 — empty list is valid | An empty `DriftRecord` list (no drift detected) is a valid output and MUST NOT trigger corrective PR or escalation actions. | `pkg/workflow/awf_config_drift_test.go` | | ||
| | T-DR-010 | §7.2 Step 5 integration | The drift detection procedure Step 5 MUST produce a list of zero or more `DriftRecord` objects; the output format MUST be a JSON array conforming to the §3.1 schema. | `pkg/workflow/awf_config_drift_test.go` | | ||
| | T-DR-011 | §6 CR-06a — escalation-owner acknowledgement | Escalations select a non-empty owner using the documented fallback and require acknowledgement within one business day. | `pkg/workflow/awf_config_safeguards_formal_test.go` | |
| - **T-GH-091**: When both `blocked-users` and `min-integrity` are configured, access is the conjunction of P5_NotBlocked AND P6_IntegrityMet; a non-blocked user with content at or above the threshold is allowed | ||
| - **T-GH-092**: Non-blocked user with content exceeding the configured threshold is allowed; P5 and P6 both pass | ||
| - **T-GH-093**: Blocked user whose content also fails the integrity threshold is denied with `-32005` (P5_NotBlocked fires before P6_IntegrityMet per §8.5 combined evaluation order), not `-32006` | ||
| - **T-GH-094**: A user in both `blocked-users` and `trusted-users` is denied because `blocked-users` takes precedence |
| | Specification area | Implementation mapping | | ||
| |---|---| | ||
| | §3.4 Layer 3 validation guardrails | `pkg/workflow/safe_outputs_validation.go`, `pkg/workflow/safe_outputs_validation_config.go` | | ||
| | §3.5 Layer 4 execution handlers | `pkg/workflow/safe_output_handlers.go`, `pkg/workflow/safe_outputs_actions.go` | |
Contributor
|
🎉 This pull request is included in a new release. Release: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses SPDD gaps across access control, guard policies, safe outputs, and AWF config-source specifications. The changes clarify ambiguous precedence and cadence rules while linking requirements to implementation and conformance coverage.
Safe Outputs sync
Sync NotesandSync Follow-ups.pkg/workflow/implementation files.Access-control and guard-policy rules
T-GH-094).lockdown→allowed-repos→min-integrityevaluation order.AWF config-source conformance
T-DR-011.Cadence and safeguard traceability