Skip to content

feat(agents): align sssc planner with rai parity, add signing and validation#1497

Open
WilliamBerryiii wants to merge 12 commits intomainfrom
feat/sssc-planner-rai-parity
Open

feat(agents): align sssc planner with rai parity, add signing and validation#1497
WilliamBerryiii wants to merge 12 commits intomainfrom
feat/sssc-planner-rai-parity

Conversation

@WilliamBerryiii
Copy link
Copy Markdown
Member

Description

Brings the SSSC Planner to parity with the RAI Planner across consent gating, footer-with-review enforcement, artifact signing, state persistence, and entry-mode prompt structure.

Highlights:

  • Disclaimer gatesssc-planner now offers a first-turn disclaimer (sssc-full-disclaimer) and persists disclaimerShownAt in state.json.
  • State schema (NEW)scripts/linting/schemas/sssc-state.schema.json (JSON Schema draft 2020-12) formalizes 16 required fields, entryMode enum [capture, from-prd, from-brd, from-security-plan], currentPhase 1–6, and userPreferences (autonomy tier, output detail level, target system, audience profile, optional artifacts).
  • State field expansion17 → 20 fields. Added signingRequested, signingManifestPath, disclaimerShownAt. Mirrored in docs/agents/agent-overview.md.
  • Signing extensionscripts/security/Sign-PlannerArtifacts.ps1 gains a BySessionPath parameter set with -SessionPath and -ManifestName, while preserving the existing -ProjectSlug behavior. Manifest output now includes sessionPath.
  • Footer-with-review classification — Renamed human-facing-with-disclaimerrai-handoff-with-disclaimer; added sssc-handoff-with-disclaimer scoped to .github/instructions/security/sssc-*.instructions.md.
  • Prompt restructure — Four entry-mode prompts (capture, from-prd, from-brd, from-security-plan) restructured with consistent Startup → Pre-Scan → Output Preferences → Scope Extraction → Initialization → Phase 1 Entry. From-* modes fall back to capture when zero artifacts are discovered. Switched bare {project-slug}${input:project-slug}.
  • TestsValidate-PlannerArtifacts.Tests.ps1 and Sign-PlannerArtifacts.Tests.ps1 updated for the rename, the new SSSC tier, and the new sessionPath manifest field.
  • Collections / plugins / docs — Synchronized SSSC prompt descriptions across hve-core-all, project-planning, and security collection manifests; regenerated plugin READMEs.

Related Issue(s)

N/A

Type of Change

Code & Documentation:

  • New feature (non-breaking change adding functionality)
  • Documentation update

Infrastructure & Configuration:

  • Linting configuration (markdown, PowerShell, etc.)

AI Artifacts:

  • Reviewed contribution with prompt-builder agent and addressed all feedback
  • Copilot instructions (.github/instructions/*.instructions.md)
  • Copilot prompt (.github/prompts/*.prompt.md)
  • Copilot agent (.github/agents/*.agent.md)

Other:

  • Script/automation (.ps1, .sh, .py)

Sample Prompts (for AI Artifact Contributions)

User Request:

"Plan supply chain security for my repo, capture mode."

Or, with an existing artifact:

"Use this PRD to start an SSSC plan."

Execution Flow:

  1. User invokes @sssc-planner (or one of the entry-mode prompts: sssc-capture, sssc-from-prd, sssc-from-brd, sssc-from-security-plan).
  2. Startup — Agent presents the full SSSC disclaimer (first turn only), captures consent, and stamps disclaimerShownAt. Standards announcement (OpenSSF Scorecard, SLSA v1.0, OpenSSF Best Practices Badge, Sigstore, NTIA SBOM) is shown.
  3. Pre-Scan — Agent searches .copilot-tracking/ and docs/ for the relevant entry artifact (PRD, BRD, or Security Plan). from-* modes fall back to capture when zero artifacts are found.
  4. Output Preferences — Agent confirms backlog target (ADO / GitHub / both / custom).
  5. Scope Extraction — Tech stack, package managers, CI platform, release strategy, compliance targets.
  6. Initializationstate.json is written, conforming to sssc-state.schema.json, preserving disclaimerShownAt.
  7. Phase 1 Entry — Eight question topics with ❓ / ✅ / ❌ markers begin.
  8. At handoff, user may invoke Sign-PlannerArtifacts.ps1 -SessionPath .copilot-tracking/sssc-plans/<slug> to sign artifacts via cosign; manifest path is recorded back to state.json.

Output Artifacts:

.copilot-tracking/sssc-plans/<project-slug>/
├── state.json                  # validates against sssc-state.schema.json
├── phase-1-scoping.md
├── phase-2-assessment.md
├── phase-3-standards.md
├── phase-4-gap-analysis.md
├── phase-5-backlog.md
├── phase-6-handoff.md
└── artifact-manifest.json      # produced by Sign-PlannerArtifacts.ps1

state.json excerpt:

{
  "$schema": "../../../scripts/linting/schemas/sssc-state.schema.json",
  "projectSlug": "my-project",
  "entryMode": "from-prd",
  "currentPhase": 1,
  "disclaimerShownAt": "2025-01-15T10:30:00Z",
  "signingRequested": false,
  "signingManifestPath": null,
  "userPreferences": { "targetSystem": "github", "autonomyTier": "guided" }
}

Success Indicators:

  • state.json validates against scripts/linting/schemas/sssc-state.schema.json.
  • disclaimerShownAt is present and stable across re-entry.
  • Footer-with-review enforcement applies the sssc-handoff-with-disclaimer tier to all .github/instructions/security/sssc-*.instructions.md outputs.
  • npm run test:ps -- -TestPath scripts/tests/linting/Validate-PlannerArtifacts.Tests.ps1 passes the new SSSC cases.
  • npm run test:ps -- -TestPath scripts/tests/security/Sign-PlannerArtifacts.Tests.ps1 passes with the new sessionPath manifest field.

Testing

  • npm run lint:md — pass
  • npm run spell-check — pass (new SSSC vocabulary added to .cspell.json)
  • npm run lint:frontmatter — pass
  • npm run validate:skills — pass (no skill changes)
  • npm run lint:md-links — pass
  • npm run lint:ps — pass
  • npm run plugin:generate — regenerated; no drift
  • npm run test:ps -- -TestPath scripts/tests/linting/Validate-PlannerArtifacts.Tests.ps1 — pass
  • npm run test:ps -- -TestPath scripts/tests/security/Sign-PlannerArtifacts.Tests.ps1 — pass

Checklist

Required Checks

  • Documentation is updated (agent-overview.md, plugin READMEs, collection markdown)
  • Files follow existing naming conventions
  • Changes are backwards compatible (signing param sets are additive; footer rename is internal)
  • Tests added for new functionality (SSSC disclaimer + tier; sessionPath field)

AI Artifact Contributions

  • Used /prompt-analyze to review contribution
  • Addressed all feedback from prompt-builder review
  • Verified contribution follows common standards and type-specific requirements

Required Automated Checks

  • Markdown linting: npm run lint:md
  • Spell checking: npm run spell-check
  • Frontmatter validation: npm run lint:frontmatter
  • Skill structure validation: npm run validate:skills
  • Link validation: npm run lint:md-links
  • PowerShell analysis: npm run lint:ps
  • Plugin freshness: npm run plugin:generate
  • Docusaurus tests: npm run docs:test

Security Considerations

  • This PR does not contain any sensitive or NDA information
  • Any new dependencies have been reviewed for security issues — none added
  • Security-related scripts follow the principle of least privilege — Sign-PlannerArtifacts.ps1 only reads under the supplied session path and writes a single manifest file

Additional Notes

sssc-planner is maturity: experimental in collections/security.collection.yml; no maturity tier change in this PR. The footer-with-review rename (human-facing-with-disclaimerrai-handoff-with-disclaimer) is internal to .github/instructions/shared/footer-with-review.yml and the corresponding Pester fixture; no published artifact referenced the old classification key.

…idation

Bring the SSSC Planner to feature parity with the RAI Planner across identity,
disclaimers, footers, phase prompts, handoff signing, validation, and docs.

Changes by RAI #1287 category:

1. Identity and state — Update sssc-identity.instructions.md to add
   signingRequested, signingManifestPath, and disclaimer acknowledgment fields
   in the state schema; add a JSON schema (sssc-state.schema.json) for
   validation; align session recovery and orchestration language with RAI.

2. Disclaimer infrastructure — Register sssc-full-disclaimer in
   .github/config/disclaimers.yml so the SSSC handoff renders the same
   professional-review notice tier RAI uses.

3. Footer tier — Add sssc-handoff-with-disclaimer to
   .github/config/footer-with-review.yml (Tier 1 + checkbox + Tier 2
   disclaimer, scoped to .github/instructions/security/sssc-*); rename the
   companion RAI tier human-facing-with-disclaimer to
   rai-handoff-with-disclaimer for naming symmetry.

4. Phase instructions and prompts — Refresh sssc-{assessment,gap-analysis,
   standards,backlog,handoff}.instructions.md and sssc-{capture,from-brd,
   from-prd,from-security-plan}.prompt.md for the parity flow, signing
   prompts, and disclaimer wiring.

5. Handoff signing — Update sssc-handoff.instructions.md Phase 6 to invoke
   pwsh scripts/security/Sign-PlannerArtifacts.ps1 with the SSSC manifest and
   to record signingRequested / signingManifestPath in state.

6. Signing script and tests — Add scripts/security/Sign-PlannerArtifacts.ps1
   (planner-agnostic cosign wrapper) plus
   scripts/tests/security/Sign-PlannerArtifacts.Tests.ps1.

7. Validation — Extend scripts/tests/linting/Validate-PlannerArtifacts.Tests.ps1
   to cover the new SSSC tier, the renamed RAI tier, and the JSON schema.

8. Documentation and generated outputs — Update sssc-planner.agent.md, the
   docs/agents/sssc-planning overview, collection markdown for hve-core-all,
   project-planning, and security, regenerate the matching plugins/ READMEs,
   and add SSSC terms to .cspell.json.

Validation: targeted Pester suite Validate-PlannerArtifacts.Tests.ps1 = 31/31
PASS; lint:yaml, lint:md, lint:ps, lint:frontmatter, lint:collections-metadata,
lint:marketplace, lint:version-consistency, lint:permissions,
lint:dependency-pinning, lint:py, spell-check, plugin:validate all PASS.

🤖 Crafted with precision by ✨Copilot following brilliant human instruction,
then carefully refined by our team of discerning human reviewers.
…ction descriptions

🔧 - Generated by Copilot
@WilliamBerryiii WilliamBerryiii requested a review from a team as a code owner May 1, 2026 01:13
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 1, 2026

Codecov Report

❌ Patch coverage is 27.27273% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.16%. Comparing base (5bedf80) to head (c99e5ba).

Files with missing lines Patch % Lines
scripts/linting/Format-MarkdownTables.ps1 0.00% 26 Missing ⚠️
scripts/security/Sign-PlannerArtifacts.ps1 66.66% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1497      +/-   ##
==========================================
- Coverage   87.41%   87.16%   -0.25%     
==========================================
  Files          68       69       +1     
  Lines       10302    10341      +39     
==========================================
+ Hits         9005     9014       +9     
- Misses       1297     1327      +30     
Flag Coverage Δ
pester 84.41% <27.27%> (-0.34%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
scripts/security/Sign-PlannerArtifacts.ps1 86.30% <66.66%> (-7.04%) ⬇️
scripts/linting/Format-MarkdownTables.ps1 0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Advisory review, this PR is from a maintainer. Findings are informational only.


Review Summary

This PR delivers a well-structured parity update that brings the SSSC Planner in line with the RAI Planner's architecture: consistent disclaimer tier handling, the new BySessionPath cosign-signing flow, a formal JSON Schema for state.json, and full prompt rewrites for all five entry modes. The overall design and implementation quality is high.

Six informational findings are noted below; the two schema-alignment issues are the most worth addressing before merge since they could cause runtime validation failures.


Issue Alignment

The PR description states "N/A" for Related Issue(s). No GitHub issue is linked. While acceptable for maintainer-driven changes, linking an issue (even a tracking issue) is recommended so change intent is traceable over time.


PR Template Compliance

  • ✅ Description is filled in with clear intent and file-level changes.
  • ✅ Type of Change checkboxes are checked and match the actual diff (Copilot agent, prompt, instructions, script, schema).
  • ⚠️ Related Issue(s) is "N/A" — no linked issue.
  • ✅ Testing section references npm run test:ps.
  • ✅ Checklist items are checked.

Coding Standards

PowerShell (Sign-PlannerArtifacts.ps1)

The new BySessionPath parameter set follows existing patterns correctly: copyright header, #Requires, CmdletBinding, [OutputType()], $ErrorActionPreference = 'Stop', and comment-based help are all in order. One path-separator edge case is noted in the inline comment at line 183.

Prompt Files (sssc-capture.prompt.md, sssc-from-prd.prompt.md, sssc-from-brd.prompt.md, sssc-from-security-plan.prompt.md)

All four entry-mode prompts reference outputPreferences as a state.json key. This field does not exist in the new sssc-state.schema.json, which uses "additionalProperties": false. The correct path is userPreferences.targetSystem. See inline comments on sssc-capture.prompt.md:43 and sssc-from-prd.prompt.md:52; the same correction applies to the BRD and security-plan variants.

Instructions Files

  • sssc-handoff.instructions.md:105 references userPreferences.signingRequested, but signingRequested is a top-level field in the schema, not nested under userPreferences. See inline comment.
  • sssc-identity.instructions.md:120 shows a userPreferences template with only autonomyTier; the schema now requires five fields. See inline comment.

Code Quality

  • sssc-state.schema.json: Well-formed JSON Schema draft 2020-12. The 16 required fields, entryMode enum, and userPreferences sub-schema with five required fields are consistent with the agent file's sample state. disclaimerShownAt and signingManifestPath are correctly omitted from required (nullable). No issues found.
  • Test files: Both Validate-PlannerArtifacts.Tests.ps1 and Sign-PlannerArtifacts.Tests.ps1 follow Pester 5 conventions. The rename from human-facing-with-disclaimer to rai-handoff-with-disclaimer and the new sssc-handoff-with-disclaimer tier are covered by the updated tests.
  • Footer config files: The tiered footer design (sssc-*.instructions.md scoped to both agentic and human-facing) mirrors the established RAI pattern. The configuration looks intentional and correct.

Action Items

# File Line Severity Summary
1 sssc-capture.prompt.md 43 Medium outputPreferences key doesn't exist in schema; use userPreferences.targetSystem
2 sssc-from-prd.prompt.md 52 Medium Same outputPreferences schema mismatch
3 sssc-from-brd.prompt.md ~52 Medium Same outputPreferences schema mismatch (not separately commented)
4 sssc-from-security-plan.prompt.md ~52 Medium Same outputPreferences schema mismatch (not separately commented)
5 sssc-handoff.instructions.md 105 Medium userPreferences.signingRequested → top-level signingRequested
6 sssc-identity.instructions.md 120 Low Stale userPreferences example missing 4 required fields
7 sssc-capture.prompt.md 55 Low "up to 5 questions" but 8 topics listed — ambiguous grouping intent
8 Sign-PlannerArtifacts.ps1 183 Low Windows path-separator edge case in StartsWith comparison

Generated by PR Review for issue #1497 · ● 2.4M

Comment thread .github/prompts/security/sssc-capture.prompt.md Outdated
Comment thread .github/prompts/security/sssc-capture.prompt.md
Comment thread .github/prompts/security/sssc-from-prd.prompt.md Outdated
Comment thread .github/instructions/security/sssc-handoff.instructions.md Outdated
Comment thread .github/instructions/security/sssc-identity.instructions.md Outdated
Comment thread scripts/security/Sign-PlannerArtifacts.ps1 Outdated
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Advisory review, this PR is from a maintainer. Findings are informational only.

Review Overview

This PR delivers a solid parity pass for the SSSC planner — the new JSON schema, signing script improvements, and four-prompt alignment are all well-structured additions. The inline comments flag a schema/documentation drift that would cause runtime validation failures if left unaddressed, and a gap in test coverage for the new BySessionPath parameter set.


Issue Alignment

No issue is linked (Related Issue(s): N/A). Not blocking in advisory mode, but linking an issue would help trace the RAI parity motivation for this work.


PR Template Compliance

✅ Description is thorough and accurate.
✅ Type of Change checkboxes correctly reflect the diff (instructions, prompts, agent, script/automation).
✅ Testing section is comprehensive.
✅ Required Automated Checks all marked.


Coding Standards

No violations found against the matched instruction files (powershell.instructions.md, pester.instructions.md, prompt-builder.instructions.md). Script structure, copyright headers, CmdletBinding, ErrorActionPreference, and comment-based help all comply.


Code Quality Findings

⚠️ HIGH — Schema/doc drift: wrong includeOptionalArtifacts field names in agent example

.github/agents/security/sssc-planner.agent.md lines 144–146 document adoptionPlaybook and executiveSummary inside includeOptionalArtifacts. The new sssc-state.schema.json defines this object with additionalProperties: false and requires sbom, scorecardProjection, and artifactSigning. An agent following the embedded example would produce a state.json that fails schema validation. See inline comment on line 146.

⚠️ HIGH — outputPreferences not a valid root state.json key (all four entry prompts)

All four entry-mode prompts instruct the agent to write outputPreferences to state.json. The schema has additionalProperties: false at the root with no outputPreferences property. Every state write following these prompts would fail validation. The correct target is userPreferences.targetSystem. See inline comments on each prompt file.

💡 MEDIUM — disclaimerShownAt and signingManifestPath absent from required

Both fields are always initialised at state creation but are not listed in required. A state file missing these keys still passes schema validation, which could silently surface undefined to agent logic that expects at least null. See inline comment on sssc-state.schema.json line 24.

💡 MEDIUM — BySessionPath parameter set has no test coverage

Sign-PlannerArtifacts.Tests.ps1 was updated to expect sessionPath in the manifest but no tests exercise the new BySessionPath invocation path, its absolute/relative path resolution, or sessionLabel derivation. See inline comment on line 215.


Action Items

  1. Fix includeOptionalArtifacts example in sssc-planner.agent.md to use sbom, scorecardProjection, artifactSigning.
  2. Replace outputPreferences instructions in all four prompts with userPreferences.targetSystem.
  3. Consider adding disclaimerShownAt and signingManifestPath to the schema required array.
  4. Consider adding BySessionPath test scenarios to Sign-PlannerArtifacts.Tests.ps1.

Generated by PR Review for issue #1497 · ● 3.7M

Comment thread .github/prompts/security/sssc-capture.prompt.md Outdated
Comment thread .github/agents/security/sssc-planner.agent.md Outdated
Comment thread .github/prompts/security/sssc-from-prd.prompt.md Outdated
Comment thread .github/prompts/security/sssc-from-brd.prompt.md Outdated
Comment thread .github/prompts/security/sssc-from-security-plan.prompt.md Outdated
Comment thread scripts/linting/schemas/sssc-state.schema.json
Comment thread scripts/tests/security/Sign-PlannerArtifacts.Tests.ps1
…th Pester coverage

- replace npm script with pwsh wrapper at scripts/linting/Format-MarkdownTables.ps1
- add 13 Pester tests covering empty repo, no-git, formatted/unformatted tables, dot-prefixed dirs, verbose mode
- guard PS7 Start-Process flush race with WaitForExit + size-check retry + ReadAllText
- surface stdout/stderr byte counts via Should -Because for diagnosability

🧪 - Generated by Copilot
- regenerate vulnerability and principle indexes across owasp-* and secure-by-design skill references
- reformat tables in CUSTOM-AGENTS, instructions README, pull-request instructions
- reformat skill READMEs (powerpoint corpus, video-to-gif examples, jql-reference, pr-reference REFERENCE)
- reformat workflow README and doc-update-check tables

📐 - Generated by Copilot
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails
actions/actions/setup-node 48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e 🟢 5.4
Details
CheckScoreReason
Code-Review🟢 6Found 16/23 approved changesets -- score normalized to 6
Binary-Artifacts🟢 9binaries present in source code
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Maintained🟢 79 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 7
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Packaging⚠️ -1packaging workflow not detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
Fuzzing⚠️ 0project is not fuzzed
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Security-Policy🟢 9security policy file detected
Branch-Protection⚠️ 1branch protection is not maximal on development and all release branches
SAST🟢 10SAST tool is run on all commits

Scanned Files

  • .github/workflows/pester-tests.yml

@github-actions github-actions Bot mentioned this pull request May 1, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Advisory review, this PR is from a maintainer. Findings are informational only.


Review Summary

This run reviews the two new commits added since the previous automated reviews — the Format-MarkdownTables.ps1 wrapper script and the table-reformatting pass — and confirms the current state of all findings from prior runs.

The two HIGH schema-drift issues flagged in earlier reviews remain unresolved and are the most important items to address before merge, as they will cause state.json files written by the agent to fail schema validation at runtime.


Issue Alignment

Related Issue(s): N/A. No linked issue. Not blocking in advisory mode, but linking a tracking issue would help trace the motivation over time.


PR Template Compliance

✅ Description is thorough and accurate.
✅ Type of Change checkboxes match the diff.
✅ Testing section is comprehensive and all checks are marked.
✅ AI Artifact Contributions checklist is present; prompt-builder review items are left unchecked per author's choice.


Coding Standards

Format-MarkdownTables.ps1 (new file)

The script follows copyright headers, #Requires, CmdletBinding, and $ErrorActionPreference = 'Stop' conventions. Two minor deviations from powershell.instructions.md are noted in the inline comment (line 99): missing main execution guard and missing #region/#endregion markers. Low risk.


Code Quality — Persistent Schema Drift

⚠️ HIGH — outputPreferences key invalid in all four entry-mode prompts

sssc-capture.prompt.md (line 43), sssc-from-prd.prompt.md (line 52), sssc-from-brd.prompt.md (line 52), and sssc-from-security-plan.prompt.md (line 48) all instruct the agent to write outputPreferences into state.json. The schema defines "additionalProperties": false at the root with no such property. Every state write following these prompts produces an invalid document. Correct key: userPreferences.targetSystem.

⚠️ HIGH — includeOptionalArtifacts example in sssc-planner.agent.md uses wrong field names

Lines 144–146 use adoptionPlaybook and executiveSummary. The schema's includeOptionalArtifacts sub-object requires exactly sbom, scorecardProjection, and artifactSigning with "additionalProperties": false. The stale example will cause schema validation failure.

💡 MEDIUM — userPreferences.signingRequested should be top-level signingRequested

sssc-handoff.instructions.md line 105 references a non-existent nested path. signingRequested is a root-level field.

💡 MEDIUM — BySessionPath parameter set has no test coverage

Sign-PlannerArtifacts.Tests.ps1 verifies the sessionPath manifest field appears but no test exercises the BySessionPath code path, absolute/relative path resolution, or sessionLabel derivation.

💡 LOW — userPreferences example in sssc-identity.instructions.md is incomplete

Line 120 shows only autonomyTier; the schema requires all five userPreferences fields.


Action Items

# File Line Severity Status
1 sssc-capture.prompt.md (+ 3 sibling prompts) 43/52/52/48 ⚠️ HIGH Unresolved
2 sssc-planner.agent.md 144–146 ⚠️ HIGH Unresolved
3 sssc-handoff.instructions.md 105 💡 MEDIUM Unresolved
4 Sign-PlannerArtifacts.Tests.ps1 215 💡 MEDIUM Unresolved
5 sssc-identity.instructions.md 120 💡 LOW Unresolved
6 Format-MarkdownTables.ps1 99 💡 LOW New

Generated by PR Review for issue #1497 · ● 3.1M

Comment thread .github/prompts/security/sssc-capture.prompt.md Outdated
Comment thread .github/agents/security/sssc-planner.agent.md Outdated
Comment thread .github/instructions/security/sssc-handoff.instructions.md Outdated
Comment thread scripts/tests/security/Sign-PlannerArtifacts.Tests.ps1
Comment thread scripts/linting/Format-MarkdownTables.ps1
Comment thread .github/instructions/security/sssc-identity.instructions.md Outdated
- Replace removed outputPreferences references with userPreferences.targetSystem to match sssc-state.schema.json

🔒 - Generated by Copilot
…orecardProjection)

- Replace removed adoptionPlaybook/executiveSummary references with the current sbom and scorecardProjection state slots

🔒 - Generated by Copilot
…-state schema

- Move signingRequested under state and expand userPreferences to the five fields defined by sssc-state.schema.json

🔒 - Generated by Copilot
…ce test cleanup catch

- Anchor repo-root boundary check on the OS directory separator to avoid prefix matches across sibling paths

- Replace empty catch in Test-Format-MarkdownTables junction cleanup with Write-Verbose to satisfy PSAvoidUsingEmptyCatchBlock

🔒 - Generated by Copilot
- Add Node 24 setup with npm cache and npm ci so the Pester job has the toolchain expected by the test fixtures

🔒 - Generated by Copilot
…ADMEs

- Bump ms.date to 2026-05-01 to clear freshness check warnings on these long-stable docs

🔒 - Generated by Copilot
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Advisory review, this PR is from a maintainer. Findings are informational only.


Review Summary: feat/sssc-planner-rai-parity

This PR aligns the SSSC Planner with the RAI Planner feature-set parity: disclaimer gating, session signing, structured state schema, and consistent prompt structure. The overall shape of the change is sound and the implementation is detailed. Five advisory observations are noted below.


Issue Alignment

  • No linked issue: The PR correctly states "Related Issue(s): N/A". No substantive misalignment concerns follow from this.

PR Template Compliance

  • AI Artifact review checkboxes unchecked: The PR modifies .agent.md, .prompt.md, and .instructions.md files. The template's AI Artifact Contributions checklist ("Used /prompt-analyze to review contribution" and "Addressed all feedback from prompt-builder review") is unchecked. These are manual attestation items that require human action before merging.

Coding Standards

Format-MarkdownTables.ps1 — Missing invocation guard and region blocks

Per powershell.instructions.md, the main execution block must be wrapped in if ($MyInvocation.InvocationName -ne '.') and organized with #region/#endregion. The script currently puts all execution directly at module scope. See inline comment at line 42.

Test-Format-MarkdownTables.Tests.ps1 — Two observations

  1. throw in BeforeAll (line 11): Throws abort the suite as an error rather than a skip, giving cryptic failures for contributors who haven't run npm install. See inline comment.
  2. -Tag 'Unit' (line 151): The tests launch subprocesses, require node_modules, and mutate the filesystem—integration-test characteristics. The tag should reflect that. See inline comment.

Code Quality

Sign-PlannerArtifacts.Tests.ps1 — Missing coverage for BySessionPath

The new BySessionPath parameter set introduces path-resolution logic, manifest naming, and parameter-set mutual exclusivity—all untested. For a signing script that produces security attestation manifests, coverage gaps here are worth closing. See inline comment at line 215.

disclaimers.yml — Shared applies-to artifact names

Both rai-planner and sssc-planner disclaimers map to handoff-summary and compact-handoff-summary. Disambiguation relies on the validation tool cross-referencing tier scopes from footer-with-review.yml. If the lookup is artifact-name-only, both disclaimers apply simultaneously. Adding an explicit scope field to disclaimer entries would make the intent self-documenting. See inline comment at line 33.


What Looks Good

  • pester-tests.yml correctly pins actions/setup-node to a full commit SHA with a version comment — fully compliant with workflow conventions.
  • Sign-PlannerArtifacts.ps1 session-path computation handles absolute paths, trailing separators, and repo-root edge cases cleanly.
  • sssc-state.schema.json uses additionalProperties: false consistently throughout and correctly models nullable fields (disclaimerShownAt, signingManifestPath) using ["string", "null"].
  • The four SSSC prompt files have been restructured consistently with parallel startup/scope/phase-entry sections matching RAI parity.

Action items for the author:

  1. Check the AI Artifact Contributions checklist items in the PR description.
  2. Consider wrapping main execution in Format-MarkdownTables.ps1 with an invocation guard and region blocks.
  3. Replace throw in Test-Format-MarkdownTables.Tests.ps1 BeforeAll with a skip-friendly guard.
  4. Reconsider -Tag 'Unit' on the Format-MarkdownTables describe block given its integration-test dependencies.
  5. Add BySessionPath parameter set tests to Sign-PlannerArtifacts.Tests.ps1.
  6. Evaluate whether disclaimers.yml entries need a scope field for explicit disambiguation.

Generated by PR Review for issue #1497 · ● 5M

$repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '..' '..')).Path
$emitVerbose = $VerbosePreference -ne 'SilentlyContinue'

Push-Location $repoRoot
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Advisory: Missing invocation guard and region blocks

The PowerShell script conventions require all main execution to be wrapped in an invocation guard and organized with #region/#endregion blocks:

#region Functions
# (no helper functions in this script, but the region still signals intent)
#endregion Functions

#region Main Execution
if ($MyInvocation.InvocationName -ne '.') {
    Push-Location $repoRoot
    try {
        # ... all current execution code ...
    }
    finally {
        Pop-Location
    }
}
#endregion Main Execution

The invocation guard (if ($MyInvocation.InvocationName -ne '.')) ensures the script can be safely dot-sourced by Pester tests. While the current tests use Start-Process rather than dot-sourcing, the guard is required by convention (powershell.instructions.md: "Wrap main execution in an invocation guard that enables dot-sourcing for test files").

Reference: .github/instructions/hve-core/powershell.instructions.md — "Main Execution Guard" section.

$script:RealNodeModules = Join-Path $script:RepoRoot 'node_modules'

if (-not (Test-Path $script:RealNodeModules)) {
throw "Cannot run Format-MarkdownTables tests: node_modules missing at $script:RealNodeModules. Run 'npm install' first."
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Advisory: throw in BeforeAll aborts the suite rather than skipping it

Using throw causes the entire Describe block to be reported as an error rather than a skip. If node_modules is absent (e.g., a fresh checkout without npm ci), CI will surface a cryptic Pester failure rather than a helpful skip.

Consider guarding tests with Set-ItResult -Skipped per context, or wrapping all It blocks in a conditional:

BeforeAll {
    $script:NodeModulesAvailable = Test-Path $script:RealNodeModules
    if (-not $script:NodeModulesAvailable) {
        Write-Warning "node_modules missing at $script:RealNodeModules. Run 'npm install' to enable these tests."
    }
}

# Then at each It block:
It 'Exits 0' {
    if (-not $script:NodeModulesAvailable) { Set-ItResult -Skipped -Because "node_modules missing" }
    $script:Result.ExitCode | Should -Be 0
}

The pester-tests.yml workflow now runs npm ci before Pester, so this shouldn't fire in CI—but it will fire for contributors running tests locally before installing dependencies, producing confusing failures.

}
}

Describe 'Format-MarkdownTables' -Tag 'Unit' {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Advisory: -Tag 'Unit' is misleading for integration-style tests

These tests:

  • Launch a subprocess (Start-Process pwsh) to invoke the SUT
  • Require a real git repository with actual .github/**/*.md files
  • Depend on node_modules being populated
  • Mutate files on disk

This is integration-test behavior. Tagging them -Tag 'Unit' can mislead future contributors about test scope and may complicate future test filtering (e.g., if a fast-unit-only CI job is ever added using -Tag 'Unit').

Consider -Tag 'Integration' or adding a second tag: -Tag 'Unit', 'Integration' with a note in the block comment explaining the external dependency.

Reference: .github/instructions/hve-core/pester-tests.instructions.md — "All Describe blocks require -Tag 'Unit'" — this requirement applies to unit tests; the convention should be revisited for tests with external dependencies.

$manifest = Get-Content $script:outputPath -Raw | ConvertFrom-Json
$fields = ($manifest | Get-Member -MemberType NoteProperty).Name | Sort-Object
$expected = @('algorithm', 'artifacts', 'fileCount', 'generatedAt', 'projectSlug', 'version') | Sort-Object
$expected = @('algorithm', 'artifacts', 'fileCount', 'generatedAt', 'projectSlug', 'sessionPath', 'version') | Sort-Object
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Advisory: Missing test coverage for new BySessionPath parameter set

This change only updates the expected manifest fields list to include sessionPath, but the new BySessionPath parameter set in Sign-PlannerArtifacts.ps1 introduces several behaviors that lack test coverage:

  • Absolute vs. relative path resolution: Does -SessionPath 'some/relative/path' resolve against the working directory or the repo root? What happens with a path that already contains the repo root prefix?
  • -ManifestName override: Does specifying a custom manifest name correctly set the output filename?
  • Parameter set mutual exclusivity: Does combining -SessionPath with -ProjectSlug produce a clear error, or silently pick one parameter set?
  • Boundary cases: What happens if the -SessionPath directory doesn't exist? What if it contains no artifacts?

These behaviors are implemented in the script but completely untested. Given that this signing workflow is security-critical (it generates SHA-256 attestation manifests), coverage gaps here are worth addressing.

Consider adding a new Describe 'BySessionPath parameter set' block parallel to the existing 'Manifest Generation' block.

sssc-planner:
id: sssc-full-disclaimer
label: "SSSC Planner Full Disclaimer"
applies-to:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Advisory: Shared applies-to artifact names may need scope-based disambiguation

Both rai-planner and sssc-planner disclaimers now use identical applies-to values:

# rai-planner (existing)
applies-to:
  - handoff-summary
  - compact-handoff-summary

# sssc-planner (new)
applies-to:
  - handoff-summary
  - compact-handoff-summary

If the validation tooling resolves which disclaimer to apply using only the artifact name (without cross-referencing the file path against the tier scopes in footer-with-review.yml), any handoff-summary artifact would match both disclaimers simultaneously. The correct disclaimer depends on the file path: SSSC paths (security/sssc-*.instructions.md) should pick up sssc-full-disclaimer; RAI paths (rai-planning/**) should pick up the RAI disclaimer.

This works correctly if the validator joins through footer-with-review.yml tier scopes. If the disclaimer lookup is purely by artifact name, it will surface both disclaimers—or non-deterministically pick one. It may be worth adding a scope field to disclaimer entries (mirroring the tier scope) to make this disambiguation explicit and self-documenting.

Copy link
Copy Markdown
Contributor

@katriendg katriendg left a comment

Choose a reason for hiding this comment

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

Reviewed for cross-agent consistency, packaging sync, schema accuracy, and prompt-builder repetition rules. Detailed inline comments above; high-level summary below.

Findings

ID Severity Theme
RI-1 High Four prompts cross-reference a Disclaimer and Attribution Protocol section that does not exist in sssc-identity.instructions.md
RI-2 High Resume Protocol step counts diverge between sssc-planner.agent.md (5/6) and sssc-identity.instructions.md (still 4/5)
RI-3 Medium Startup, Output Preferences, Pre-Scan supporting context, and one Phase 1 question are duplicated verbatim across the four entry-mode prompts (~150 lines), violating prompt-builder.instructions.md line 37
RI-4 Medium sssc-state.schema.json requires signingRequested but not signingManifestPath or disclaimerShownAt, although the agent always writes them on init
RI-5 Low docs/agents/sssc-planning/agent-overview.md userPreferences row is stale (manual autonomy tier no longer exists; structured object not described)
RI-6 Trivia PR description says the footer-with-review file lives under .github/instructions/shared/; actual path is .github/config/footer-with-review.yml
RI-7 Low sssc-state.schema.json is referenced via $schema only — no validator runs against produced state.json files. Same is true of rai-state.schema.json, so parity is preserved; flagging as a follow-up

Strengths

  • Footer-with-review rename (human-facing-with-disclaimerrai-handoff-with-disclaimer) is clean — zero stale references repo-wide, tests and config updated together.
  • Sign-PlannerArtifacts.ps1 parameter sets (ByProjectSlug vs BySessionPath) are backward-compatible; the new sessionPath manifest field uses an OS-agnostic forward-slash relative path.
  • pester-tests.yml correctly adds actions/setup-node + npm ci because the new Format-MarkdownTables.ps1 depends on node_modules. Action SHA pinning with version comment respected.
  • SSSC standards rename (SLSA v1.0, NTIA SBOM minimum elements, Sigstore (cosign)) propagates consistently through frontmatter, three collection manifests, and three plugin READMEs.
  • sssc-full-disclaimer wording correctly substitutes "OpenSSF Scorecard evaluators / SLSA auditors / supply chain security review boards" for the RAI legal/compliance equivalents.
  • Tests cover the new SSSC handoff tier (pass + fail) and the sessionPath manifest field.

"nextActions",
"userPreferences",
"ssscEnabled",
"signingRequested"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Schema required set understates the contract (Medium).

signingRequested is now required, but signingManifestPath and disclaimerShownAt are not — even though the State Creation rules in sssc-identity.instructions.md ("State Creation" section) initialize all three on first invocation. With additionalProperties: false at the top level the schema is internally consistent, but the required set diverges from what the agent always writes.

Suggested change
"signingRequested"
"userPreferences",
"ssscEnabled",
"signingRequested",
"signingManifestPath",
"disclaimerShownAt"

Their existing ["string", "null"] type unions allow null, so adding them to required is non-breaking. For full parity, consider applying the same change to rai-state.schema.json (which has the same gap).

* Begin the Phase 1 interview about the project's supply chain security posture with 3-5 focused questions covering: project name and purpose, technology stack, package managers, CI/CD platform, release strategy, and known compliance targets (OpenSSF Scorecard, SLSA, Best Practices Badge).
### Pre-Scan

Before initialization, scan the workspace for context that can pre-populate Phase 1:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Heavy repetition across the four entry-mode prompts (Medium).

Per prompt-builder.instructions.md line 37: "Avoid adding Required Phases, Required Steps, or Required Protocol sections that duplicate or conflict with the parent agent's protocol."

The following blocks are duplicated verbatim across sssc-capture, sssc-from-prd, sssc-from-brd, and sssc-from-security-plan:

  • The entire Startup section (disclaimer-check + standards announcement, ~2 paragraphs).
  • The Output Preferences paragraph (~5 lines, identical wording).
  • The supporting-context bullet list under Pre-Scan (package.json, pyproject.toml, *.csproj, Cargo.toml, go.mod, CI paths, sibling planner trees, references/).
  • The trailing question about user-supplied evaluation standards under Phase 1 Entry.

sssc-planner.agent.md already loads sssc-identity.instructions.md for every entry mode, so factoring the shared blocks into the identity instruction (Startup Announcement + Output Preferences + supporting-context list) costs nothing at runtime and removes ~150 duplicated lines. Each prompt would then carry only its mode-specific delta: primary discovery path, scope-extraction list, and fallback rule.

@@ -87,6 +87,9 @@ The state file tracks 17 fields across scoping, analysis, and handoff concerns.
| `nextActions` | string[] | Pending actions for the current or next phase |
| `userPreferences` | object | Autonomy preference: `full`, `partial`, or `manual` |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Stale userPreferences description (Low).

The new signing/disclaimer rows added by this PR are correct, but this row was missed:

  • manual is not a valid autonomyTier — the schema enum is ["guided", "partial", "full"].
  • userPreferences is now a structured object with outputDetailLevel, targetSystem, audienceProfile, and includeOptionalArtifacts.
Suggested change
| `userPreferences` | object | Autonomy preference: `full`, `partial`, or `manual` |
| `userPreferences` | object | Autonomy tier (`guided`/`partial`/`full`), output detail level, target system, audience profile, and optional artifact toggles |


Before any phase work, check `state.json` for `disclaimerShownAt`. If `disclaimerShownAt` is `null` or `state.json` does not yet exist, display the SSSC Planning CAUTION block from #file:../../instructions/shared/disclaimer-language.instructions.md verbatim and set `disclaimerShownAt` to the current ISO 8601 timestamp in `state.json`.

After the disclaimer, announce the SSSC Planner standards baseline following the Disclaimer and Attribution Protocol in #file:../../instructions/security/sssc-identity.instructions.md: OpenSSF Scorecard, SLSA Build Levels, OpenSSF Best Practices Badge, Sigstore, and SBOM standards (CycloneDX, SPDX).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Broken cross-reference (High). This line points readers to a Disclaimer and Attribution Protocol section in sssc-identity.instructions.md, but no such section exists there — only rai-identity.instructions.md defines that protocol.

The same dangling reference appears at line 16 of sssc-from-prd.prompt.md, sssc-from-brd.prompt.md, and sssc-from-security-plan.prompt.md.

Suggested fix: add a ## Disclaimer and Attribution Protocol section to sssc-identity.instructions.md (preserves RAI parity), or re-point these prompts to the agent's existing ## Startup Announcement in sssc-planner.agent.md.

* `ssscEnabled` set to `true`
* `signingRequested` set to `false` until the user opts in during scoping
* `signingManifestPath` set to `null` until handoff signing runs
* `disclaimerShownAt` set to `null` until the SSSC Planning disclaimer is presented at session start
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Resume protocol now contradicts the agent file (High).

This PR added disclaimerShownAt initialization here, and sssc-planner.agent.md was updated to a Five-Step Resume Sequence and Six-Step Post-Summarization Recovery that re-display the disclaimer when disclaimerShownAt is null.

However, the ## Resume Protocol section further down in this same file (line ~178) was not updated — it still reads "Four-Step Resume Sequence" and "Five-Step Post-Summarization Recovery" with no disclaimer-check step. The two authoritative files now disagree on whether the disclaimer is re-shown after resume.

Suggested fix: update both subsections of ## Resume Protocol to match the agent's new step counts and insert the disclaimer-check step (If disclaimerShownAt is null, display the Startup Announcement and stamp the timestamp).

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.

3 participants