feat: add standardize-ai-native-development change proposal#218
feat: add standardize-ai-native-development change proposal#218marcusburghardt wants to merge 11 commits into
Conversation
|
Since this is mostly about documentation, maybe it would be helpful to trigger the |
Add OpenSpec change proposal for org-wide AI-native development standardization covering three capabilities: - ai-native-standards: community AI_NATIVE_DEVELOPMENT.md + STYLE_GUIDE alignment - per-repo-ai-guide: docs/AI.md rename, restructure, and sync updates - ci-council-review: reusable Vertex AI council review workflow Artifacts: proposal, design, 3 capability specs, tasks (18 tasks in 5 phases). Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: Marcus Burghardt <[email protected]>
Fix 1 CRITICAL and 10 HIGH findings from the Divisor review council: - Add positive-path gate check scenario (CRITICAL - Tester) - Add diff size limit: 1000 lines per persona (Adversary, SRE) - Add prompt injection resilience requirement (Adversary) - Add partial API failure and error handling scenarios (Architect, Adversary) - Define secret interface: AI_API_KEY + AI_API_ENDPOINT (Adversary) - Add old AI_TOOLING.md cleanup scenario and task (Guard, Architect, SRE) - Add cross-repo phase ordering constraints to tasks.md (Architect, SRE) - Fix step count: 5-step workflow, not 6 (Tester) - Document 5-vs-9 Divisor persona rationale in design (Architect, SRE) - Add deduplication gate check (same HEAD SHA) (Adversary) - Classify validation tasks by type (Tester) - Resolve open question Q2 (diff size limit) Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: Marcus Burghardt <[email protected]>
d90c63e to
66fdd85
Compare
Phase 2 — Per-Repo AI Guide: - Rename docs/AI_TOOLING.md to docs/AI.md - Add convention packs section, Divisor agent guidance, community link - Update AGENTS.md references and add Divisor agent documentation Phase 3 — CI Council Review Workflow: - Create reusable_council_review.yml with gate checks, mode detection, governance context collection, Divisor file presence check, and structured PR comment posting - Gate checks: skip dependabot, draft, binary-only, fork PRs - Spec vs code review auto-detection via changed files - AI API calls placeholder (provider TBD during integration phase) - Secret interface: AI_API_KEY + AI_API_ENDPOINT - 1000-line diff limit per persona with truncation notice - Prompt injection resilience via XML structural delimiters - Create ci_council_review.yml consumer workflow Phase 4 — Sync Configuration: - Update sync-config.yml: AI_TOOLING.md -> AI.md path - Add 5 Divisor review persona files to sync - Add ci_council_review.yml to sync - Document old file cleanup for downstream repos Phase 1 (community/) deferred — requires separate PR merged first. Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: Marcus Burghardt <[email protected]>
|
The parallel work in |
|
@marcusburghardt Just read that the complytime/community#10 must merge first. Please do add us for review whenever its ready. :) |
sonupreetam
left a comment
There was a problem hiding this comment.
AI Review Summary
2 HIGH findings that block CI and violate constitution requirements. 4 MEDIUM findings will be tracked in a separate follow-up PR via OpenSpec.
Verdict: REQUEST CHANGES
This review was generated by /review-pr (AI-assisted).
There was a problem hiding this comment.
[HIGH] SC2053: Unquoted glob in [[ ]] — blocking actionlint/CI
This comparison intentionally uses glob matching (patterns like *.png), but shellcheck flags it as SC2053. Add a disable directive on the line before this comparison:
# shellcheck disable=SC2053
if [[ "${file}" == ${pattern} ]]; then
[HIGH] Missing top-level name: key — constitution §YAML requires descriptive workflow names
The consumer workflow declares name: Council Review but reusable_council_review.yml has no name: key. Add name: Reusable Council Review before the on: block (line 21) for Actions UI clarity and constitution compliance.
These are the 2 HIGH findings from the AI review. See the REQUEST CHANGES review above for the full summary.
The inline suggestion format didn't work due to GitHub API issues with the diff position resolution on this PR (likely because the file was introduced across multiple commits).
The review body with REQUEST CHANGES and the detailed findings comment are both posted.
This is a known issue with the GitHub Reviews API when a PR has a complex diff history (this PR has 3 commits modifying the same new file). The diff position calculation can become ambiguous.
Follow-up: MEDIUM findings tracked via OpenSpecThe MEDIUM findings for follow-up
These will be addressed in a separate PR driven by This comment was generated by /review-pr (AI-assisted). |
The coordinator, worker, background-worker, gaze-reporter, and gaze-test-generator agents were missing the mode: subagent frontmatter field. OpenCode defaults missing mode to 'all', which made them appear as primary agents in the Tab cycle alongside Build and Plan. Also removed redundant name: fields from forge agents (OpenCode derives agent name from filename) and added scaffold comments for consistency with other uf-scaffolded agents. Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: Marcus Burghardt <[email protected]>
Add quotes around ${{ inputs.new-function-threshold }} shell assignment
to match the defensive quoting pattern used by BASELINE and EPSILON.
Addresses the pre-existing convention pack SC-002 finding noted in
PR complytime#224 review.
Includes OpenSpec change artifacts (proposal, design, spec, tasks).
Assisted-by: OpenCode (claude-opus-4-6)
Signed-off-by: sonupreetam <[email protected]>
Bumps [complytime/org-infra/.github/workflows/reusable_ci.yml](https://github.com/complytime/org-infra) from 0.1.0 to 0.2.0. - [Release notes](https://github.com/complytime/org-infra/releases) - [Commits](complytime/org-infra@baf5b2e...393ab96) --- updated-dependencies: - dependency-name: complytime/org-infra/.github/workflows/reusable_ci.yml dependency-version: 0.2.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
…ad_analysis.yml Bumps [complytime/org-infra/.github/workflows/reusable_crapload_analysis.yml](https://github.com/complytime/org-infra) from 0.1.0 to 0.2.0. - [Release notes](https://github.com/complytime/org-infra/releases) - [Commits](complytime/org-infra@baf5b2e...393ab96) --- updated-dependencies: - dependency-name: complytime/org-infra/.github/workflows/reusable_crapload_analysis.yml dependency-version: 0.2.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
…ance.yml Bumps [complytime/org-infra/.github/workflows/reusable_compliance.yml](https://github.com/complytime/org-infra) from 0.1.0 to 0.2.0. - [Release notes](https://github.com/complytime/org-infra/releases) - [Commits](complytime/org-infra@baf5b2e...393ab96) --- updated-dependencies: - dependency-name: complytime/org-infra/.github/workflows/reusable_compliance.yml dependency-version: 0.2.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
…ty.yml Bumps [complytime/org-infra/.github/workflows/reusable_security.yml](https://github.com/complytime/org-infra) from 0.1.0 to 0.2.0. - [Release notes](https://github.com/complytime/org-infra/releases) - [Commits](complytime/org-infra@baf5b2e...393ab96) --- updated-dependencies: - dependency-name: complytime/org-infra/.github/workflows/reusable_security.yml dependency-version: 0.2.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
…led.yml Bumps [complytime/org-infra/.github/workflows/reusable_scheduled.yml](https://github.com/complytime/org-infra) from 0.1.0 to 0.2.0. - [Release notes](https://github.com/complytime/org-infra/releases) - [Commits](complytime/org-infra@baf5b2e...393ab96) --- updated-dependencies: - dependency-name: complytime/org-infra/.github/workflows/reusable_scheduled.yml dependency-version: 0.2.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
HIGH fixes: - Add 'name: Reusable Council Review' to reusable workflow - Add shellcheck disable=SC2053 for intentional glob matching MEDIUM fixes: - Add exclude_repos (.github, complyscribe) to ci_council_review.yml sync entry for consistency with all other synced files - Use env var for skip_reason interpolation instead of direct expression injection in run block Deferred to follow-up (openspec/changes/fix-council-review-quality/): - gh pr diff pathspec syntax (needs design) - SHA deduplication gate check (document as deferred) Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: Marcus Burghardt <[email protected]>
Document the 7 changes needed to add Vertex AI WIF support to reusable_council_review.yml from PR complytime#218. Covers two-tier provider detection, WIF auth step, API call abstraction, consumer workflow updates, and concurrency control. This patch spec enables PR complytime#218 to be updated with WIF support once the GCP infrastructure is fully provisioned. Assisted-by: Cursor (claude-opus-4-6) Signed-off-by: sonupreetam <[email protected]> Co-authored-by: Cursor <[email protected]>
Document the 7 changes needed to add Vertex AI WIF support to reusable_council_review.yml from PR complytime#218. Covers two-tier provider detection, WIF auth step, API call abstraction, consumer workflow updates, and concurrency control. This patch spec enables PR complytime#218 to be updated with WIF support once the GCP infrastructure is fully provisioned. Assisted-by: Cursor (claude-opus-4-6) Signed-off-by: sonupreetam <[email protected]> Co-authored-by: Cursor <[email protected]>
| review_mode: ${{ steps.detect-mode.outputs.review_mode }} | ||
| skip_reason: ${{ steps.gate.outputs.skip_reason }} | ||
| steps: | ||
| - name: Check gate conditions |
There was a problem hiding this comment.
🛡️ We should only allow team members to trigger this. Otherwise, a malicious actor could burn our tokens excessively.
| fi | ||
|
|
||
| # Gate 2: Skip draft PRs | ||
| if [[ "${PR_DRAFT}" == "true" ]]; then |
There was a problem hiding this comment.
💡 Can we add a way to trigger this on a draft? It would be nice to be able to put things into the draft state to fix the review-council issues without needing to bother the rest of the team.
| exit 0 | ||
| fi | ||
|
|
||
| # Gate 3: Skip if API credentials are missing (fork PRs) |
There was a problem hiding this comment.
Nit: I'd move this one up front since it's the most "fail fast" case.
| id: binary-check | ||
| env: | ||
| ALL_CHANGED: ${{ steps.changed-files.outputs.all_changed_files }} | ||
| GENERATED_PATTERNS: ${{ inputs.generated_file_patterns }} |
There was a problem hiding this comment.
🐛 This is getting set, but not being used.
| ALL_CHANGED: ${{ steps.changed-files.outputs.all_changed_files }} | ||
| GENERATED_PATTERNS: ${{ inputs.generated_file_patterns }} | ||
| run: | | ||
| binary_lock_patterns=( |
There was a problem hiding this comment.
💡 If we want this to be comprehensive, we may want to override the .gitignore in the repo with a static file on the fly.
If that's not possible, we might want to remove the files directly unless that would break the review council functionality (it might since IIRC it runs tests). Disabling the review council test runs is probably a good idea on CI builds since we should not be running this job and burning tokens if the CI tests didn't pass.
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Check Divisor agent files |
There was a problem hiding this comment.
💡 How about a quick pre-check where we have an LLM check for "meaningful" changes? This would prevent burning tokens on a full review cycle where all of the changes are trivial.
| fetch-depth: 0 | ||
|
|
||
| - name: Check Divisor agent files | ||
| id: check-agents |
There was a problem hiding this comment.
❓ Not quite sure what this job is for? Is this just checking that everything was installed properly? If so, that should be caught in the tests for publishing the origin material.
| echo "---" >> /tmp/council_comment.md | ||
| echo "*Generated by AI Council Review. Findings assist" \ | ||
| "human reviewers and do not replace human judgment.*" \ | ||
| >> /tmp/council_comment.md |
There was a problem hiding this comment.
💡 I'm going to suggest that the run steps be replaced with an inline call to opencode directly with the credentials wired up or being driven by promptfoo.
I have some pre-existing work in lola-eval that we could steal from if we decide to go this way.
| - **Standards**: All coding standards are in `.specify/memory/constitution.md`. Do not duplicate them. | ||
| - **AI tooling**: Setup, commands, and skill creation documented in `docs/AI_TOOLING.md`. | ||
| - **AI tooling**: Setup, commands, convention packs, and skill creation documented in `docs/AI.md`. | ||
| - **Divisor agents**: 5 review personas in `.opencode/agents/divisor-*.md` (guard, architect, adversary, testing, sre) are synced to all org repos and used by the CI council review workflow. 4 additional content personas (curator, scribe, herald, envoy) are org-infra only. |
There was a problem hiding this comment.
💡 Instead of a push, this may be more effective as a pull from the extracted review-council using lola
There was a problem hiding this comment.
💡 Instead of a push, this may be more effective as a pull from the extracted review-council using lola
Summary
ai-native-standards,per-repo-ai-guide,ci-council-reviewArtifacts
proposal.mddesign.mdspecs/ai-native-standards/spec.mdspecs/per-repo-ai-guide/spec.mdspecs/ci-council-review/spec.mdtasks.mdKey Design Decisions
AI_NATIVE_DEVELOPMENT.mdas org-wide standard (philosophy + workflow)docs/AI.md(renamed fromAI_TOOLING.md) synced from org-infrapull_requestevent — secrets not exposed to forks, external PRs skip review