Repository navigation
fix: repair CI documentation contracts and regression fixtures - #71194
Conversation
Modernize audit diff literals and split oversized diff functions without changing their output. Update documentation compilation coverage, queue synchronization fixtures, reserved-job assertions, and quota stubs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Ponytail ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Commit pushed:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Request changes
The refactor replaces compilable DiffEntryBase: DiffEntryBase{Status: ...} initializers with promoted Status: keys. Go rejects promoted fields in keyed struct literals, so pkg/cli/audit_diff.go will not compile; please restore explicit embedded-field initialization at all affected sites.
The review is static; no repository commands or tests were executed.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · 1.94 AIC · ⌖ 0.547 AIC · ⊞ 1.6K
Comment /review to run again
Comments that could not be inline-anchored
pkg/cli/audit_diff.go:146
These composite literals use Status as if it were a declared field, but DomainDiffEntry embeds DiffEntryBase, so Go does not allow promoted fields in keyed struct literals; this makes the package fail to compile at every newly rewritten Status: literal in this file.
Content redacted to fit size limit.
🏗️ Design Decision Gate — ADR RequiredThis PR adds 283 new lines in business-logic directories ( 📄 Draft ADR committed: The draft records the main design choice inferred from the diff. Gallery docs now link to canonical repository workflows instead of embedding
📋 What to do next
After the ADR is linked in the PR body, this gate will run again and check that the implementation matches the decision. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections:
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd. This PR is primarily a regression-fixture repair; no blocking issues found. Two minor suggestions posted inline.
📋 Key Themes & Highlights
Key Themes
- Reserved-job-name regression repaired:
compiler_development_effects_validation_test.goandcompiler_development_safety_test.gopreviously asserted that compiling a custom job namedupload_assets(a reserved builtin name) succeeded and preserved an attacker-controlledBASH_ENV: ./unsafe.shenv override. This PR corrects those tests to expect the actual (and correct)jobs.upload_assets: this name is reservedrejection — this is a genuine/diagnosing-bugsfix for a prior test that masked a real validation gap. audit_diff.gorefactor is behavior-preserving: theDiffEntryBaseflattening relies on Go's promoted-field composite-literal syntax, which is valid and compiles cleanly — verified locally. Function extraction (mcpToolsForDiff,appendMCPToolDiff,toolCallsForDiff,appendToolCallDiff,runSummaryDiffMetrics,setRunDurationDiff) is a clean, low-risk split.- New integration-test fallback (
compileRepositoryExampleWorkflowindocs_examples_integration_test.go) silently drops the original extraction error when it falls back to regex-matched repository links — flagged inline since it weakens future diagnosability if both paths fail. work_command_test.goadds ref-pinned settings verification but only covers the 404 path foraw.json; the 200/found path for settings application isn't exercised, flagged inline per/tdd.
Positive Highlights
- ✅ Reserved-name test fix closes a real gap where a dangerous custom job override was silently accepted by test expectations.
- ✅
health_approval_expiry_backfill_test.goadds arateCheckscounter assertion ensuring quota rechecking happens without live GitHub credentials — solid regression guard. - ✅ Large
audit_diff.godiff preserves output format while meaningfully reducing duplication.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 76.8 AIC · ⌖ 16.3 AIC · ⊞ 10.2K
Comment /matt to run again
There was a problem hiding this comment.
🟡 Changes recommended
The gallery validation fallback can suppress malformed inline-example errors and allow broken documentation to pass.
2 open issues
What changed in this PR
Repairs gh-aw documentation contracts and regression fixtures after the work-queue documentation relocation, alongside an audit-diff refactor.
Changes:
- Refresh work-queue links, documentation checks, and Issue-projection diagnostic guidance.
- Add isolated compilation fixtures for linked gallery workflows and repair CLI/compiler test mocks.
- Extract audit-diff helpers and simplify literals, with no intended output changes.
| File | Description |
|---|---|
specs/work-queue/README.md |
Update the native queue deployment link. |
pkg/workflow/work_queue_persistent_memory_integration_test.go |
Check workflow links in the relocated gallery page. |
pkg/workflow/docs_examples_integration_test.go |
Compile linked repository workflows with isolated import fixtures. |
pkg/workflow/compiler_development_safety_test.go |
Expect reserved job-name rejection outside dry-run mode. |
pkg/workflow/compiler_development_effects_validation_test.go |
Align restore-memory assertions with reserved job-name validation. |
pkg/cli/work_command_test.go |
Mock verified default-revision settings lookup. |
pkg/cli/health_approval_expiry_backfill_test.go |
Stub quota refreshes and verify rate-limit checks. |
pkg/cli/audit_diff.go |
Extract comparison helpers and modernize struct literals. |
actions/setup/js/work_queue_documentation.test.cjs |
Update relocated paths, settings examples, and link checks. |
.github/aw/debug-playbook.md |
Expand Issue-projection diagnostics within the size budget. |
Review effort: Max
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Clarify projection recovery guidance and finalize the gallery validation ADR. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Addressed the remaining review feedback in c0bd98e: gallery fallback now rejects malformed or duplicate inline examples, projection recovery guidance no longer assumes access failure, and the generated ADR is finalized and linked in the PR description. All four inline threads have been answered and resolved. The older audit-diff compilation objection is declined: this repository requires Go 1.27.3, which supports these promoted-field literals; Changed-code lint is clean. The full publication gate remains blocked by existing modernization diagnostics in unchanged audit_expanded.go/audit_summary_build.go and an unknown-linter warning; its custom-lint, JavaScript, schema, and impacted-test stages pass. |
|
🎉 This pull request is included in a new release. Release: |
Summary
ADR: ADR-71194: Validate Gallery Docs by Compiling Linked Repository Workflows
Validation
Passed:
make build,make fmt,make lint-cjs, andmake test-guidance(24 tests).make check-workflow-drift: all 346 workflows compile, with locks in sync.--new-from-rev=origin/main: zero changed-code issues.Limitation:
make agent-report-progressremains blocked by existing modernization diagnostics in unchangedpkg/cli/audit_expanded.goandpkg/cli/audit_summary_build.go, plus an unknown-linter warning that prevents the gate from filtering those unrelated diagnostics. Custom Go lint, JavaScript lint, schema, and impacted-test stages passed. Earlier full-lint attempts timed out; remote CI has not been verified for the latest revision.