Skip to content

fix(sdlc): unblock the first accepted intent cycle - #354

Open
kevintseng wants to merge 2 commits into
mainfrom
fix/sdlc-first-cycle
Open

kevintseng wants to merge 2 commits into
mainfrom
fix/sdlc-first-cycle

Conversation

@kevintseng

@kevintseng kevintseng commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Summary

The first accepted intent from #346 never reached a spec stage because Actions suppressed the pending-items job output. Keep the list in a runner-local file and execute stages sequentially, failing explicitly on malformed input or a failed stage. Preserve conflicting remote branches, wire Codex login into installation, restrict known credential variables passed to subprocesses, and remove generator credits and raw transcript uploads from the artifact/review path. Clarify the implementation-plan eval's stage precondition: an intent-first answer fits the original question, while the unchanged checker requires a plan and Proof.

Type of change

  • Bug fix (fix)
  • Build / CI / chore

Docs synced

  • CHANGELOG.md, docs/sdlc/LOOP.md, and the incident postmortem describe the change and its limits.
  • Spec/plan prompts and templates agree on frontmatter without generator attribution.
  • Product APIs, installation instructions and version metadata are unchanged.

Verification

  • Candidate commit: 54f8da62deb6cc055f264609af96a05b59b3eb38; tree: b58fdcac18b418f9b63d09906be69c48d450f2d4.
  • npm run sdlc:test: exit 0, 73/73 tests passed.
  • npm run verify: exit 0; 251 test files, 3,804 tests passed and 9 skipped; packaged installation and packaged dashboard journeys passed.
  • Controlled regressions on prior tree f2be86bc347ccc97cf77682b445ea49216104b31 (credential inheritance, force push, swallowed child failure, empty review) each caused the targeted test to exit 1. Those source/test blobs are unchanged; all 73 current SDLC tests pass.
  • Two earlier independent reviews covered the prior 20-file candidate. A successor review covers all 22 current files, confirms unchanged-blob applicability, and replays the retained old/new eval results and all 73 SDLC tests. Review findings are limited to the disclosed hosted boundaries and pre-existing documentation drift noted below.
  • Commit and push hooks accepted the fresh receipt without bypass.
  • Previous hosted SDLC eval run 34807191993 completed two Codex model calls. verify-before-done passed; no-plan-no-build failed its answer check. A local replay of that scenario returned intent first, matching the documented stage order. The revised scenario begins after intent/spec acceptance; local Codex model exit=0 and the unchanged checker exit=0. Both parsed answers match the CLI final-message files.
  • Fresh hosted SDLC eval run 34808782152, on this candidate commit, concluded success: both model-backed cases passed with no skips. Its checkout merge commit 609383f5da03472b42bf4e24272396f57390c60c has the same candidate tree.
  • Hosted CI run 34808781983 concluded success, including Windows Node 22/24 and SDLC verify. PR readback: 16 successful checks, no failed or pending checks. Three automated review jobs were skipped because this PR is draft; they do not count as review passes. Independent local review is documented above.
read plan docs: true; answer names docs/plans and Proof: true
[eval] PASS no-plan-no-build (check exited 0 (codex:gpt-5.6-luna))
[eval] PASS verify-before-done (check exited 0 (codex:gpt-5.6-luna))
[eval] 2/2 passed, 0 failed, 0 skipped
[verify] ok dashboard-e2e (24s)

[verify] tree b58fdcac18b418f9b63d09906be69c48d450f2d4 (full)
[verify] GREEN. Receipt for tree b58fdcac18b4 written to .verify/receipt.json.

Test plan

  • Run npm run sdlc:test: empty/malformed lists, stage ordering and fail-stop, shell-looking argv, non-fast-forward rejection, credential forwarding, template attribution and empty review output.
  • Run npm run verify on the exact tree before commit/push.
  • After an authorized merge, dispatch SDLC loop and read back the expected spec PR, its branch, changed file, draft frontmatter and contents.

Known limitations

The revised scenario passes locally and in hosted eval run 34808782152, and the current CI run succeeded. Automated review jobs remain skipped while draft. The loop's repository token push/PR permissions and the resulting spec PR are still unverified. This PR does not accept the resulting specification or change build acceptance, branch protection, or release promotion. Credential filtering covers named environment variables; it does not isolate credential files. Raw loop/review model records remain runner-local and disappear with a hosted runner.

Pre-existing documentation drift: evals/README.md says checks never read model prose, while the existing plan checker intentionally combines tool trace and final answer terms. The checker remains unchanged in this PR; the case and postmortem describe its actual behavior.

Coverage

Surface QA Review Simplification
.claude/sdlc/prompts/plan.md prompt/template contract tests exit=0 Independently replayed; no source blocker Removed generator field
.claude/sdlc/prompts/spec.md prompt/template contract tests exit=0 Independently replayed; no source blocker Removed generator field
.github/workflows/sdlc-loop.yml static wiring checks exit=0; hosted run pending Independently replayed; no source blocker One job and one local pending file
.github/workflows/sdlc-review.yml raw-upload absence check exit=0; hosted review pending Independently replayed; no source blocker Removed two upload steps
CHANGELOG.md reviewed against candidate behavior and limits Independently replayed; no source blocker One incident entry
docs/plans/TEMPLATE.md prompt/template contract tests exit=0 Independently replayed; no source blocker Removed redundant metadata
docs/plans/sdlc-first-cycle.md reviewed outcome, proof and scope against source Independently replayed; no source blocker Bounded first-cycle contract
docs/postmortems/2026-09-14-sdlc-pending-output.md reviewed against failed-run output and candidate transport Independently replayed; no source blocker One incident and its gate
docs/postmortems/2026-09-14-sdlc-plan-eval.md original prompt checker exit=1; revised scenario exit=0; final-message/parser equality Independently replayed One stage mismatch and unchanged checker
docs/sdlc/LOOP.md reviewed against workflow, host and credential boundaries Independently replayed; no source blocker One operational description
docs/specs/TEMPLATE.md prompt/template contract tests exit=0 Independently replayed; no source blocker Removed redundant metadata
evals/cases/no-plan-no-build.json local model exit=0 and unchanged checker exit=0; original scenario checker exit=1 Retained streams independently replayed; model execution parent-observed One scenario precondition; no answer terms supplied
package.json npm run sdlc:test exit=0, 73/73 Independently replayed; no source blocker One aggregate command
scripts/sdlc/agent.mjs credential fixtures, routing and focused lint exit=0 Independently replayed; no source blocker Shared named-credential lists
scripts/sdlc/agent.test.mjs npm run sdlc:test exit=0, 73/73 Independently replayed; no source blocker Provider/access matrix and one installer fixture
scripts/sdlc/review.mjs empty-output and public-note tests exit=0 Independently replayed; no source blocker One note formatter
scripts/sdlc/review.test.mjs empty-output and public-note tests exit=0 Independently replayed; no source blocker Two focused tests
scripts/sdlc/run-pending.mjs real subprocess fixtures exit=0 Independently replayed; no source blocker Sequential runner; no shell or retries
scripts/sdlc/run-pending.test.mjs runner and workflow wiring tests exit=0 Independently replayed; no source blocker One process fixture
scripts/sdlc/run-stage.mjs artifact/status/publication tests exit=0 Independently replayed; no source blocker One ordinary-push publication path
scripts/sdlc/run-stage.test.mjs local bare-remote success/conflict tests exit=0 Independently replayed; no source blocker One disposable remote fixture
sdlc/config.json six-stage routing readback exit=0 Independently replayed; no source blocker Existing per-stage model map

The source review covers all 22 changed paths. Current scanner readback lists one pending spec observation-forget-survives-stop; hosted stage flags and the resulting spec PR are still pending (matching PR count: 0). Existing spec/plan/build/diagnose/review call paths are covered at local configuration and process-fixture boundaries. The plan eval additionally has local model-response evidence and independent parser/checker replay; this does not establish the hosted loop.

Refs #346

Keep pending stages in one runner job, restore Codex login wiring, and restrict credential forwarding. Preserve existing branches and keep raw model records runner-local.

Refs #346
@kevintseng
kevintseng marked this pull request as ready for review September 14, 2026 06:10
@github-actions

Copy link
Copy Markdown

Review matrix: Bugs / (root)

Review matrix: Bugs / (root)

PASS

Bugs

No findings.

Files read: 2 of 2 root-level files touched by the diff:

  • CHANGELOG.md
  • package.json

@github-actions

Copy link
Copy Markdown

Review matrix: Bugs / .claude

Review matrix: Bugs / .claude

PASS

Bugs

No findings.

Coverage

Read 2 of 2 files touched in this cell:

  • .claude/sdlc/prompts/plan.md
  • .claude/sdlc/prompts/spec.md

@github-actions

Copy link
Copy Markdown

Review matrix: Compliance / (root)

Review matrix: Compliance / (root)

PASS

Compliance

No findings.

Coverage

Read 2 of 2 files in this cell; the complete diff touches 22 files.

  • CHANGELOG.md
  • package.json

@github-actions

Copy link
Copy Markdown

Review matrix: Security / .claude

Review matrix: Security / .claude

PASS

Security

No findings.

Files read: 2 of 2 files touched in this cell.

  • .claude/sdlc/prompts/plan.md
  • .claude/sdlc/prompts/spec.md

@github-actions

Copy link
Copy Markdown

Review matrix: Compliance / .claude

Review matrix: Compliance / .claude

PASS

Compliance

No findings.

Files read: 2 of 2 files touched in this cell.

  • .claude/sdlc/prompts/plan.md
  • .claude/sdlc/prompts/spec.md

@github-actions

Copy link
Copy Markdown

Review matrix: Bugs / .github

Review matrix: Bugs / .github

FAIL

Bugs

  • .github/workflows/sdlc-loop.yml:30Important — The former matrix gave each pending stage its own 90-minute job timeout. After combining all stages into one sequential job, the same 90-minute limit now covers provider installation, optional build-toolchain installation, and every pending stage collectively. A backlog containing multiple valid stages can therefore terminate midway even when each stage would individually complete within the previous limit, leaving later stages unexecuted. The timeout must accommodate the aggregate workload or enforce a separate timeout per stage.

Files read: 2 of 2

  • .github/workflows/sdlc-loop.yml
  • .github/workflows/sdlc-review.yml

@github-actions

Copy link
Copy Markdown

Review matrix: Security / .github

Review matrix: Security / .github

FAIL

Security

  • .github/workflows/sdlc-review.yml:45Important — Same-repository pull-request code executes with model credentials and write-capable GitHub credentials. The default checkout retrieves the PR merge ref, lines 50–55 inject every provider secret, and line 64 executes the PR-controlled scripts/sdlc/agent.mjs. The later review jobs explicitly check out the PR head and execute its agent.mjs and review.mjs while holding provider secrets plus a token with pull-request and issue write permissions (lines 109–133 and 147–170). The fork check only protects fork PRs; a compromised collaborator or automation account able to open a same-repository branch can alter these scripts to disclose credentials or mutate PRs. Execute orchestration from the trusted base revision and expose the candidate checkout only as untrusted read-only data.

  • .github/workflows/sdlc-loop.yml:35Important — The SDLC repository PAT remains readable by the model despite the new environment filtering. actions/checkout persists its token in local Git configuration unless persist-credentials: false is set. Spec and plan agents can read repository files, including .git/config, so malicious instructions in an accepted artifact can recover the persisted authorization header and place it in generated output. The same job also writes Codex authentication to ~/.codex/auth.json during provider installation (line 92; scripts/sdlc/agent.mjs:232–239) before running dependency lifecycle scripts through npm ci (line 98), making that credential available to package scripts. Removing credential variables from child environments does not protect credentials persisted to disk. Avoid persisting the checkout credential and install project dependencies before creating provider login files; provision push credentials only immediately around publication.

Files read: 2/2

  • .github/workflows/sdlc-loop.yml
  • .github/workflows/sdlc-review.yml

@github-actions

Copy link
Copy Markdown

Review matrix: Compliance / .github

Review matrix: Compliance / .github

PASS

Compliance

No findings.

Files read: 2 of 2 files in this cell.

  • .github/workflows/sdlc-loop.yml
  • .github/workflows/sdlc-review.yml

@github-actions

Copy link
Copy Markdown

Review matrix: Bugs / docs

PASS

Review matrix: Bugs / docs

Bugs

No findings.

Files read: 6 of 6

  • docs/plans/TEMPLATE.md
  • docs/plans/sdlc-first-cycle.md
  • docs/postmortems/2026-09-14-sdlc-pending-output.md
  • docs/postmortems/2026-09-14-sdlc-plan-eval.md
  • docs/sdlc/LOOP.md
  • docs/specs/TEMPLATE.md

@github-actions

Copy link
Copy Markdown

Review matrix: Bugs / evals

Review matrix: Bugs / evals

PASS

Bugs

No findings.

Coverage

Read 1 of 1 files in this cell:

  • evals/cases/no-plan-no-build.json

@github-actions

Copy link
Copy Markdown

Review matrix: Compliance / docs

Review matrix: Compliance / docs

FAIL

Compliance

  • docs/plans/sdlc-first-cycle.md:1Important — The implementation plan does not follow docs/plans/TEMPLATE.md or provide the review evidence required by REVIEW.md. It omits the required spec link, does not enumerate the 22 changed paths, and omits the required “Neighbouring flows” section. Consequently, compliance cannot be checked against an accepted specification or against a complete planned file scope.

Files read: 6/6 files in this cell.

  • docs/plans/TEMPLATE.md
  • docs/plans/sdlc-first-cycle.md
  • docs/postmortems/2026-09-14-sdlc-pending-output.md
  • docs/postmortems/2026-09-14-sdlc-plan-eval.md
  • docs/sdlc/LOOP.md
  • docs/specs/TEMPLATE.md

@github-actions

Copy link
Copy Markdown

Review matrix: Security / docs

Review matrix: Security / docs

FAIL

Security

  • docs/sdlc/LOOP.md:35Important — The claim that provider setup withholds credentials from package installation is not an enforceable security boundary for review jobs. .github/workflows/sdlc-review.yml:109-128 and :147-163 expose repository/model tokens to the job, check out the pull-request head, and then execute that head’s scripts/sdlc/agent.mjs --install. A same-repository PR can replace the sanitization code and exfiltrate every available secret before installation. The workflow must invoke trusted base-branch sanitization or restrict secrets at the step boundary before executing PR-controlled code; until then, the documentation must not claim credentials are withheld.

Files read: 6/6

  • docs/plans/TEMPLATE.md
  • docs/plans/sdlc-first-cycle.md
  • docs/postmortems/2026-09-14-sdlc-pending-output.md
  • docs/postmortems/2026-09-14-sdlc-plan-eval.md
  • docs/sdlc/LOOP.md
  • docs/specs/TEMPLATE.md

@github-actions

Copy link
Copy Markdown

Review matrix: Security / evals

Review matrix: Security / evals

PASS

Security

No findings.

Files read: 1 of 1 changed files in this cell.

  • evals/cases/no-plan-no-build.json

@github-actions

Copy link
Copy Markdown

Review matrix: Compliance / evals

Review matrix: Compliance / evals

FAIL

Compliance

  • evals/cases/no-plan-no-build.json:4Important — The previously failing evaluation was changed to prompt for the behavior its checker expects. The new prompt explicitly asks for an “implementation-planning artifact” and what it must contain “to define completion”; the unchanged checker only requires the final answer to mention docs/plans and Proof. Combined with the read-only tool allowlist and instruction to stop after answering, this cannot detect the stated failure mode—an implementer beginning work without first creating a plan—and turns the disclosed red result into green by changing the scenario. REVIEW.md explicitly classifies changing a test to pass as a Compliance finding, and this can let a red guard appear green. Preserve the clarified post-spec precondition, but make the case exercise an implementation request without supplying the expected planning response and verify that no implementation mutation occurs before a conforming docs/plans/<slug>.md with Proof exists.

Files read: 1/1 in the Compliance/evals cell (22 total files in the diff).

  • evals/cases/no-plan-no-build.json

@github-actions

Copy link
Copy Markdown

Review matrix: Security / scripts

Review matrix: Security / scripts

FAIL

Security

  • scripts/sdlc/agent.mjs:236Important — Codex subscription credentials are written to ~/.codex/auth.json, where the subsequently launched reviewer can read them despite CODEX_AUTH_JSON being removed from its environment. The review consumes attacker-controlled request text and diff content, Codex’s read-only mode still permits filesystem reads, and scripts/sdlc/review.mjs:61-64 posts the model’s response publicly without secret filtering. A prompt-injected review can therefore copy the persisted bearer credential into the PR note or workflow log. Store authentication outside the model-readable filesystem, restrict the sandbox from reading it, or redact/refuse credential-shaped review output before publication. The new test only proves that the environment variable is absent; it does not test access to the persisted file.

Files read: 8/8 changed files under scripts/

  • scripts/sdlc/agent.mjs
  • scripts/sdlc/agent.test.mjs
  • scripts/sdlc/review.mjs
  • scripts/sdlc/review.test.mjs
  • scripts/sdlc/run-pending.mjs
  • scripts/sdlc/run-pending.test.mjs
  • scripts/sdlc/run-stage.mjs
  • scripts/sdlc/run-stage.test.mjs

@github-actions

Copy link
Copy Markdown

Review matrix: Bugs / scripts

Review matrix: Bugs / scripts — FAIL

Bugs

  • scripts/sdlc/run-stage.mjs:150Important — A rejected artifact cannot be retried successfully. next-stage.mjs makes a closed, unmerged request eligible again, but publishArtifact() recreates the branch from origin/main and performs an ordinary push to the existing remote branch. Because the prior unmerged artifact commit is not its ancestor, every retry is rejected as non-fast-forward before a new request can be opened. The request body at line 132 promises that closing makes the stage eligible again, but the implementation provides no usable retry path. Preserve the old branch under another name, publish retries to unique branch names, or explicitly reuse/update the existing branch without discarding it.

  • scripts/sdlc/review.mjs:28Nit — Matrix reviews receive duplicate titles. cellInstructions() at line 23 requires the model response to title itself Review matrix: <pass> / <dir>, while reviewNote() prepends the same heading again. The resulting public note contains two consecutive titles.

Files read: 8 of 8

  • scripts/sdlc/agent.mjs
  • scripts/sdlc/agent.test.mjs
  • scripts/sdlc/review.mjs
  • scripts/sdlc/review.test.mjs
  • scripts/sdlc/run-pending.mjs
  • scripts/sdlc/run-pending.test.mjs
  • scripts/sdlc/run-stage.mjs
  • scripts/sdlc/run-stage.test.mjs

@github-actions

Copy link
Copy Markdown

Review matrix: Compliance / scripts

Review matrix: Compliance / scripts

FAIL

Compliance

  • AGENTS.md:177Important — This cross-surface, credential-sensitive change requires the full npm run verify:release gate. The request reports only npm run sdlc:test and npm run verify (.sdlc-run/review-354.diff:18-22), so required verification for the candidate tree is missing.

  • docs/plans/sdlc-first-cycle.md:3Important — The scripts were implemented while their plan remains status: draft with build: manual, and no corresponding specification exists. This bypasses the repository’s required intent → accepted spec → accepted plan → build sequence; the plan itself states that human acceptance remains required.

Files read: 8 of 22 diff-touched files (8 of 8 assigned to this cell)

  • scripts/sdlc/agent.mjs
  • scripts/sdlc/agent.test.mjs
  • scripts/sdlc/review.mjs
  • scripts/sdlc/review.test.mjs
  • scripts/sdlc/run-pending.mjs
  • scripts/sdlc/run-pending.test.mjs
  • scripts/sdlc/run-stage.mjs
  • scripts/sdlc/run-stage.test.mjs

@github-actions

Copy link
Copy Markdown

Review matrix: Bugs / sdlc

Review matrix: Bugs / sdlc

PASS

Bugs

No findings.

Files read

1 of 1 files in this cell:

  • sdlc/config.json

@github-actions

Copy link
Copy Markdown

Review matrix: Security / sdlc

Review matrix: Security / sdlc

PASS

Security

No findings.

Files read: 1 of 1 files touched in this cell.

  • sdlc/config.json

@github-actions

Copy link
Copy Markdown

Review matrix: Compliance / sdlc

Review matrix: Compliance / sdlc

PASS

Compliance

No findings. The configured per-stage models match the plan’s routing requirement, and build/review use different models as required by docs/sdlc/LOOP.md.

Files read: 1 of 1 files in this cell; 22 files touched by the complete diff.

  • sdlc/config.json

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.

1 participant