From fde029b9bd89830564a08ac9bc6603428184c835 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 14:42:53 +0900 Subject: [PATCH 1/2] feat(ci): add evidence-gated issue-draft composer; add ADR-0022 The owner asked that Noema and/or the OpenCode Agent also handle PR follow-up work, process review feedback, search the web, find papers, handle issues, and author issues, to the same evidence-based, ADR-traceable standard as docs/product-goal-directive.md. Re-verified the given hard constraint (opencode.jsonc's edit:deny on the pull_request_target-triggered required review gates) is still true against a fresh clone before any design work. Investigation (re-verified, not assumed from a prior read) found: review/rca/conflict autofix already exists and needed no widening; contextual-orchestrator's web_search.py is real but unmerged with zero callers; academic paper search has no code anywhere; noema-core (PR #536, hours old) has no tool/edit/sandbox machinery and is not a viable host yet; and, critically, docs/product-goal-directive.md's standing autonomous-loop authorization is written entirely in PR terms and never mentions issue creation - a genuine authorization gap, not an assumption in either direction. ADR-0022 records the trust-boundary reasoning (the required gate and the autofix worker sit on opposite sides of the same boundary and stay there), the chosen architecture, and the full deferred roadmap (web search wiring, paper search, unattended issue-creation triggering, noema-core adoption), connecting to backlog item 5 and noema's ADR-0012. The one increment shipped for real: scripts/ci/issue_draft_composer.py, a pure evidence-gated composer that rejects a draft with no findings, citations, or traceable source, and only calls `gh api -X POST repos/{repo}/issues` when invoked with --create explicitly. No workflow wires --create into any trigger in this change, sidestepping the authorization gap rather than assuming it away. 100% coverage/docstrings on the new module, matching this repository's existing scripts/ci gate. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 11 + ...llowup-search-and-issue-authoring-scope.md | 238 +++++++++++++++ docs/product-technical-gap-baseline.md | 124 ++++++++ scripts/ci/issue_draft_composer.py | 209 ++++++++++++++ tests/test_issue_draft_composer.py | 270 ++++++++++++++++++ 5 files changed, 852 insertions(+) create mode 100644 docs/adr/0022-agent-pr-followup-search-and-issue-authoring-scope.md create mode 100644 scripts/ci/issue_draft_composer.py create mode 100644 tests/test_issue_draft_composer.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 66145dc939..96615eff0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,17 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- **Add an evidence-gated issue-draft composer (`scripts/ci/issue_draft_composer.py`), the first + increment of ADR-0022's Noema/OpenCode agent PR-follow-up design.** Composes a GitHub issue + title/body from structured evidence (summary, citation-backed findings, source), rejecting a + draft with no findings, citations, or traceable source. Renders the draft by default; only calls + `gh api -X POST repos/{repo}/issues` when invoked with `--create` explicitly. No workflow wires + `--create` into any scheduled/dispatched trigger in this change — `docs/product-goal-directive.md` + authorizes autonomous PR work but never mentions issue creation, so unattended use is deferred + until that directive text is extended. See + [ADR-0022](docs/adr/0022-agent-pr-followup-search-and-issue-authoring-scope.md) and the + 2026-09-02 entry in `docs/product-technical-gap-baseline.md` for the full architecture decision + and deferred roadmap (web search, paper search, unattended issue creation, `noema-core` adoption). - **Consolidate the 18 per-repository hourly review-repair caller workflows into one file.** At the repository owner's request ("이런 Workflow는 단일 파일로 통합하라"), replaced `accounting-information-platform-`, `afipc-`, `bandscope-`, `clearfolio-`, diff --git a/docs/adr/0022-agent-pr-followup-search-and-issue-authoring-scope.md b/docs/adr/0022-agent-pr-followup-search-and-issue-authoring-scope.md new file mode 100644 index 0000000000..f241eaa9c8 --- /dev/null +++ b/docs/adr/0022-agent-pr-followup-search-and-issue-authoring-scope.md @@ -0,0 +1,238 @@ +# ADR-0022: Trust-bounded scope for agent PR follow-up, web/paper search, and issue authoring + +- **Status:** Accepted +- **Date:** 2026-09-02 +- **Decision owners:** ContextualWisdomLab platform maintainers +- **Scope:** ContextualWisdomLab/.github control plane. Informs, but does not bind without its + own review, ContextualWisdomLab/contextual-orchestrator (PR #1009) and ContextualWisdomLab/noema + (ADR-0012 / PR #536). +- **Figma File ID:** N/A. This repository has no customer UI. + +## Context + +The owner asked that Noema and/or the OpenCode Agent also handle PR follow-up work more broadly, +process review feedback, search the web, find papers, handle issues, and author issues on the +owner's behalf — to the same evidence-based, ADR-traceable, root-cause standard as +`docs/product-goal-directive.md`. A hard constraint was given and independently re-verified before +any design work started: the required, `pull_request_target`-triggered review gates +(`opencode-review.yml:11`, `noema-review.yml:11`) must keep `"edit": "deny"` — set globally and on +every agent in `opencode.jsonc:13,32,51,71` — because that job runs the base branch's trusted +scripts against an arbitrary, unauthenticated PR author's diff *as data*, before any human or +independent-agent review has happened. Nothing in this ADR touches that file or that trigger. + +A fresh investigation (fresh clones, `.github@fb847d6a`, `contextual-orchestrator@88390816`, +`noema@6b2b3e90`, re-verified independently a second time before this design) found: + +1. **PR follow-up autofix already exists and is already broader than "merge conflicts only."** + `scripts/ci/pr_review_fix_scheduler.py:58` defines `REPAIR_MODES = frozenset({"review", "rca", + "conflict"})`. `needs_autofix` (`pr_review_fix_scheduler.py:237-250`) dispatches bounded repair + when the current-head OpenCode review is `CHANGES_REQUESTED` and the body is not a blocker + marker; `needs_rca_repair` (`:253-266`) dispatches on failed-check evidence; `needs_conflict_resolution` + (`:304-326`) is the merge-conflict path. All three route through `.github/workflows/pr-review-autofix.yml`, + which triggers on `repository_dispatch` — explicitly *not* `pull_request_target` (comment at line 8) — + checks out the trusted base-branch source (`github.sha`, lines 42-49), and scopes edit access to a + sealed, review-thread-derived path allowlist verified by `pr_review_conflict_scope.py` after the run + (line 460). No widening of dispatch scope is needed today; re-litigating "conflict-only" against this + code would be building on a stale read. +2. **Web search exists as code but is unmerged, uncalled, and denied everywhere by design.** + `contextual_orchestrator/web_search.py` is not on `contextual-orchestrator`'s `main` + (`git merge-base --is-ancestor` is false); it lives on open PR #1009 + (`feat/web-search-mcp-a2a-foundation`), whose own commit message states there is "no concrete + Strix/Noema caller yet." It has zero callers anywhere. Every `opencode.jsonc` in this + organization — the required gate's (`opencode.jsonc:20-21,39-40,58-59,78-79`) and the autofix + worker's generated one (`pr-review-autofix.yml:310-311,338-339`) — sets `"webfetch": "deny"` and + `"websearch": "deny"`, and `"mcp": {}` besides. This is deliberate: the required gate runs against + untrusted fork content, so outbound network access is an SSRF/exfiltration vector regardless of + `edit`. +3. **Academic paper search has no code anywhere in the organization.** No arXiv/Semantic + Scholar/OpenAlex/Zotero client exists in any script. `docs/product-goal-directive.md:36` + (§3) already names Local Zotero as the intended source, conditionally ("Local Zotero API가 + 되면…"), and `docs/doctoring/product-technical-gap-baseline.md:70-71` records a real session that + found it unreachable and fell back to manual APA citation. This is aspirational infrastructure, + not a live gap this ADR can close by itself. +4. **Issue authoring is missing in the three repos this ask names, but the pattern is solved + elsewhere in the organization.** No `gh issue create` exists anywhere in `.github`, + `contextual-orchestrator`, or `noema` — every `issues/{n}/comments` call found + (`pr_review_fix_scheduler.py:159,351`, `agent_mention_sweep.py:271`, and others) is the + PR-comments endpoint, not standalone-issue creation. But `four-pillars`' + `hourly-product-loop.yml` already runs a scheduled, deterministic, idempotent issue + sync (`gh issue create`/`comment`/`close` against one fixed-title issue) with plain + `github.token`, and `mhtml-etl-gateway`'s `opencode.jsonc` already allows an OpenCode agent + `"gh issue create *"`/`"edit *"`/`"comment *"`/`"close *"` on trusted, scheduled triggers, still + with `webfetch`/`websearch` denied. This is a port, not an invention. +5. **`noema-core` (PR #536, opened hours before this design work, `mergeState: BLOCKED`) is not a + viable host for any of this today.** Its entire surface is `build_openai_model()` and + `build_agent()` — two functions wiring `pydantic_ai.Agent(...)`, plus a persona string. Its own + module docstring says it "deliberately owns none of a consumer's domain logic: no verdict + schema, no tool/deps machinery, no credential resolution or validation policy, no tenant + isolation." No tools, no edit access, no sandboxing, no bash. ADR-0012 in that PR explicitly + rejected a shared *service* (its Option B) because two of its three envisioned endpoints have no + real callers yet, and scoped v1 to construction-wiring only. naruon's actual do-anything agent + (`backend/services/noema_agent.py`, main, six `@agent.tool` closures at lines 504-537) is real but + scoped to email/knowledge-graph/task tooling only, per `CWL-MASTER-CONTEXT.md:16`'s platform + boundary, and does not consume `noema-core` yet. +6. **The organization's own "immature core gets completed at its owner, not worked around" rule is + drafted but not binding yet.** The exact sentence — an immature dependency is fixed at its + canonical owner via RED → GREEN → versioned release, never duplicated or worked around in a + consumer, with an ADR as the only escape hatch — is not in `main`'s `docs/product-goal-directive.md` + §2 today; it is live only on open PR `ContextualWisdomLab/.github#1682` (`mergeStateStatus: + BLOCKED`). This ADR follows its intent anyway (it matches the owner's already-stated direction and + this repository's existing `CLAUDE.md` "immature core" guidance), but does not treat #1682 as + merged, binding text. +7. **No existing standing authorization names unattended issue creation.** `docs/product-goal-directive.md` + contains zero occurrences of "이슈"/"issue" anywhere in its 96 lines. Its §2 ("동시 작업·PR + 운영·근본 수정") is written entirely in terms of PRs — Stacking, merge-readiness, Force-Push + avoidance, Check failures. The standing autonomous-loop authorization this organization already + operates under is real and broad for *PR* work; it does not, on its own text, extend to opening + new public-visible Issues unattended. This is a genuine gap, not an assumption in either + direction, and it directly shapes the issue-authoring design below. + +## Trust-boundary decision + +The required gates and the autofix worker sit on opposite sides of the same boundary, and every +decision in this ADR keeps them there: + +- **`opencode-review.yml` / `noema-review.yml`** (`pull_request_target`, `edit: deny`, `mcp: {}`, + `webfetch`/`websearch: deny`): judge *first-look, unauthenticated-author* content. Nothing has + been reviewed yet; the diff is data, never a command; no write, no outbound network call, no merge + decision can originate here. **Unchanged by this ADR, and nothing proposed here ever runs inside + it.** +- **`pr-review-autofix.yml`** (`repository_dispatch` from the trusted scheduler only, base-branch + source, OIDC/App-token credentials distinct from the required job's own token): acts only on a PR + that has *already* received a formal review verdict — `needs_autofix`/`needs_rca_repair` gate on + `has_current_head_changes_requested`, `needs_conflict_resolution` gates on + `has_current_head_approval` unless the scheduled caller explicitly allows unreviewed conflict + repair (and even then, produces a new head that must be fully re-reviewed). Its edit access is a + sealed allowlist derived from the actual review threads, verified post-run. Its output is never + merge evidence on its own: a fresh push resets prior approvals and Checks per the merge + scheduler's exact-head requirement, so a wrong repair cannot merge unreviewed. + +This is why the autofix worker may be trusted with *more* than the required gate ever gets, and why +this ADR's roadmap (web search, paper search) only ever proposes extending *that* worker, never the +required gate: its blast radius is already bounded by (a) a narrow, verified edit-path allowlist, +(b) mandatory fresh re-review of everything it touches, and (c) it only ever runs on content a formal +review has already looked at once. Adding a capability there is extending an already-bounded trust +region, not opening a new one — provided each addition gets its own equivalent bound (§ Deferred +items below states what that bound must be for web/paper search specifically, since neither is +merged yet). + +Issue authoring's draft-first design (below) applies the same boundary at a different layer: because +no existing standing text authorizes *unattended* issue creation, the correct boundary today is +between *composing* evidence-gated content (safe, reversible, no GitHub-visible effect) and *making +it visible* (irreversible-ish, needs an explicit human or explicitly-authorized trigger) — the same +shape as this repository's own `infra/cloudflare/reconcile.sh` convention: dry-run by default, +writes only on an explicit `mode = apply`. + +## Chosen architecture + +1. **Review-finding autofix (existing, re-verified, not extended in this PR).** `review`/`rca`/ + `conflict` modes already cover ordinary review findings, failed-check RCA, and merge conflicts, + with the correct trust model. No code change ships here; re-verifying this against a fresh read + (not the original investigation's cached understanding) is itself part of this ADR's evidence. +2. **Web search (deferred, not implemented in this PR).** `contextual_orchestrator/web_search.py` + must first merge on its own review in `contextual-orchestrator` (PR #1009) as an independent + decision — this ADR does not pre-approve that PR. Once merged, it may be wired *only* into the + `pr-review-autofix.yml` worker's generated `opencode.jsonc` (never the required gate's), scoped to + a narrow verification task (e.g. "does this citation/library-version/API claim in the review + thread hold up"), with an outbound-URL/domain scope check equivalent in spirit to + `pr_review_conflict_scope.py`'s path allowlist — that scope check does not exist yet and is + required before this un-denying is safe, not optional polish. +3. **Academic paper search (deferred, not implemented in this PR).** No code exists anywhere in the + organization. The right shape, when built, is a minimal client against a permissive, free, + ZDR-compatible API (OpenAlex or arXiv, evaluated the way `contextual-orchestrator` already + evaluates provider ZDR posture) offered as a tool to the same trusted autofix worker, with Local + Zotero preferred when reachable per `docs/product-goal-directive.md` §3 and OA/DOI citation as the + documented fallback exactly as practiced today. +4. **Issue authoring — the first increment shipped in this PR.** A pure, evidence-gated draft + composer, `scripts/ci/issue_draft_composer.py`: validates that a proposed issue has a non-empty + summary, at least one citation-backed finding, and a traceable source, renders it to Markdown, and + *only* calls `gh api -X POST repos/{repo}/issues` when a caller explicitly passes `--create`. No + workflow in this PR triggers `--create` on a schedule or on any automated event — the tool exists, + is tested, and is invokable, but nothing currently invokes it unattended. This sidesteps the + authorization gap found in Context item 7 rather than assuming it away in either direction: a + human (or an agent working interactively, as this session is) can use `--create` today; wiring it + into a scheduled/dispatched trigger is deferred until `docs/product-goal-directive.md` names + issue-creation authorization explicitly (mirroring how PR #1682 is the tracked, not-yet-merged + place that kind of directive-text change belongs). +5. **Host for all of the above: `.github`/`contextual-orchestrator` today, not `noema-core`.** + Per Context item 5, `noema-core` cannot host tool-calling, edit, or sandboxed work yet — it is + three-days-old construction-wiring DRY-ing with no tool machinery at all. Building any of this + *inside* `noema-core` now, or working around its immaturity by duplicating logic in a product + repo, would be exactly the pattern PR #1682's pending directive text (and this repository's + existing `CLAUDE.md` "immature core" guidance) says not to do. The composer shipped here is + deliberately structured to make a future move cheap rather than to pre-empt the decision: its + validation and Markdown-rendering functions (`load_draft`, `render_markdown_body`) are pure, + side-effect-free, and stdlib-only, so they can become a `noema-core`/naruon `@agent.tool` closure + later with no restructuring — only the CLI/`gh`-invocation wrapper (`create_issue`, `main`) is + `.github`-specific and would stay behind. + +## Connection to backlog item 5 and ADR-0012 + +`CWL-MASTER-CONTEXT.md:36` already names the target shape: "noema — agent runtime … a GitHub Review +Agent in CI + a do-anything agent inside naruon." Backlog item 5 (Noema as a reusable DDD agent for +naruon, not just a CI reviewer) and `noema`'s own ADR-0012 (unify on a shared `noema-core` package) +are the same convergence this ADR defers into, not a separate track: once `noema-core` grows tool/ +edit/sandbox machinery (ADR-0012's own stated next step, not yet scoped there), the review-finding +autofix worker's OpenCode CLI invocation, naruon's six-tool agent, and this ADR's issue-draft/ +web-search/paper-search capabilities become the same kind of `@agent.tool` surface on the same +runtime instead of three independently-maintained integrations. This ADR does not implement that +convergence — `noema-core` is not ready, and forcing it in now would itself violate the reuse-boundary +rule this ADR otherwise follows — but the composer's pure-function design keeps that later move +a lift, not a rewrite. + +## Deferred items (roadmap) + +Recorded in full, with owning repository, in `docs/product-technical-gap-baseline.md`'s +2026-09-02 entry so a later cycle does not need to re-investigate from scratch: + +1. Merge `contextual-orchestrator#1009` (web search) on its own review; only then wire it into + `pr-review-autofix.yml`'s generated config with a new outbound-scope check. Owner: + `contextual-orchestrator`, then `.github`. +2. Build a minimal, ZDR-evaluated academic paper search client and offer it to the same worker; + prefer Local Zotero when reachable per the standing directive. Owner: `contextual-orchestrator` + or `noema-core` once tool-capable, then `.github`. +3. Wire `issue_draft_composer.py --create` into a trusted, scoped trigger (e.g. a + `repository_dispatch` sibling to the autofix worker, gated the same way) once + `docs/product-goal-directive.md` explicitly authorizes unattended issue creation. Owner: + `.github`, blocked on a directive-text decision, not on code. +4. Once `noema-core` grows tool/edit/sandbox machinery (its own future ADR, not this one), migrate + `load_draft`/`render_markdown_body` there as an `@agent.tool`, alongside naruon's existing six + tools and any web/paper-search tools from items 1-2, per ADR-0012's shared-runtime direction. + Owner: `noema`. +5. Re-adopt this ADR's reasoning once `.github#1682` merges the explicit "immature core" directive + text, to confirm nothing here drifted from the final wording. Owner: `.github`. + +## Alternatives rejected + +- **Widen the required gate's `opencode.jsonc` (`edit`, `webfetch`, `websearch`) directly.** + Rejected outright — this is the constraint the owner gave and independently re-verified as still + true; it judges unauthenticated-author content and must stay fail-closed. +- **Build issue-authoring as a new invention instead of porting the `four-pillars`/ + `mhtml-etl-gateway` pattern.** Rejected: those two repositories already carry a working, scheduled, + trusted, idempotent version of exactly this; porting is lower-risk than a fresh design and matches + this organization's stated preference for reusing solved patterns over inventing new ones. +- **Ship `issue_draft_composer.py` wired to auto-create on a schedule now, treating the standing + PR-loop authorization as implicitly covering issues too.** Rejected per Context item 7: the + directive text is PR-specific by its own words, and assuming coverage either way was explicitly + out of scope for this design. Flagging the gap and shipping the safe (draft-only) half is the + responsible middle path. +- **Force the new capabilities into `noema-core` now to avoid a second migration later.** Rejected: + `noema-core` has no tool/edit/sandbox surface to build against yet (Context item 5); building + against a package that owns none of the needed machinery would mean effectively vendoring that + machinery into `.github` anyway, which is the exact "consumer works around an immature core" + pattern the org's own pending rule (and existing `CLAUDE.md` guidance) rejects. +- **Treat PR #1682's pending directive text as already binding.** Rejected: it is open and + `BLOCKED`. This ADR follows its intent because it matches the owner's already-stated direction, but + documents the distinction rather than treating an unmerged PR as governance. + +## Validation + +`scripts/ci/issue_draft_composer.py` ships with unit tests covering: evidence validation (missing/ +empty summary, title, source, findings, malformed labels, oversized title, malformed repo), Markdown +rendering (summary/evidence/source/attribution sections all present, citations attached to their +findings), the CLI's draft-only default (no `gh` invocation, evidence-gate errors surfaced as a +non-zero exit with a clear message), and the `--create` path (exact `gh api` argv, including +repeated `labels[]` fields) via a monkeypatched `run()` — the same seam +`pr_review_fix_scheduler.py`'s own tests use for `gh` calls. `coverage run -m pytest tests` and +`interrogate` must both stay at 100% on `scripts/ci`, matching every other module in this +repository; no exception is requested for this file. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 29acdfeecc..0f1d81e237 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2613,3 +2613,127 @@ Higgins, S. S., Crepalde, N., & Fernandes, L. (2021). Segmented multiplexity: A **Expected effect.** No observable change to any current GitHub Actions review run (every current invocation already resolves to `free`). The effect is structural: it is no longer possible for a future workflow edit or manual dispatch override to admit priced-model spend into a required review check without an explicit, reviewed code change to this one `case` statement (and its now-locked-in regression test) first. **Follow-up.** If the organization later solves free+ZDR routing robustly enough to deliberately widen required-review CI to `orchestrator/auto` (e.g. once a spend ceiling and reviewer-visible cost evidence exist for that path), the change is exactly one `case` arm plus the corresponding assertions in `test_sidecar_pins_the_pool_to_free_for_github_actions` — this entry is the record of *why* it was narrowed, not a permanent prohibition. + +## 2026-09-02 Noema/OpenCode agent PR follow-up scope: design, ADR-0022, and the issue-draft-composer first increment + +**Ask.** The owner asked that Noema and/or the OpenCode Agent also handle PR follow-up work more +broadly, process review feedback, search the web, find papers, handle issues, and author issues on +the owner's behalf, to the standard this document and `docs/product-goal-directive.md` already +hold PR work to — evidence-based, ADR-traceable, root-cause fixes. The hard constraint given +alongside the ask — the required, `pull_request_target`-triggered review gates +(`opencode-review.yml`, `noema-review.yml`) must keep `"edit": "deny"` — was independently +re-verified true a second time before any design work started (`opencode.jsonc:13,32,51,71`, +`opencode-review.yml:11`, `noema-review.yml:11`), against a fresh clone at `.github@fb847d6a`, +`contextual-orchestrator@88390816`, `noema@6b2b3e90`. Nothing in this design touches that file or +that trigger. + +**Investigation summary (re-verified, not assumed from a prior read).** + +1. `scripts/ci/pr_review_fix_scheduler.py:58`'s `REPAIR_MODES = frozenset({"review", "rca", + "conflict"})` already dispatches bounded, trust-safe repair for ordinary review findings + (`needs_autofix`, `:237-250`), failed-check RCA (`needs_rca_repair`, `:253-266`), and merge + conflicts (`needs_conflict_resolution`, `:304-326`) — all through `pr-review-autofix.yml`, which + triggers on `repository_dispatch` (never `pull_request_target`), runs the trusted base-branch + source, and scopes edit access to a review-thread-derived path allowlist verified post-run by + `pr_review_conflict_scope.py`. **This is already broader than "merge conflicts only" and needed + no widening in this pass** — re-litigating that against a stale read would have been the wrong + move. +2. `contextual_orchestrator/web_search.py` exists but is unmerged (open PR + `contextual-orchestrator#1009`, zero callers anywhere, its own commit message says there is "no + concrete Strix/Noema caller yet"), and every `opencode.jsonc` in the organization — + the required gate's and the autofix worker's generated one alike — sets `webfetch`/`websearch: + deny` and `mcp: {}` by design, since the required gate judges untrusted fork content. +3. No academic paper search client exists anywhere in the organization. Local Zotero is named as + the intended source in `docs/product-goal-directive.md` §3 but is conditional + ("Local Zotero API가 되면…") and was found unreachable in a prior session + (this document, 2026-08-xx entries above). Current practice is manual APA citation. +4. No `gh issue create` exists in `.github`, `contextual-orchestrator`, or `noema` — every + `issues/{n}/comments` call found is the PR-comments endpoint. The pattern is already solved + elsewhere in the organization: `four-pillars/hourly-product-loop.yml` runs a scheduled, + idempotent `gh issue create`/`comment`/`close` sync against one fixed-title issue with plain + `github.token`, and `mhtml-etl-gateway`'s `opencode.jsonc` allows an OpenCode agent + `gh issue create/edit/comment/close *` on trusted, scheduled triggers, `webfetch`/`websearch` + still denied. +5. `noema`'s `noema-core` (PR #536, opened hours before this design session, + `mergeState: BLOCKED`) is not a viable host today: its entire surface is two model-construction + functions and a persona string, with — by its own module docstring — "no tool/deps machinery, no + credential resolution or validation policy, no tenant isolation." naruon's real six-tool + do-anything agent (`backend/services/noema_agent.py`) is real but scoped to email/KG/task tooling + only and does not consume `noema-core`. +6. **A genuine authorization gap, found by re-reading the standing directive rather than assuming + either way**: `docs/product-goal-directive.md` (96 lines) contains zero occurrences of + "이슈"/"issue". Its §2 standing autonomous-loop authorization — the text this organization's + continuous PR review→fix→merge→develop loop already operates under — is written entirely in + PR terms (Stacking, merge-readiness, Force-Push avoidance, Check failures). It does not, on its + own words, extend to opening new public-visible Issues unattended. This shaped the first + increment's design directly (see below). +7. The organization's own "immature core gets completed at its owner, not worked around" rule + (RED → GREEN → versioned release at the canonical owner, never duplicated or worked around in a + consumer) is drafted on open PR `.github#1682` (`mergeStateStatus: BLOCKED`) but is **not yet + merged, binding text on `main`**. This design follows its intent anyway — it matches the owner's + already-stated direction and this repository's existing `CLAUDE.md` guidance — but does not treat + #1682 as governance. + +**Architecture decision.** Full reasoning, trust-boundary argument, and roadmap are in +[ADR-0022](adr/0022-agent-pr-followup-search-and-issue-authoring-scope.md). Summary: the required +gate and the autofix worker sit on opposite sides of the same trust boundary and stay there — the +required gate judges first-look unauthenticated content and is untouched; the autofix worker acts +only on already-reviewed content with a bounded, verified edit scope and mandatory fresh re-review +of its output, which is why it is the only place future web/paper-search wiring may ever land, and +only once each capability's own scope check exists. `noema-core` is not chosen as a host for +anything in this pass — it has no tool/edit/sandbox machinery yet — but the first increment below is +deliberately structured (pure, side-effect-free evidence validation and rendering) so a future move +there, once ADR-0012 grows tool machinery, is a lift, not a rewrite. This connects directly to +backlog item 5 (Noema as a reusable DDD agent for naruon, not just a CI reviewer) and `noema`'s own +ADR-0012 (unify on a shared `noema-core` package): the autofix worker's OpenCode CLI, naruon's +six-tool agent, and this design's issue-draft/web-search/paper-search capabilities are the same +convergence, deferred until `noema-core` is ready to host it. + +**First increment shipped for real.** `scripts/ci/issue_draft_composer.py` + +`tests/test_issue_draft_composer.py`: a pure, evidence-gated issue-draft composer. `load_draft` +rejects a payload with no findings, no citations, or no traceable source +(`IssueDraftError`) rather than silently accepting one; `render_markdown_body`/`render_draft_text` +render the human-reviewable Markdown; `create_issue` calls `gh api -X POST repos/{repo}/issues` +**only** when the CLI is invoked with `--create` — the default (no flag) prints the draft and makes +no GitHub call. No workflow in this PR wires `--create` into any scheduled or dispatched trigger: +per gap 6 above, this sidesteps the authorization question rather than assuming it away in either +direction. A human, or an agent working interactively, can use `--create` today; wiring it into an +unattended trigger is deferred until `docs/product-goal-directive.md` names issue-creation +authorization explicitly (tracked in ADR-0022's deferred items). + +**Developer experience.** `coverage run -m pytest tests/test_issue_draft_composer.py` and +`interrogate scripts/ci/issue_draft_composer.py` both report 100% (28 tests: evidence-gate +rejection for every required field, malformed repo/oversized title/malformed labels, Markdown +rendering content, the `--create`-gated `gh api` argv including repeated `labels[]` fields via a +monkeypatched `run()` — the same seam `pr_review_fix_scheduler.py`'s own tests use — and the CLI's +draft-only default, error paths, and `__main__` guard via `runpy`). + +**User experience.** Nothing changes for a PR author or reviewer today: no new workflow trigger +exists, so no issue can appear on any repository as a side effect of this PR. The capability is +available to whoever runs the CLI by hand with `--create`, exactly as any other `gh` command already +is, with the same evidence-gate refusing to compose an unsupported draft in the first place. + +**Verified before touching anything.** Re-cloned and re-fetched `.github`, `contextual-orchestrator`, +and `noema` fresh rather than reusing a prior investigation's cached understanding; re-checked +`opencode.jsonc`'s `edit: deny` lines, the two required workflows' `pull_request_target` triggers, +`pr_review_fix_scheduler.py`'s three repair modes, `contextual-orchestrator#1009`'s merge state and +caller count, `noema#536`'s diff size and creation timestamp, `.github#1682`'s merge state, and — +critically — the live ADR directory listing on current `main` before picking ADR-0022's number +(a stale worktree read had shown only through `0020`; the fresh clone showed `0021` already taken, +which is exactly the kind of stale-read mistake this organization has been burned by before). + +**Deferred items (full roadmap, so a later cycle does not re-investigate from scratch).** +1. Merge `contextual-orchestrator#1009` on its own review; only then wire `web_search()` into + `pr-review-autofix.yml`'s generated config with a new outbound-scope check (does not exist yet). + Owner: `contextual-orchestrator`, then `.github`. +2. Build a minimal, ZDR-evaluated academic paper search client (OpenAlex/arXiv) for the same + worker; prefer Local Zotero when reachable. Owner: `contextual-orchestrator` or `noema-core` once + tool-capable, then `.github`. +3. Wire `issue_draft_composer.py --create` into a trusted, scoped trigger once + `docs/product-goal-directive.md` explicitly authorizes unattended issue creation. Owner: + `.github`, blocked on a directive-text decision, not on code. +4. Migrate `load_draft`/`render_markdown_body` into `noema-core` as an `@agent.tool` once it grows + tool/edit/sandbox machinery, alongside naruon's existing six tools and any web/paper-search tools + from items 1-2. Owner: `noema`. +5. Re-adopt ADR-0022's reasoning once `.github#1682` merges the explicit "immature core" directive + text, to confirm nothing drifted from the final wording. Owner: `.github`. diff --git a/scripts/ci/issue_draft_composer.py b/scripts/ci/issue_draft_composer.py new file mode 100644 index 0000000000..6bfb5480b9 --- /dev/null +++ b/scripts/ci/issue_draft_composer.py @@ -0,0 +1,209 @@ +#!/usr/bin/env python3 +"""Compose evidence-gated GitHub issue drafts; create one only when explicitly told to. + +This module is the first increment of ADR-0022 +(``docs/adr/0022-agent-pr-followup-search-and-issue-authoring-scope.md``): issue authoring is more +consequential than PR comments or review-finding repair because it creates new public-visible +content, and ``docs/product-goal-directive.md`` — this organization's standing autonomous-loop +authorization — names PR handling explicitly but never mentions issue creation. Until that +directive text is extended (see ``.github#1682``), no workflow in this repository invokes this +module's ``--create`` path unattended. + +Two safety properties hold regardless of caller: + +1. ``load_draft``/``render_markdown_body`` are pure and never touch the network. A draft with no + findings, no citations, or no traceable source is rejected outright (``IssueDraftError``), + matching this organization's "no heuristics without justification" evidence standard. +2. ``main`` only calls ``gh api`` when the caller passes ``--create`` explicitly. The default mode + (no flag) renders the draft to stdout and returns without any GitHub side effect, mirroring + ``infra/cloudflare/reconcile.sh``'s dry-run-by-default convention for consequential writes. +""" + +from __future__ import annotations + +import argparse +import dataclasses +import json +import re +import sys +from pathlib import Path +from typing import Any, Sequence + +try: + from pr_review_merge_scheduler import run +except ModuleNotFoundError: # pragma: no cover - import shape depends on caller cwd + from scripts.ci.pr_review_merge_scheduler import run + + +REPOSITORY_RE = re.compile(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") +LABEL_RE = re.compile(r"^[A-Za-z0-9_.: -]{1,50}$") +MAX_TITLE_LENGTH = 256 +GOVERNING_ADR = "docs/adr/0022-agent-pr-followup-search-and-issue-authoring-scope.md" +ATTRIBUTION_FOOTER = ( + "---\n" + "Drafted by an evidence-gated composer (`scripts/ci/issue_draft_composer.py`); not opened " + "automatically by any scheduled or dispatched workflow. A human (or an agent working " + f"interactively) ran this tool with `--create` explicitly. See {GOVERNING_ADR}." +) + + +class IssueDraftError(ValueError): + """Raised when evidence for a drafted issue is missing or malformed.""" + + +@dataclasses.dataclass(frozen=True) +class Finding: + """One evidence-backed observation supporting the drafted issue.""" + + description: str + citation: str + + +@dataclasses.dataclass(frozen=True) +class IssueDraft: + """Structured, evidence-gated input for one drafted GitHub issue.""" + + repo: str + title: str + summary: str + findings: tuple[Finding, ...] + source: str + labels: tuple[str, ...] = () + + +def _non_empty_str(value: Any, *, field: str) -> str: + """Return ``value`` as a non-empty stripped string or raise ``IssueDraftError``.""" + if type(value) is not str or not value.strip(): + raise IssueDraftError(f"{field} must be a non-empty string") + return value.strip() + + +def load_draft(payload: dict[str, Any]) -> IssueDraft: + """Validate a raw evidence payload and return a structured, evidence-gated ``IssueDraft``. + + Rejects a draft with no findings, no citations, or no traceable source rather than silently + accepting one, so a caller cannot compose (and, with ``--create``, publish) an unsupported + issue by omission. + """ + if type(payload) is not dict: + raise IssueDraftError("evidence payload must be a JSON object") + + repo = _non_empty_str(payload.get("repo"), field="repo") + if not REPOSITORY_RE.fullmatch(repo): + raise IssueDraftError(f"repo must look like owner/repo, got {repo!r}") + + title = _non_empty_str(payload.get("title"), field="title") + if len(title) > MAX_TITLE_LENGTH: + raise IssueDraftError(f"title exceeds {MAX_TITLE_LENGTH} characters") + + summary = _non_empty_str(payload.get("summary"), field="summary") + source = _non_empty_str(payload.get("source"), field="source") + + raw_findings = payload.get("findings") + if type(raw_findings) is not list or not raw_findings: + raise IssueDraftError("findings must be a non-empty array") + findings: list[Finding] = [] + for index, raw in enumerate(raw_findings): + if type(raw) is not dict: + raise IssueDraftError(f"findings[{index}] must be an object") + description = _non_empty_str(raw.get("description"), field=f"findings[{index}].description") + citation = _non_empty_str(raw.get("citation"), field=f"findings[{index}].citation") + findings.append(Finding(description=description, citation=citation)) + + raw_labels = payload.get("labels", []) + if type(raw_labels) is not list: + raise IssueDraftError("labels must be an array") + labels: list[str] = [] + for index, raw in enumerate(raw_labels): + if type(raw) is not str or not LABEL_RE.fullmatch(raw): + raise IssueDraftError(f"labels[{index}] is invalid") + labels.append(raw) + + return IssueDraft( + repo=repo, + title=title, + summary=summary, + findings=tuple(findings), + source=source, + labels=tuple(labels), + ) + + +def render_markdown_body(draft: IssueDraft) -> str: + """Render the drafted issue body as evidence-gated Markdown.""" + lines = ["## Summary", "", draft.summary, "", "## Evidence", ""] + for finding in draft.findings: + lines.append(f"- {finding.description} ({finding.citation})") + lines.extend(["", "## Source", "", draft.source, "", ATTRIBUTION_FOOTER]) + return "\n".join(lines) + + +def render_draft_text(draft: IssueDraft) -> str: + """Render the full human-reviewable draft: repo, title, labels, and body.""" + header = [f"Repo: {draft.repo}", f"Title: {draft.title}"] + if draft.labels: + header.append(f"Labels: {', '.join(draft.labels)}") + return "\n".join(header) + "\n\n" + render_markdown_body(draft) + + +def create_issue(draft: IssueDraft) -> str: + """Create the drafted issue via the GitHub REST API and return its URL. + + Only reached when a caller explicitly passes ``--create``; see the module docstring and + ``ATTRIBUTION_FOOTER`` for the authorization boundary this preserves. + """ + args = [ + "gh", + "api", + "-X", + "POST", + f"repos/{draft.repo}/issues", + "-f", + f"title={draft.title}", + "-f", + f"body={render_markdown_body(draft)}", + ] + for label in draft.labels: + args.extend(["-f", f"labels[]={label}"]) + result = json.loads(run(args)) + return str(result.get("html_url") or "") + + +def build_parser() -> argparse.ArgumentParser: + """Build the CLI argument parser.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--evidence-file", + type=Path, + required=True, + help="Path to a JSON evidence payload (repo, title, summary, findings, source, labels).", + ) + parser.add_argument( + "--create", + action="store_true", + help="Actually create the issue via `gh api`. Default: render the draft only, no GitHub call.", + ) + return parser + + +def main(argv: Sequence[str] | None = None) -> int: + """CLI entrypoint: draft-only by default, create only with ``--create``.""" + parser = build_parser() + args = parser.parse_args(argv) + try: + payload = json.loads(args.evidence_file.read_text(encoding="utf-8")) + draft = load_draft(payload) + except (OSError, json.JSONDecodeError, IssueDraftError) as exc: + print(f"issue_draft_composer: {exc}", file=sys.stderr) + return 1 + + if not args.create: + print(render_draft_text(draft)) + return 0 + + print(create_issue(draft)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_issue_draft_composer.py b/tests/test_issue_draft_composer.py new file mode 100644 index 0000000000..e4f17a99cb --- /dev/null +++ b/tests/test_issue_draft_composer.py @@ -0,0 +1,270 @@ +"""Tests for scripts/ci/issue_draft_composer.py (ADR-0022's draft-first issue composer).""" + +from __future__ import annotations + +import json +import runpy +import sys + +import pytest + +from scripts.ci import issue_draft_composer as composer + + +def valid_payload(**overrides): + """Return a minimal, valid evidence payload, with optional field overrides.""" + payload = { + "repo": "ContextualWisdomLab/.github", + "title": "Example drafted issue", + "summary": "A short summary of the observed gap.", + "findings": [ + {"description": "Something was observed.", "citation": "scripts/ci/example.py:10"}, + ], + "source": "docs/product-technical-gap-baseline.md#2026-09-02", + "labels": ["gap-baseline"], + } + payload.update(overrides) + return payload + + +# --- load_draft: evidence-gate validation --------------------------------- + + +def test_load_draft_accepts_a_complete_payload(): + """A fully specified payload becomes a structured IssueDraft.""" + draft = composer.load_draft(valid_payload()) + assert draft.repo == "ContextualWisdomLab/.github" + assert draft.title == "Example drafted issue" + assert draft.findings == ( + composer.Finding(description="Something was observed.", citation="scripts/ci/example.py:10"), + ) + assert draft.labels == ("gap-baseline",) + + +def test_load_draft_defaults_labels_to_empty_tuple_when_omitted(): + """labels is optional; omitting it composes a label-less draft.""" + payload = valid_payload() + del payload["labels"] + draft = composer.load_draft(payload) + assert draft.labels == () + + +def test_load_draft_rejects_non_object_payload(): + """A top-level non-dict payload is rejected.""" + with pytest.raises(composer.IssueDraftError, match="JSON object"): + composer.load_draft([]) # type: ignore[arg-type] + + +@pytest.mark.parametrize("field", ["repo", "title", "summary", "source"]) +def test_load_draft_rejects_missing_or_blank_required_strings(field): + """Each required string field must be present and non-blank.""" + for bad_value in (None, "", " ", 42): + payload = valid_payload(**{field: bad_value}) + with pytest.raises(composer.IssueDraftError, match=field): + composer.load_draft(payload) + + +def test_load_draft_rejects_malformed_repo(): + """repo must look like owner/repo, not a bare name or URL.""" + with pytest.raises(composer.IssueDraftError, match="owner/repo"): + composer.load_draft(valid_payload(repo="not-a-repo-slug")) + + +def test_load_draft_rejects_oversized_title(): + """title beyond GitHub's 256-character limit is rejected.""" + with pytest.raises(composer.IssueDraftError, match="256"): + composer.load_draft(valid_payload(title="x" * 257)) + + +def test_load_draft_rejects_non_list_findings(): + """findings must be an array, not a scalar or object.""" + with pytest.raises(composer.IssueDraftError, match="findings"): + composer.load_draft(valid_payload(findings={"not": "a list"})) + + +def test_load_draft_rejects_empty_findings(): + """An issue draft with zero findings has no supporting evidence.""" + with pytest.raises(composer.IssueDraftError, match="non-empty"): + composer.load_draft(valid_payload(findings=[])) + + +def test_load_draft_rejects_non_object_finding_entry(): + """Each findings[] entry must itself be an object.""" + with pytest.raises(composer.IssueDraftError, match=r"findings\[0\]"): + composer.load_draft(valid_payload(findings=["not an object"])) + + +@pytest.mark.parametrize("missing_field", ["description", "citation"]) +def test_load_draft_rejects_finding_missing_description_or_citation(missing_field): + """A finding lacking either half of its evidence pair is rejected.""" + finding = {"description": "d", "citation": "c"} + del finding[missing_field] + with pytest.raises(composer.IssueDraftError, match=missing_field): + composer.load_draft(valid_payload(findings=[finding])) + + +def test_load_draft_rejects_non_list_labels(): + """labels must be an array when present.""" + with pytest.raises(composer.IssueDraftError, match="labels"): + composer.load_draft(valid_payload(labels="not-a-list")) + + +def test_load_draft_rejects_invalid_label_entry(): + """Each label must be a short, safely-charactered string.""" + with pytest.raises(composer.IssueDraftError, match=r"labels\[1\]"): + composer.load_draft(valid_payload(labels=["ok", 42])) + with pytest.raises(composer.IssueDraftError, match=r"labels\[0\]"): + composer.load_draft(valid_payload(labels=["x" * 51])) + + +# --- rendering -------------------------------------------------------------- + + +def test_render_markdown_body_includes_summary_evidence_source_and_attribution(): + """The rendered body carries every evidence-gated section and its citation.""" + draft = composer.load_draft(valid_payload()) + body = composer.render_markdown_body(draft) + assert "## Summary" in body + assert draft.summary in body + assert "## Evidence" in body + assert "Something was observed. (scripts/ci/example.py:10)" in body + assert "## Source" in body + assert draft.source in body + assert composer.GOVERNING_ADR in body + assert "--create" in body + + +def test_render_draft_text_includes_header_with_labels(): + """The human-reviewable draft text names the target repo, title, and labels.""" + draft = composer.load_draft(valid_payload()) + text = composer.render_draft_text(draft) + assert text.startswith("Repo: ContextualWisdomLab/.github\nTitle: Example drafted issue\nLabels: gap-baseline") + assert "## Summary" in text + + +def test_render_draft_text_omits_labels_line_when_none_given(): + """No Labels: line is emitted for a label-less draft.""" + payload = valid_payload() + del payload["labels"] + draft = composer.load_draft(payload) + text = composer.render_draft_text(draft) + assert "Labels:" not in text + + +# --- create_issue ------------------------------------------------------------- + + +def test_create_issue_posts_expected_argv_and_returns_url(monkeypatch): + """create_issue calls gh api with title/body/labels and returns the created URL.""" + captured = {} + + def fake_run(args): + captured["args"] = args + return json.dumps({"html_url": "https://github.com/ContextualWisdomLab/.github/issues/9001"}) + + monkeypatch.setattr(composer, "run", fake_run) + draft = composer.load_draft(valid_payload(labels=["a", "b"])) + url = composer.create_issue(draft) + + assert url == "https://github.com/ContextualWisdomLab/.github/issues/9001" + args = captured["args"] + assert args[:5] == ["gh", "api", "-X", "POST", "repos/ContextualWisdomLab/.github/issues"] + assert "title=Example drafted issue" in args + assert any(a.startswith("body=") and "## Summary" in a for a in args) + assert "labels[]=a" in args + assert "labels[]=b" in args + + +def test_create_issue_omits_label_flags_when_no_labels(monkeypatch): + """No labels[] flags are emitted for a label-less draft.""" + monkeypatch.setattr(composer, "run", lambda args: json.dumps({"html_url": "u"})) + payload = valid_payload() + del payload["labels"] + draft = composer.load_draft(payload) + composer.create_issue(draft) + + +def test_create_issue_returns_empty_string_when_response_lacks_html_url(monkeypatch): + """A malformed gh api response degrades to an empty URL rather than raising.""" + monkeypatch.setattr(composer, "run", lambda args: json.dumps({})) + draft = composer.load_draft(valid_payload()) + assert composer.create_issue(draft) == "" + + +# --- CLI ---------------------------------------------------------------------- + + +def test_main_draft_mode_prints_and_never_calls_run(monkeypatch, tmp_path, capsys): + """Without --create, main renders the draft and makes no gh api call.""" + def fail_run(args): + raise AssertionError(f"run() must not be called in draft mode, got {args}") + + monkeypatch.setattr(composer, "run", fail_run) + evidence_file = tmp_path / "evidence.json" + evidence_file.write_text(json.dumps(valid_payload()), encoding="utf-8") + + exit_code = composer.main(["--evidence-file", str(evidence_file)]) + + assert exit_code == 0 + out = capsys.readouterr().out + assert "Repo: ContextualWisdomLab/.github" in out + assert "## Summary" in out + + +def test_main_create_mode_calls_run_and_prints_url(monkeypatch, tmp_path, capsys): + """--create renders nothing extra; it prints only the created issue URL.""" + monkeypatch.setattr( + composer, "run", lambda args: json.dumps({"html_url": "https://example.invalid/issues/1"}) + ) + evidence_file = tmp_path / "evidence.json" + evidence_file.write_text(json.dumps(valid_payload()), encoding="utf-8") + + exit_code = composer.main(["--evidence-file", str(evidence_file), "--create"]) + + assert exit_code == 0 + assert capsys.readouterr().out.strip() == "https://example.invalid/issues/1" + + +def test_main_reports_invalid_json_and_exits_nonzero(monkeypatch, tmp_path, capsys): + """Malformed evidence JSON is reported to stderr with exit code 1.""" + evidence_file = tmp_path / "evidence.json" + evidence_file.write_text("{not json", encoding="utf-8") + + exit_code = composer.main(["--evidence-file", str(evidence_file)]) + + assert exit_code == 1 + assert "issue_draft_composer:" in capsys.readouterr().err + + +def test_main_reports_missing_file_and_exits_nonzero(tmp_path, capsys): + """A nonexistent evidence file is reported to stderr with exit code 1.""" + exit_code = composer.main(["--evidence-file", str(tmp_path / "missing.json")]) + + assert exit_code == 1 + assert "issue_draft_composer:" in capsys.readouterr().err + + +def test_main_reports_evidence_gate_failure_and_exits_nonzero(tmp_path, capsys): + """A structurally valid JSON file that fails the evidence gate is reported and exits 1.""" + evidence_file = tmp_path / "evidence.json" + payload = valid_payload(findings=[]) + evidence_file.write_text(json.dumps(payload), encoding="utf-8") + + exit_code = composer.main(["--evidence-file", str(evidence_file)]) + + assert exit_code == 1 + err = capsys.readouterr().err + assert "issue_draft_composer:" in err + assert "non-empty" in err + + +def test_module_main_guard_exits_zero(monkeypatch, tmp_path): + """Running the module as __main__ exits with main()'s return code.""" + evidence_file = tmp_path / "evidence.json" + evidence_file.write_text(json.dumps(valid_payload()), encoding="utf-8") + monkeypatch.setattr( + sys, "argv", ["issue_draft_composer.py", "--evidence-file", str(evidence_file)] + ) + with pytest.raises(SystemExit) as exc: + runpy.run_path("scripts/ci/issue_draft_composer.py", run_name="__main__") + assert exc.value.code == 0 From 57ecdafb2ec3527fccc79277e052bbc25668c423 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 14:52:01 +0900 Subject: [PATCH 2/2] test(ci): make issue_draft_composer's import-fallback coverage order-independent The module's try/except ModuleNotFoundError import (the pattern most cross-referencing scripts/ci modules use to work both package-qualified and as a bare sibling import) only had its except branch exercised by accident of which test file happened to run first in the full suite and mutate sys.path - a stale worktree read had masked this behind a `# pragma: no cover` initially, and removing it exposed a genuine 98% gap (lines 34-35) when the full suite's actual collection order was checked instead of assumed. Added a dedicated test that clears the bare-name sys.modules cache entry and strips scripts/ci from sys.path before re-executing the module fresh under a private name, forcing the except branch deterministically regardless of what else has run. Registers the fresh module in sys.modules before exec so its dataclasses can resolve their (`from __future__ import annotations`-deferred) field types via sys.modules[cls.__module__], which importlib.util.module_from_spec does not do automatically the way a normal import statement does. Verified both orderings directly: standalone (tests/test_issue_draft_composer.py alone) and with tests/test_agent_mention_sweep.py collected first (the file whose sys.path mutation had been masking the gap) - both report 100% coverage on scripts/ci/issue_draft_composer.py now. Co-Authored-By: Claude Sonnet 5 --- ...llowup-search-and-issue-authoring-scope.md | 7 ++++- docs/product-technical-gap-baseline.md | 10 +++++-- scripts/ci/issue_draft_composer.py | 2 +- tests/test_issue_draft_composer.py | 29 +++++++++++++++++++ 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/docs/adr/0022-agent-pr-followup-search-and-issue-authoring-scope.md b/docs/adr/0022-agent-pr-followup-search-and-issue-authoring-scope.md index f241eaa9c8..2cf5e53796 100644 --- a/docs/adr/0022-agent-pr-followup-search-and-issue-authoring-scope.md +++ b/docs/adr/0022-agent-pr-followup-search-and-issue-authoring-scope.md @@ -235,4 +235,9 @@ non-zero exit with a clear message), and the `--create` path (exact `gh api` arg repeated `labels[]` fields) via a monkeypatched `run()` — the same seam `pr_review_fix_scheduler.py`'s own tests use for `gh` calls. `coverage run -m pytest tests` and `interrogate` must both stay at 100% on `scripts/ci`, matching every other module in this -repository; no exception is requested for this file. +repository; no exception is requested for this file. Its module-level `try`/`except +ModuleNotFoundError` import fallback (the same pattern most cross-referencing `scripts/ci` modules +use) is exercised by a dedicated test that clears the bare-name cache entry and strips `scripts/ci` +from `sys.path` before re-executing the module fresh, rather than relying on another test file's +incidental `sys.path` mutation earlier in collection order — the mechanism most sibling modules +currently depend on for that branch's coverage. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 0f1d81e237..5809c86e02 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2702,11 +2702,15 @@ unattended trigger is deferred until `docs/product-goal-directive.md` names issu authorization explicitly (tracked in ADR-0022's deferred items). **Developer experience.** `coverage run -m pytest tests/test_issue_draft_composer.py` and -`interrogate scripts/ci/issue_draft_composer.py` both report 100% (28 tests: evidence-gate +`interrogate scripts/ci/issue_draft_composer.py` both report 100% (29 tests: evidence-gate rejection for every required field, malformed repo/oversized title/malformed labels, Markdown rendering content, the `--create`-gated `gh api` argv including repeated `labels[]` fields via a -monkeypatched `run()` — the same seam `pr_review_fix_scheduler.py`'s own tests use — and the CLI's -draft-only default, error paths, and `__main__` guard via `runpy`). +monkeypatched `run()` — the same seam `pr_review_fix_scheduler.py`'s own tests use — the CLI's +draft-only default, error paths, the `__main__` guard via `runpy`, and a dedicated +`importlib`-driven test that deterministically forces the module's `except ModuleNotFoundError` +package-qualified import fallback rather than relying on another test file's incidental `sys.path` +mutation — the codebase-wide try/except-import pattern this module follows is otherwise only +covered by accident of cross-file test collection order, which this file does not depend on). **User experience.** Nothing changes for a PR author or reviewer today: no new workflow trigger exists, so no issue can appear on any repository as a side effect of this PR. The capability is diff --git a/scripts/ci/issue_draft_composer.py b/scripts/ci/issue_draft_composer.py index 6bfb5480b9..5d7c2124c7 100644 --- a/scripts/ci/issue_draft_composer.py +++ b/scripts/ci/issue_draft_composer.py @@ -31,7 +31,7 @@ try: from pr_review_merge_scheduler import run -except ModuleNotFoundError: # pragma: no cover - import shape depends on caller cwd +except ModuleNotFoundError: from scripts.ci.pr_review_merge_scheduler import run diff --git a/tests/test_issue_draft_composer.py b/tests/test_issue_draft_composer.py index e4f17a99cb..ab7ee0eda9 100644 --- a/tests/test_issue_draft_composer.py +++ b/tests/test_issue_draft_composer.py @@ -2,14 +2,19 @@ from __future__ import annotations +import importlib.util import json import runpy import sys +from pathlib import Path import pytest from scripts.ci import issue_draft_composer as composer +MODULE_PATH = Path(__file__).resolve().parents[1] / "scripts" / "ci" / "issue_draft_composer.py" +SCRIPTS_CI_DIR = str(MODULE_PATH.parent) + def valid_payload(**overrides): """Return a minimal, valid evidence payload, with optional field overrides.""" @@ -258,6 +263,30 @@ def test_main_reports_evidence_gate_failure_and_exits_nonzero(tmp_path, capsys): assert "non-empty" in err +def test_module_falls_back_to_package_qualified_import(monkeypatch): + """When the bare module name can't be resolved, the except branch imports it package-qualified. + + Deterministically reproduces the ModuleNotFoundError path (rather than relying on another test + file's incidental sys.path mutation) by clearing the bare cache entry and stripping scripts/ci + from sys.path before re-executing the module fresh under a private name. + """ + monkeypatch.delitem(sys.modules, "pr_review_merge_scheduler", raising=False) + monkeypatch.setattr(sys, "path", [p for p in sys.path if p != SCRIPTS_CI_DIR]) + + spec = importlib.util.spec_from_file_location( + "issue_draft_composer_fallback_import_check", MODULE_PATH + ) + module = importlib.util.module_from_spec(spec) + # Register before exec: the module's dataclasses resolve their string annotations (from + # __future__ import annotations) via sys.modules[cls.__module__], which only exists for a + # normal `import` statement by default. + monkeypatch.setitem(sys.modules, spec.name, module) + spec.loader.exec_module(module) + + assert module.run is not None + assert module.load_draft(valid_payload()).repo == "ContextualWisdomLab/.github" + + def test_module_main_guard_exits_zero(monkeypatch, tmp_path): """Running the module as __main__ exits with main()'s return code.""" evidence_file = tmp_path / "evidence.json"