SPDD 2026-06-30: tighten normative requirements across 5 specs

[Copilot]

Five spec files had gaps identified in daily SPDD review: missing normative language, undefined terms, unspecified failure modes, and advisory-only integrity requirements that should be binding.

bash-command-parser-specification.md

• §2.2: MUST-level requirement for Class I consumers to apply default-deny when parser returns empty list or nil
• §9: Pinned canonical test vector path to specs/test-vectors/bash-command-parser/<version>-<source>.json; upgraded storage requirement from SHOULD to MUST
• §11: Expanded security section with three concrete misuse examples (sandbox-proof misuse, empty-result allow, single-command bypass) and a normative MUST NOT in new §11.2 covering all parser output types

ai-credits-specification.md

• §5.4 (new): MUST-level sync failure handling — unreachable source falls back to last valid catalog, malformed updates are rejected entirely, missing input/output fields skip the entry, and divergent catalog paths block normal operation
• Appendix C: Upgraded all three integrity requirements from SHOULD to MUST; added specificity around authorized-upstream confirmation and reviewer qualifications

copilot-sdk-driver-specification.md

• §6.5 (new): Normative field tables for lifecycle event logs (7 fields) and policy-denial log entries (6 fields) with MUST/MAY qualifications
• Appendix C: Added Glossary defining "non-ephemeral events" normatively — events persisted to durable storage, distinct from ephemeral in-memory/transient I/O state

security-architecture-spec-validation.md

• Re-validation Triggers: Converted "Revalidation Cadence" section to normative MUST; added 5-step failure escalation path (downgrade grade → open tracking issue → mark table row ❌ → block dependent merges → restore grade after re-verify)
• Automation Approach (new): Two concrete CI mechanism proposals — compiled YAML structural assertions (job presence/ordering, permission sets, SHA pinning, concurrency expression) and a spec-to-implementation linter hook in gh aw compile
• §12 Gap Analysis (new): Full audit of 9 test categories from security-architecture-spec.md §12 against existing evidence; Sandbox Isolation (T-SI-001–007) flagged as highest-priority gap with zero evidence; 5 others partially evidenced

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

No test files were added or modified in this PR. All 4 changed files are specification/documentation markdown files (docs/src/content/docs/specs/.md and specs/.md). Test Quality Sentinel skipped.

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #42530 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100).

[github-actions]

🧠 Matt Pocock Skills Reviewer is reviewing this pull request using Matt Pocock's engineering skills...

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[Copilot]

Pull request overview

This PR tightens normative requirements across several specification documents, primarily by upgrading advisory language to MUST-level requirements and adding explicit failure modes, logging schemas, and validation/automation expectations to reduce ambiguity and spec drift.

Changes:

• Strengthens security-architecture-spec-validation with MUST-level re-validation triggers, a defined failure escalation path, and a proposed CI automation approach.
• Adds a structured log field schema and a glossary term to the Copilot SDK driver spec to standardize event serialization and persistence expectations.
• Tightens the bash command parser spec’s integration default-deny and test-vector canonicalization requirements, plus expanded security misuse guidance.
• Adds MUST-level catalog sync failure handling and upgrades integrity requirements in the AI credits spec.

Show a summary per file

File	Description
specs/security-architecture-spec-validation.md	Converts re-validation guidance into normative triggers/escalation and proposes CI assertions + a compliance gap audit.
docs/src/content/docs/specs/copilot-sdk-driver-specification.md	Defines structured event log schemas and adds a glossary definition related to event persistence.
docs/src/content/docs/specs/bash-command-parser-specification.md	Adds/strengthens default-deny requirements, pins a canonical test-vector path, and expands security misuse requirements.
docs/src/content/docs/specs/ai-credits-specification.md	Specifies MUST-level catalog sync failure handling and strengthens integrity/provenance controls.

Review details

Tip

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

• Files reviewed: 4/4 changed files
• Comments generated: 8
• Review effort level: Low

[github-actions]

Hey @copilot-swe-agent 👋 — great normative tightening pass across four specs! The section-by-section breakdown in the PR body is clear and well-motivated: the SHOULD→MUST upgrades in ai-credits and bash-command-parser, the new lifecycle-log field tables in copilot-sdk-driver, and the failure-escalation path added to security-architecture-spec-validation are all coherent, focused improvements.

One thing worth addressing before merge:

• No test or validation artifacts — the diff is purely markdown, so there are no explicit coverage changes. The bash-command-parser update adds a pinned canonical test vector path (specs/test-vectors/bash-command-parser/<version>-<source>.json) and three MUST NOT misuse cases in new §11.2, but no corresponding test vector file is included. If the project has a spec linter or structural validation hook (e.g., the gh aw compile linter mentioned in the security-architecture-spec-validation changes), it would be worth confirming those checks pass and adding a test vector for the new §11.2 requirements.

If you'd like a hand, here's a ready-to-use prompt:

For PR #42530 in github/gh-aw (SPDD 2026-06-30 normative tightening), add validation artifacts for the new requirements:

1. Create a test vector file at specs/test-vectors/bash-command-parser/ that exercises the three MUST NOT misuse cases added in §11.2 of bash-command-parser-specification.md:
   - sandbox-proof misuse (crafted input that bypasses sandbox)
   - empty-result allow (consumer treats empty/nil parse output as permit)
   - single-command bypass (single-token input used to sidestep parser)

2. If a spec linter or structural assertion exists (e.g., via `gh aw compile`), confirm it validates the updated normative language in the four changed spec files and add any necessary linter assertions for the new MUST-level clauses.

Generated by ✅ Contribution Check · 116.2 AIC · ⌖ 15.5 AIC · ⊞ 6.3K · ◷

[github-actions]

Review: SPDD 2026-06-30 — Tighten normative requirements across 5 specs

The direction of this PR is good — upgrading advisory SHOULD requirements to binding MUST and adding missing failure modes, misuse examples, and structured log schemas all improve specification quality. However, several of the new MUST-level requirements contain correctness problems that should be addressed before merge.

Blocking issues

#	File	Issue
1	ai-credits-specification.md L237	§5.4.1 missing first-run behavior — no prior catalog edge case undefined
2	ai-credits-specification.md L243	§5.4.4 "normal operation" undefined in MUST NOT requirement
3	ai-credits-specification.md L513	"authorized upstream" undefined — normative MUST on undefined term
4	bash-command-parser-specification.md L282	Test vector canonical path does not exist — MUST references non-existent location
5	copilot-sdk-driver-specification.md L468	Glossary MUST is logically circular
6	security-architecture-spec-validation.md L495	Double negative flips the intended meaning of a normative MUST statement

Non-blocking suggestions

#	File	Issue
7	ai-credits-specification.md L515	Subjective process requirement (reviewer qualification) belongs in process doc, not technical spec
8	bash-command-parser-specification.md L76	length === 0 (JS syntax) in a language-neutral normative requirement
9	copilot-sdk-driver-specification.md L345	"MUST when available" is non-standard RFC 2119
10	security-architecture-spec-validation.md L518	SHOULD for CI adoption / MUST for assertions within it — inconsistent normative levels

What is well done

• §5.4 sync failure handling closes a real gap with concrete, testable rules.
• §11 upgrade from SHOULD to MUST for deny-on-ambiguous-input is correct; the three misuse examples are clear and useful.
• §6.5 field tables are complete and the lifecycle vs. policy-denial log entry distinction is appropriate.
• The §12 gap analysis in security-arch-validation is honest and useful — it clearly flags what is and is not evidenced.
• The Failure Escalation path is a well-structured governance mechanism.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 66.8 AIC · ⌖ 10.6 AIC · ⊞ 4.9K

[github-actions]

Missing first-run edge case in MUST requirement.

The requirement says "the implementation MUST fall back to the last successfully synchronized catalog version", but does not define behavior when no prior version exists (e.g., on first run or after a clean installation). If there is no prior synchronized catalog, this MUST requirement cannot be satisfied and the implementation has no normative guidance on how to proceed safely.

Suggest adding a clause such as: "If no prior synchronized catalog exists, the implementation MUST fail closed and MUST NOT operate until a valid catalog can be obtained."

@copilot please address this.

[github-actions]

"Normal operation" is undefined in a normative MUST requirement.

"The implementation MUST NOT permit normal operation until consistency is restored" — the term "normal operation" is not defined in this specification. Implementations cannot conformantly satisfy a MUST NOT that references an undefined concept.

This needs to specify what is prohibited — e.g., MUST NOT compute or report AIC costs, MUST NOT process new billing events, MUST surface an error to callers — so the constraint is unambiguous and testable.

@copilot please address this.

[github-actions]

"Authorized upstream" is undefined — normative MUST references an undefined term.

"The source origin MUST be confirmed as an authorized upstream before the catalog is consumed" — the term "authorized upstream" is not defined anywhere in this specification. A normative MUST requirement that relies on an undefined term creates an unverifiable conformance requirement.

Define "authorized upstream" in the Glossary (or inline here), specifying at minimum: how authorization is established, where the list of authorized upstreams is maintained, and what mechanism (e.g., URL allowlist, signing key, CODEOWNERS entry) is used to confirm it.

@copilot please address this.

[github-actions]

Subjective process requirement embedded in a normative technical spec.

"Reviews MUST be performed by at least one team member with knowledge of the active model pricing" — "knowledge of the active model pricing" is a subjective, unverifiable qualification. This introduces a MUST requirement that cannot be tested for conformance in any automated or objective way.

Process and qualification requirements of this kind belong in a CONTRIBUTING.md or code-review process document, not in a normative technical specification. If this must remain in the spec, the qualification criterion needs to be made objective and verifiable (e.g., referencing a specific role or CODEOWNERS group).

@copilot please address this.

[github-actions]

JavaScript-specific syntax length === 0 used in a normative requirement within a language-neutral specification.

This spec is explicitly "Language-Neutral" (see §2.3). Using length === 0 — JavaScript strict-equality notation — in a normative MUST requirement creates inconsistency: developers implementing in Go, Python, Rust, or other languages encounter JS-specific syntax in a binding requirement.

Use language-neutral phrasing instead, e.g.: the returned collection is empty (length 0) or reference the abstract type contract in §2.3 directly.

@copilot please address this.

[github-actions]

Normative MUST references a path that does not exist in the repository.

"Reference vectors MUST be stored at specs/test-vectors/bash-command-parser/" — this directory does not currently exist in the repository root. A MUST-level conformance requirement that mandates storage at a non-existent location is immediately violated and cannot be satisfied until the directory and its content are created.

Either:

1. Create the specs/test-vectors/bash-command-parser/ directory (with at least a placeholder or initial vectors) as part of this PR, or
2. Downgrade to SHOULD until the canonical path is established and the first test vectors are committed.

@copilot please address this.

[github-actions]

"MUST when available" is not a standard RFC 2119 keyword and creates conformance ambiguity.

The Required column says "MUST when available" for sessionId. RFC 2119 defines MUST as an absolute requirement. Conditionalizing a MUST with "when available" converts it into something closer to SHOULD, but without the clear semantics of either keyword.

The description already captures the conditional correctly: "MUST be omitted before session is established." Suggest replacing the Required column value with MUST (if session established) and stating clearly in the description: "MUST be present once a session identifier has been assigned; MUST be omitted before session establishment."

This makes the requirement unambiguously testable in both the pre- and post-session states.

@copilot please address this.

[github-actions]

Glossary MUST is logically circular — the condition negates itself.

The definition states: "A conforming implementation MUST restrict event persistence to non-ephemeral events only when those events do not contain sensitive transient state."

This is logically contradictory: non-ephemeral events are already defined in the same sentence as events persisted to durable storage, which is distinct from sensitive transient state. The clause "only when those events do not contain sensitive transient state" therefore applies a condition to a set (non-ephemeral events) that by definition already excludes sensitive transient state.

The intended constraint appears to be two separate requirements:

1. MUST restrict persistence to non-ephemeral events (do not persist ephemeral events).
2. MUST NOT write sensitive transient state (e.g., raw permission-request payloads, connection token values) to durable storage.

Split the single MUST into these two independent requirements for clarity.

@copilot please address this.

[github-actions]

Grammatically incorrect normative statement — creates opposite meaning.

"No new security-impacting features that depend on the unverified claim MUST be merged" — the double negative ("No ... MUST be merged") is grammatically incorrect and produces the opposite of the intended meaning. As written, it could be parsed as: features MUST be merged, which contradicts the intent.

The intended normative statement is: "Security-impacting features that depend on the unverified claim MUST NOT be merged until the claim is re-verified or the specification is amended to match the implementation."

This is a correctness issue in a normative MUST statement and must be fixed before merge.

@copilot please address this.

[github-actions]

SHOULD for the CI approach but MUST for assertions within it — internal normative inconsistency.

Line 506 uses "Assertions MUST include" (absolute requirement) for the contents of the CI job, while line 518 says "A conforming CI pipeline SHOULD implement at least Approach 1" (optional). This creates an internal inconsistency: if adoption of Approach 1 is optional (SHOULD), then the MUST constraints within it are unreachable for pipelines that choose not to adopt it.

Options:

1. Upgrade the overall adoption requirement to MUST if the assertions are truly mandatory.
2. Downgrade the assertion list to SHOULD to match the optional adoption stance.
3. Add a conditional: "When Approach 1 is adopted, the CI job MUST include the following assertions:" — making the inner MUSTs conditional on the outer SHOULD.

@copilot please address this.

[github-actions]

REQUEST_CHANGES — 2 critical spec defects and 8 high/medium issues across all 4 files

Two bugs must be fixed before merge; the others are significant normative weaknesses that will create compliance drift if left unaddressed.

### Blocking issues

1. Normative inversion (security-architecture-spec-validation.md line 495): "No new security-impacting features ... MUST be merged" is an obligation to merge, not a prohibition. Must be rewritten as MUST NOT be merged.

2. Dead CI trigger glob (security-architecture-spec-validation.md line 518): .github/workflows/*.md will never match any GitHub Actions file — they use .yml/.yaml. The proposed security assertion suite would never fire on the files it is meant to guard.

### High-severity normative weaknesses

• copilot-sdk-driver-specification.md line 345: "MUST when available" is not a valid RFC 2119 term; it collapses to OPTIONAL.
• copilot-sdk-driver-specification.md line 468: Glossary non-ephemeral events sentence is logically inverted — a careful reading allows persisting sensitive data in "ephemeral" events.
• ai-credits-specification.md line 237: No bootstrap behavior specified — the fallback to the "last successfully synchronized catalog version" is undefined on first run.
• ai-credits-specification.md line 243: "normal operation" is undefined; hardcoded implementation paths will silently invalidate when renamed.
• ai-credits-specification.md lines 513–515: Both "authorized upstream" and "knowledge of active model pricing" are MUST requirements with no testable conformance criterion.

### Medium-severity issues

• bash-command-parser-specification.md line 76: length === 0 is JavaScript-specific syntax in a normative clause of a language-neutral spec.
• copilot-sdk-driver-specification.md line 346: "completion events" is used in two MUST requirements but never defined.

🔎 Code quality review by PR Code Quality Reviewer · 76.9 AIC · ⌖ 9.55 AIC · ⊞ 1.6K
_{Comment /review to run again}

[github-actions]

Normative inversion makes this requirement the opposite of its intent: "No new security-impacting features ... MUST be merged" is a positive obligation — it tells maintainers they must merge, not that they must not.

💡 Suggested fix

Change from:

4. No new security-impacting features that depend on the unverified claim MUST be merged until the claim is re-verified...

To:

4. New security-impacting features that depend on the unverified claim MUST NOT be merged until the claim is re-verified...

As written, a compliant maintainer reading this normatively is required to merge dependent features — the exact opposite of the safety gate this step is meant to enforce.

[github-actions]

The .github/workflows/*.md glob will never match any GitHub Actions file: Actions workflow files use .yml or .yaml extensions, not .md. This trigger condition is silently inert — PRs modifying compiled workflow YAML will not trigger the proposed security assertion suite.

💡 Suggested fix

A conforming CI pipeline SHOULD implement at least Approach 1 and SHOULD run the assertion suite on every pull request that modifies `.github/workflows/*.yml` files, `.github/workflows/*.yaml` files, or `specs/security-architecture-spec.md`.

The source workflow authoring files are Markdown under .github/workflows/, but the compiled outputs consumed by GitHub Actions (the ones that contain job structures, permission sets, and SHA-pinned actions the CI assertions would verify) are the generated .lock.yml files. The trigger should target those compiled YAML files, not their Markdown sources.

[github-actions]

"MUST when available" is not a valid RFC 2119 term and strips the normative force from this field: RFC 2119 defines MUST as unconditional. Adding "when available" makes this effectively optional — implementors can always claim the session isn't yet "available" — which defeats the schema contract this table is meant to establish.

💡 Suggested fix

Split into two clearly-scoped rows, or use unambiguous RFC 2119 language:

Field	Type	Required	Description
sessionId	string	MUST (if session is established)	Session identifier assigned at session creation; MUST be omitted from log entries emitted before the session identifier is assigned

Alternatively, define a separate schema for pre-session log entries rather than embedding a conditional MUST in a shared table.

[github-actions]

The glossary definition of non-ephemeral events contains a logical inversion that reverses the intended constraint: "MUST restrict event persistence to non-ephemeral events only when those events do not contain sensitive transient state" reads as: you may persist non-ephemeral events only if they aren't sensitive — but says nothing about ephemeral events containing sensitive state.

💡 Suggested fix

The intended constraint appears to be: do not write sensitive transient data to durable storage. Rewrite as:

A conforming implementation MUST NOT persist events containing sensitive transient state (such as raw permission-request payloads or connection token values) to durable storage, regardless of whether those events are otherwise considered non-ephemeral.

The current formulation creates an exploitable ambiguity: an implementor could persist sensitive data in a log entry classified as "ephemeral" and argue conformance, since the MUST only restricts non-ephemeral events.

[github-actions]

§5.4 item 1 is silent on the first-run case where no prior sync exists: The fallback to "the last successfully synchronized catalog version" is undefined when no successful sync has ever occurred (fresh install, clean CI environment, new deployment). The constraint "MUST NOT silently use an empty or zero-cost catalog" is then unresolvable — there's no prior version to fall back to.

💡 Suggested fix

Add a normative clause for the bootstrap case, for example:

If no previously synchronized catalog exists, the implementation MUST either use a bundled baseline catalog included at build time or MUST refuse to process AIC computations and emit a diagnostic indicating that catalog initialization is required. The implementation MUST NOT default to a zero-cost or empty catalog in this state.

Without this, every implementation independently invents its own bootstrap behavior, which is exactly the kind of undefined-corner that leads to divergent security properties.

[github-actions]

Two problems on this line: (1) "normal operation" is undefined, making the MUST NOT unenforceable; (2) the hardcoded paths pkg/cli/data/models.json and actions/setup/js/models.json in a normative requirement will silently invalidate this clause if either path is restructured.

💡 Suggested fix

For (1): define what "normal operation" means when it is blocked — does the implementation exit non-zero, refuse incoming requests, enter degraded mode, or something else? For example:

...the implementation MUST treat this as a sync failure: it MUST NOT compute or return AIC values and MUST emit a diagnostic identifying the divergence, until catalog consistency is restored.

For (2): replace hardcoded paths with a symbolic reference defined elsewhere in the spec (e.g., a §2 Definitions entry: "authoritative catalog paths"), so that a path rename requires only a single definition update rather than a spec edit at every mention.

[github-actions]

"authorized upstream" is undefined, making this MUST requirement untestable: The requirement to confirm "the source origin MUST be confirmed as an authorized upstream" has no normative definition of what constitutes an authorized upstream or what "confirmed" means (GPG signature? CODEOWNERS membership? an allowlist?). A MUST with no conformance criterion is indistinguishable from a SHOULD.

💡 Suggested fix

Define the mechanism. For example:

...the source origin MUST be verified to match an entry in the project's authorized catalog sources list (defined in CATALOG_SOURCES.md or equivalent configuration). Verification MUST be performed via one of: (a) a GPG signature from a known key, (b) a hash comparison against a pinned expected digest, or (c) a source URL allowlist enforced by the CI pipeline.

Without a mechanism, this MUST cannot be verified by an auditor, and cannot be implemented consistently across different teams consuming this spec.

[github-actions]

Elevating to MUST a review requirement defined only by subjective expertise makes it unverifiable and unenforceably broad: "reviews MUST be performed by at least one team member with knowledge of the active model pricing" has no testable conformance criterion. Any team member could assert they have "knowledge" — or none could, and no process enforces it.

💡 Suggested fix

Either define a role with formal qualification (e.g., "a designated billing-catalog maintainer listed in CODEOWNERS"), or downgrade to SHOULD with a rationale that this is a process recommendation rather than a verifiable constraint:

Reviews SHOULD be performed by at least one team member designated as a billing-catalog maintainer (listed in .github/CODEOWNERS for the catalog paths). Where no designated reviewer is available, the review MUST be documented as an exception in the PR.

A MUST obligation that cannot be objectively verified is normative debt — it degrades trust in all MUST requirements in the spec.

[github-actions]

length === 0 embeds JavaScript-specific syntax in a language-neutral normative requirement: The spec header declares language-neutrality, but length === 0 is a JavaScript expression (strict equality operator, .length property). Readers implementing in Go, Python, or Rust encounter a term with no direct equivalent.

💡 Suggested fix

Use language-neutral prose:\n\n> A Class I (Integration Consumer) conforming implementation MUST apply default-deny behavior when the parser returns an empty result set (a list or collection containing no command identifiers) or a nil/null command identifier. An empty or nil result MUST NOT be treated as implicit authorization to proceed with command execution.

The parenthetical (§4.3, length === 0) cross-reference can be retained in a non-normative note if needed to correlate with a JS-specific test vector, but must not be part of the normative sentence.

[github-actions]

"completion events" is used in two MUST requirements but is never defined: Both durationMs (MUST for completion events) and hasOutput (MUST for completion events) depend on the term, but §6.5 has no definition of what constitutes a completion event. Implementors independently deciding what qualifies will produce inconsistent schemas.

💡 Suggested fix

Add a definition, either inline or in a preceding note — e.g.:\n\n> Completion event: A lifecycle log entry with event value "session.completed" or "error" (as enumerated in the event field description above).

Without this, the MUST obligation for durationMs and hasOutput cannot be consistently validated and test vectors cannot be written for this schema.

[pelikhan]

@copilot run pr-finisher skill

[Copilot]

@copilot run pr-finisher skill

Done. I ran the PR-finisher flow and pushed commit d1c3e57 addressing the outstanding spec review feedback, then revalidated locally (make fmt, make lint, make test-unit, make test, make recompile, make agent-report-progress) and ran parallel validation.

done

[github-actions]

🤖 PR Triage

Field	Value
Category	docs
Risk	🟢 Low
Score	18 / 100
Action	⏸ defer

Score breakdown: Impact 12/50 · Urgency 3/30 · Quality 3/20

Rationale: SPDD daily spec-tightening work. Blocked by changes_requested (github-actions). Address review feedback on normative language changes.

Generated by 🔧 PR Triage Agent · 83.9 AIC · ⌖ 17.1 AIC · ⊞ 1.6K · ◷

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.82.1