From 5cfeb65fc1fecdaa79811f38d263bce1f6cb40d5 Mon Sep 17 00:00:00 2001 From: "claude-ceo-assistant (Claude Opus 4.7 on Hongming's MacBook)" Date: Fri, 8 May 2026 23:47:28 -0700 Subject: [PATCH] feat(team): comprehensive PR-gate + 100% coverage + e2e + identity-tag mechanics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Hongming's audit directive (2026-05-09): make the core team operate-by-mechanism rather than self-report. ## SHARED_RULES.md §PR Merge Approval Gate (extended) - Tag prefix corrected: `[-qa-agent]` etc., not bare `[qa-agent]`. Bare unprefixed tags rejected by lint. Each persona has its own Gitea identity (post-2026-05-06; feedback_per_agent_gitea_identity_default), so the tag reflects who actually authored. - Coverage bar bumped 80% → 100% per-changed-file. Aggregate doesn't satisfy. Doc-only files exempt. - e2e gate added: PRs touching workspace-server/canvas/workspace/ controlplane/plugins MUST run the matching tests/e2e/test_*.sh and the QA approval comment MUST report `e2e: =pass`. - §Issue Discipline tightened to a 5-min SLA. The orchestrator cross-checks Loki finding-events vs Gitea issue creates and files a [missed-finding] issue when a finding event has no matching issue. - §PR Template requirement added (links to .gitea/pull_request_template.md in internal + molecule-core; scripts-lint enforces). - §Identity Tag updated: "GitHub" → "Gitea"; mechanical-parsing rationale spelled out. ## dev-lead/core-lead/schedules/orchestrator-pulse.md (rewritten) - Replaces "merge CI-green PRs FIRST" with the four-condition gate-check sequence (CI green AND [core-qa-agent] ✅ AND [core-security-agent] ✅ AND [core-uiux-agent] ✅-or-N/A). - Force-merge call-out: explicitly fires incident.force_merge to Loki and reports to orchestrator (audit-force-merge.scripts). - Reviewer-rotation §SOP-10 check before approving. - Structured-logging report at end of each pulse so the orchestrator can monitor team behavior in Loki. - Fixes duplicate "Step 2" (was both SCAN TEAM STATE and REVIEW OPEN PRs). ## dev-lead/core-lead/core-qa/system-prompt.md - IDENTITY TAG header points at the gate-parsing role. - 100% per-changed-file coverage codified. - e2e mandatory on platform-touching PRs. - New §PR Review section: required comment-on-every-open-PR each cycle with one of three exact forms (APPROVED/CHANGES/N-A). ## dev-lead/core-lead/core-security/system-prompt.md - Same identity-tag fix. - File findings as Gitea issues (was "GitHub issues") within 5 min. - Required PR review on every PR touching auth/middleware/db/handlers/ plugin-install; quick-N/A on the rest. - New §PR Review section with the three exact comment forms. Tier: medium (changes how 9 personas behave; spine of dev tree). Verification: - Markdown structurally consistent - All edits surgical — no per-engineer prompt changes (those follow naturally from SHARED_RULES.md) - live verification deferred to Layer C (when workspaces actually boot with the new prompts) --- SHARED_RULES.md | 30 ++++++--- dev-lead/core-lead/core-qa/system-prompt.md | 21 ++++++- .../core-lead/core-security/system-prompt.md | 19 +++++- .../core-lead/schedules/orchestrator-pulse.md | 62 +++++++++++++------ 4 files changed, 101 insertions(+), 31 deletions(-) diff --git a/SHARED_RULES.md b/SHARED_RULES.md index 54fdeda..2f3510f 100644 --- a/SHARED_RULES.md +++ b/SHARED_RULES.md @@ -295,9 +295,9 @@ The 24h log shows multiple "PM not responding to DMs" escalations within minutes ## Identity Tag Every External Comment -Every GitHub PR description, issue body, comment, and Slack message MUST start with `[-agent]` on the first line (e.g., `[core-lead-agent]`, `[devrel-engineer-agent]`). +Every Gitea PR description, issue body, comment, and Slack message MUST start with `[-agent]` on the first line (e.g., `[core-lead-agent]`, `[devrel-engineer-agent]`). -This is required because the team shares one GitHub App identity (`molecule-ai[bot]`). Without tags, post-incident review can't attribute work to the right agent. +Tags are now ALSO mechanically required for PR approval gates. The PR Merge Approval Gate above parses comment bodies for `[-qa-agent] APPROVED` / `[-security-agent] APPROVED` / `[-uiux-agent] APPROVED`; an unprefixed `[qa-agent]` is rejected by the lint workflow. Each persona has its own Gitea identity (post-2026-05-06; see `feedback_per_agent_gitea_identity_default`), so the tag reflects who actually authored the comment — and the gate enforces that the right roles spoke. ## Merge Authority — Leads Merge in Their Domain @@ -315,19 +315,35 @@ If you're an engineer and find yourself wanting to run `tea pr merge`, stop and Before a Lead runs `tea pr merge`, **all four** of these must be on the PR: -1. **All required CI checks green** — `tea pr checks ` shows every gating check passing -2. **`[qa-agent] APPROVED`** — QA Engineer ran tests and reports clean (or `[qa-agent] N/A — docs only` waiver) -3. **`[security-auditor-agent] APPROVED`** — Security Auditor reviewed for CWE classes (or `N/A — pure docs/marketing` waiver) -4. **`[uiux-agent] APPROVED`** — UIUX Designer reviewed any canvas/UI changes (or `N/A — backend-only` waiver) +1. **All required CI checks green** — `tea pr checks ` shows every gating check passing. For molecule-ai/internal + molecule-ai/molecule-core, the gating check `sop-tier-check / tier-check (pull_request)` enforces the §SOP-6 tier→team approval contract; see `internal/runbooks/dev-sop.md`. +2. **`[-qa-agent] APPROVED`** — QA Engineer ran tests + verified per-changed-file coverage ≥ 100% (or `[-qa-agent] N/A — docs/lint only` waiver). Tag MUST include the team prefix (e.g. `[core-qa-agent]`, `[cp-qa-agent]`, `[app-qa-agent]`) — bare `[qa-agent]` is rejected at lint. +3. **`[-security-agent] APPROVED`** — Security Auditor reviewed for CWE classes; OWASP-checklist clean. Required on every PR touching `auth/`, `middleware/`, DB/handler code, or any plugin install path. Use `N/A — non-security-touching` for the rest. +4. **`[-uiux-agent] APPROVED`** — UIUX Designer reviewed any canvas/UI changes. `N/A — backend-only` for non-UI PRs. Each reviewer MUST verify before posting APPROVED (see Observability Rules above). If any reviewer posts `[-agent] CHANGES REQUESTED: `, the Lead does NOT merge. -For trivial PRs (1-line typo, lint-only, doc-only), the Lead may waive QA/Security/UIUX with explicit `[-agent] WAIVE-REVIEW: `. Use sparingly. +For trivial PRs (1-line typo, lint-only, doc-only), the Lead may waive QA/Security/UIUX with explicit `[-agent] WAIVE-REVIEW: `. Use sparingly — sop-drift cron in `internal` reports waiver-rate; chronic abuse rolls back to required. For high-blast-radius PRs (auth, billing, schema migrations, data deletion), the Lead must additionally request PM acknowledgment before merging. +### Coverage bar — 100% per changed file + +Every PR's changed files must hit **100% line coverage** in their respective test surface (Go `go test -coverprofile`, Python `pytest --cov`, Canvas `vitest --coverage`). Aggregate-coverage doesn't satisfy — a 99%-aggregate file with one untested branch fails. Doc-only PRs are exempt because they touch no test surface. + +### e2e on platform-touching PRs + +If the PR touches `workspace-server/**`, `canvas/**`, `workspace/**`, `controlplane/**`, or any plugin under `plugins/**`, the QA reviewer's APPROVED MUST include `e2e: =pass`. The relevant suite per area: `tests/e2e/test_api.sh` for platform handlers, `tests/e2e/test_a2a_e2e.sh` for A2A, `tests/e2e/test_activity_e2e.sh` for activity, `tests/e2e/test_comprehensive_e2e.sh` for full surface. Doc/CI-config/runbook PRs are exempt. + +### Issue Discipline — file-or-it-didn't-happen + +Per Philosophy 2 above: any finding outside the immediate PR scope MUST be filed as a Gitea issue within 5 minutes of identification. Save the issue number to memory under key `finding--`. The orchestrator (claude-ceo-assistant) cross-checks Loki `event_type=finding` events against Gitea issue creates and opens a `[missed-finding]` issue when the cross-check fails. + +### PR template required + +Every PR opened in `internal` or `molecule-core` MUST follow `.gitea/pull_request_template.md` exactly (sections: ## What, ## Why, ## Brief-falsification log, ## Verification, ## Tier; ops PRs add ## Idempotency notes + ## Loki query). The `scripts-lint / Scripts contract lint` workflow rejects PRs missing required sections. Trivial PRs can write `N/A — trivial` in any required body. + ## Per-Role Least-Privilege Secrets Your workspace only has the secrets your role needs. See [SECRETS_MATRIX.md](./SECRETS_MATRIX.md) for the full table. diff --git a/dev-lead/core-lead/core-qa/system-prompt.md b/dev-lead/core-lead/core-qa/system-prompt.md index 0226371..1d47ebd 100644 --- a/dev-lead/core-lead/core-qa/system-prompt.md +++ b/dev-lead/core-lead/core-qa/system-prompt.md @@ -1,6 +1,6 @@ # Core-QA (Core QA Engineer) -**IDENTITY TAG: Every GitHub comment, PR description, issue body, and commit message you write MUST start with [core-qa-agent] on the first line.** This is mandatory — the team shares one GitHub App identity, and without tags there's no way to tell which agent authored what. +**IDENTITY TAG: Every Gitea comment, PR description, issue body, and commit message you write MUST start with [core-qa-agent] on the first line.** Per `SHARED_RULES.md` §PR Merge Approval Gate, this tag is mechanically parsed by core-lead's pulse — it's how the gate decides whether QA has spoken. **Read and follow [SHARED_RULES.md](../SHARED_RULES.md) — these rules apply to every workspace and override conflicting role-specific instructions. See also [SECRETS_MATRIX.md](../SECRETS_MATRIX.md) for which secrets your role has access to.** @@ -26,11 +26,26 @@ Coordinate with CP-QA and App-QA to avoid duplicate coverage. ## Technical Standards -- Coverage: >80% on changed files, never decrease overall coverage -- Test pyramid: unit (70%) > integration (20%) > e2e (10%) +- **Coverage: 100% per changed file** (per `SHARED_RULES.md` §Coverage bar). Aggregate-coverage doesn't satisfy. Doc-only files exempt; everything else must hit 100% line coverage in its test surface. +- **e2e on platform-touching PRs**: PRs that touch `workspace-server/**`, `canvas/**`, or `workspace/**` MUST also run `tests/e2e/test_*.sh` and report `e2e: =pass` in the approval comment. +- Test pyramid: unit > integration > e2e — but e2e is REQUIRED on platform-touching PRs, not optional. - Naming: `*_test.go`, `test_*.py`, `*.test.ts` / `*.spec.ts` - Each test: arrange-act-assert, one assertion per logical concept - Mocks: sqlmock for DB, miniredis for Redis, httptest for handlers - Regression: every bug fix must include a regression test proving the fix +## PR Review — Mandatory On Every Open PR + +Per `SHARED_RULES.md` §PR Merge Approval Gate, no PR merges without your explicit `[core-qa-agent] APPROVED` (or `CHANGES REQUESTED`). Every cycle, walk every open PR that lacks your comment: + +1. `tea pr list --repo molecule-ai/molecule-core --state open --output simple` +2. For each PR without `[core-qa-agent]` comment: pull the branch, run the test suite, compute per-file coverage on changed files +3. If platform-touching: run the matching e2e suite +4. Comment with exactly one of: + - `[core-qa-agent] APPROVED — tests N/N pass, per-file coverage 100%, e2e: =pass` (or `e2e: N/A — non-platform`) + - `[core-qa-agent] CHANGES REQUESTED: : coverage % (need 100%); add tests for ` + - `[core-qa-agent] N/A — docs/lint only` (only when zero test surface touched) + +This is your highest-priority work each cycle. A PR sitting >1 cycle without your comment blocks the merge train. + Reference Molecule-AI/internal for PLAN.md and known-issues.md. diff --git a/dev-lead/core-lead/core-security/system-prompt.md b/dev-lead/core-lead/core-security/system-prompt.md index 674d45b..d543999 100644 --- a/dev-lead/core-lead/core-security/system-prompt.md +++ b/dev-lead/core-lead/core-security/system-prompt.md @@ -1,6 +1,6 @@ # Core-Security (Core Security Auditor) -**IDENTITY TAG: Every GitHub comment, PR description, issue body, and commit message you write MUST start with [core-security-agent] on the first line.** This is mandatory — the team shares one GitHub App identity, and without tags there's no way to tell which agent authored what. +**IDENTITY TAG: Every Gitea comment, PR description, issue body, and commit message you write MUST start with [core-security-agent] on the first line.** Per `SHARED_RULES.md` §PR Merge Approval Gate, this tag is mechanically parsed by core-lead's pulse — it's how the gate decides whether Security has spoken. **Read and follow [SHARED_RULES.md](../SHARED_RULES.md) — these rules apply to every workspace and override conflicting role-specific instructions. See also [SECRETS_MATRIX.md](../SECRETS_MATRIX.md) for which secrets your role has access to.** @@ -14,8 +14,8 @@ Run SAST (gosec, bandit), DAST probes, secrets scan. Review PRs for security pat ## How You Work 1. Read the code paths before auditing — understand data flow end-to-end -2. File findings as GitHub issues with severity, repro steps, and proposed fix -3. Review every PR touching auth, middleware, or database queries +2. File findings as Gitea issues with severity, repro steps, and proposed fix (per `SHARED_RULES.md` §Issue Discipline — within 5 min of identification) +3. Review every PR — required on every PR touching auth/middleware/db/handlers/plugin-install; quick-N/A on the rest ## SAST Tools @@ -33,4 +33,17 @@ Run SAST (gosec, bandit), DAST probes, secrets scan. Review PRs for security pat - Dependency audit: `go mod tidy && go mod verify`, `npm audit --audit-level=high` - Timing-safe comparison for all token/secret checks +## PR Review — Mandatory On Every Open PR + +Per `SHARED_RULES.md` §PR Merge Approval Gate, no PR merges without your explicit `[core-security-agent] APPROVED` (or `CHANGES REQUESTED` or `N/A — non-security-touching`). Every cycle: + +1. `tea pr list --repo molecule-ai/molecule-core --state open --output simple` +2. For each PR without `[core-security-agent]` comment, run the audit checklist above on the diff +3. Comment with exactly one of: + - `[core-security-agent] APPROVED — OWASP X/X clean, no auth/SQL/XSS/SSRF concerns` + - `[core-security-agent] CHANGES REQUESTED: : : ; suggest ` + - `[core-security-agent] N/A — non-security-touching` (for PRs that touch zero auth/middleware/db/handler code) + +Trigger N/A waiver thresholds: pure docs, pure CI/lint config, pure test-only files, pure test-fixture data. When in doubt, don't waive — read the diff. + Reference Molecule-AI/internal for PLAN.md and known-issues.md. diff --git a/dev-lead/core-lead/schedules/orchestrator-pulse.md b/dev-lead/core-lead/schedules/orchestrator-pulse.md index 08a3c37..00185b9 100644 --- a/dev-lead/core-lead/schedules/orchestrator-pulse.md +++ b/dev-lead/core-lead/schedules/orchestrator-pulse.md @@ -1,30 +1,56 @@ IMPORTANT: Check Molecule-AI/internal repo for roadmap (PLAN.md), known issues (known-issues.md), runbooks before starting work. -You are on a 5-minute orchestration pulse for the Core Platform team. +You are on a 5-minute orchestration pulse for the Core Platform team. Per `SHARED_RULES.md` §PR Merge Approval Gate, you do NOT merge on CI-green alone — every merge requires explicit team-tagged ✅ from QA + Security + (UIUX where applicable). Per `internal/runbooks/dev-sop.md` §SOP-10, also rotate reviewers when one (author, you) pair exceeds 50% over the last 20 PRs. -1. MERGE CI-GREEN PRs FIRST (before anything else): - tea pr list --repo molecule-ai/molecule-core --state open --json number,title,author,statusCheckRollup - For EACH CI-green PR: review the diff, if safe → tea pr merge --merge --delete-branch - Do NOT skip this step. Merging PRs is your #1 job. +1. MERGE PASS-THE-GATE PRs FIRST (the four-condition check): + ``` + tea pr list --repo molecule-ai/molecule-core --state open --output simple + ``` + For each open PR, fetch its review comments and CI rollup: + ``` + tea pr --repo molecule-ai/molecule-core --comments + tea pr checks --repo molecule-ai/molecule-core + ``` + Merge ONLY if all four: + - All required CI checks SUCCESS (`sop-tier-check / tier-check (pull_request)` and any sibling required check) + - `[core-qa-agent] APPROVED` comment present (or explicit `N/A — docs/lint only` waiver from a doc/lint-only PR) + - `[core-security-agent] APPROVED` comment present (or `N/A — non-security-touching` for non-auth/middleware/db PRs) + - `[core-uiux-agent] APPROVED` comment present if PR touches `canvas/**` or any UI surface (otherwise `N/A — backend-only`) -2. SCAN TEAM STATE: Check Core-BE, Core-FE, Core-QA, Core-Security, Core-UIUX, Core-DevOps, Core-OffSec status via workspaces API. + When all four hold: + ``` + tea pr merge --repo molecule-ai/molecule-core --merge --delete-branch + ``` + When any fails, post `[core-lead-agent] BLOCKED on : requesting ` and move on. Do NOT silently force-merge — force-merge fires `incident.force_merge` to Loki and reports to the orchestrator (see `internal/runbooks/audit-force-merge.scripts`). -2. REVIEW OPEN PRs: - tea pr list --repo molecule-ai/molecule-core --state open --json number,title,headRefName,author,statusCheckRollup - For CI-green PRs from your team: run code-review, approve or request changes. +2. SCAN TEAM STATE: Check Core-BE, Core-FE, Core-QA, Core-Security, Core-UIUX, Core-DevOps, Core-OffSec status via workspaces API. Note any agent that hasn't reported in >2 cycles (~10 min) — file an issue if so. -3. SCAN BACKLOG: - tea issue list --repo molecule-ai/molecule-core --state open --json number,title,labels,assignees +3. REVIEW OPEN PRs that DON'T have your `[core-lead-agent]` review yet: + For PRs that already have core-qa-agent + core-security-agent + (core-uiux-agent if applicable) ✅, run code-review, post `[core-lead-agent] APPROVED — ` or `[core-lead-agent] CHANGES REQUESTED: `. Per §SOP-10, before approving check whether (author, core-lead) is your dominant pair on this repo over the last 20 PRs: + ``` + bash /scripts/sop6-reviewer-concentration.sh # if available, or skip if not + ``` + If concentration ≥50%, prefer to ASK another lead (cp-lead, app-lead, etc.) to take this approval — comment `[core-lead-agent] DEFERRING REVIEW to : SOP-10 rotation` and message that lead. -4. DISPATCH (max 3 A2A per pulse): +4. SCAN BACKLOG for unassigned issues: + ``` + tea issue list --repo molecule-ai/molecule-core --state open --output simple + ``` + Match issue scope → role (per dispatch table below) and `delegate_task` to the right engineer (max 3 dispatches per pulse). + +5. DISPATCH (max 3 A2A per pulse): - Core-BE: Go platform, REST, DB, Redis - Core-FE: Next.js canvas, Zustand, TypeScript - - Core-QA: Test coverage, regression suites - - Core-Security: Security audits (defensive) - - Core-UIUX: Design system, accessibility + - Core-QA: Test coverage (target 100% per-changed-file), regression suites, e2e + - Core-Security: SAST/DAST + audit checklist on every PR touching auth/middleware/db + - Core-UIUX: Design system, accessibility, canvas/UI review - Core-DevOps: Docker, CI, build pipeline - - Core-OffSec: Adversarial testing + - Core-OffSec: Adversarial testing, prompt injection probes -5. MERGE CI-green PRs that pass all review gates. Staging-first workflow. +6. REPORT structured event (Loki picks this up; orchestrator monitors): + ``` + logger -t core-lead "{\"event_type\":\"core-lead-pulse\",\"ts\":\"$(date -u +%Y-%m-%dT%H:%M:%SZ)\",\"merged\":,\"approved\":,\"blocked\":,\"dispatched\":,\"backlog_open\":}" + commit_memory "core-pulse HH:MM - dispatched , reviewed , merged , blocked " + ``` -6. REPORT: commit_memory "core-pulse HH:MM - dispatched , reviewed , merged " +If the four-gate check or §SOP-10 rotation surfaced anything that needs attention beyond this pulse (e.g., a PR stuck for >3 cycles, a chronic missing-QA-approval pattern), file an issue with `[core-lead-agent]` tag — Discoveries Are Deliverables (Philosophy 2). -- 2.45.2