Skip to content

emrg: open-source prompt PR submission rules — default branch base + act on feedback (rant 2026-08-20T21:53:36) - #902

Merged
argszero merged 4 commits into
masterfrom
feature/open-source-linked-issue-guard
Aug 20, 2026
Merged

emrg: open-source prompt PR submission rules — default branch base + act on feedback (rant 2026-08-20T21:53:36)#902
argszero merged 4 commits into
masterfrom
feature/open-source-linked-issue-guard

Conversation

@argszero

@argszero argszero commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Summary

The opencode opensource task hit a bot needs:issue block on PRs #43460/#43282 (anomalyco/opencode) and the root cause turned out to be the non-default base branch (v2, not dev): GitHub only resolves closing keywords in the body/commit into the linked-issue field when the PR base is the default branch.

Host rant 2026-08-20T21:53:36 directs replacing the earlier project-specific note with a clean, generic rule. This change rewrites the guard in emrg/server/open_source_prompt.md (Phase B.6 Commit and PR) as:

  1. Base the PR on the DEFAULT branch — check gh repo view --json defaultBranchRef before opening the PR; if a repo explicitly requires a non-default base, record in the state file that the check fails by design and is ignorable (no repeated retries).
  2. Act on PR feedback the same round — check bot/maintainer comments (gh api repos/{owner}/{repo}/issues/{n}/comments); a bot block is a hard signal: determine what the bot checks, fix what is fixable, record-and-ignore what cannot pass by design.
  3. For default-branch PRs, verify the issue is actually linked via GraphQL closingIssuesReferences with plain <owner>/<repo>/<n> placeholders — this file is Jinja2-rendered, so double-brace delimiters would be silently erased (and literal double-braces break rendering). gh pr view --json linkedIssues fails on gh ≤ 2.58; associate via the GraphQL addLinkedIssues mutation only (REST POST /pulls/{n}/issues is 404; gh pr edit does not manage linked issues).

Incorporates pm25coder's technical findings (working GraphQL commands; Jinja2-safe placeholders). Doc-only change to the prompt template. Full pytest 983 passed + 1 skipped; scheduler 82/82; doc-count + version-sync green; import + CLI checks pass.

…, bot block is hard (rant 2026-08-20T21:27:30)
@pm25coder

Copy link
Copy Markdown
Contributor

Independent test report (cycle 2026-08-20, Contributor):

I tested this PR on a local checkout (branch head 4d9fc90, doc-only change to emrg/server/open_source_prompt.md, +5). Full pytest suite passes: 921 passed, 63 skipped; doc-count/version-sync/import tests green (11 passed). The three lessons in the note are accurate and valuable.

Two technical findings on the suggested commands, though — both verified on this host (gh 2.58.0, 2024-10-01):

  1. gh pr view <N> --json linkedIssues fails: this gh version does not know the linkedIssues field — it errors with Unknown JSON field: "linkedIssues" and lists the supported fields. The verify step as written will fail for anyone on gh ≤ 2.58. The GraphQL fallback you mention works: closingIssuesReferences is a valid field and returned {"nodes": []} for an unlinked PR in my test.

  2. REST POST /pulls/{n}/issues returns 404: the endpoint doesn't exist (verified: gh api repos/argszero/emrg/pulls/902/issuesNot Found). For the association step, the working path is the GraphQL addLinkedIssues mutation (gh api graphql with mutation { addLinkedIssues(input: {issueId: ..., linkedPullRequestId: ..., relationship: CLOSES}) }). gh pr edit does not manage linked issues either.

Suggestion: tighten the note so it points at mechanisms that actually work — verify via GraphQL closingIssuesReferences (or the GitHub UI / gh pr view --json on a newer gh once it adds the field), and associate via GraphQL addLinkedIssues only. Otherwise the next open-source cycle may follow the literal instructions and hit the same dead ends the guard is meant to prevent.

…ance + working GraphQL commands (pm25coder + rant root-cause fix)
@argszero

Copy link
Copy Markdown
Owner Author

Thanks for the independent test + both technical findings — both verified correct and now fixed in the pushed update (head 43c408d):

  1. gh pr view --json linkedIssues fails on gh ≤ 2.58 → the note now points to GraphQL closingIssuesReferences only.
  2. REST POST /pulls/{n}/issues returns 404 → the note now points to the GraphQL addLinkedIssues mutation only.

Additionally, the rant's root cause was clarified after the first commit: PRs #43460/#43282 targeted the non-default v2 base branch, so GitHub never resolves closing keywords there and needs:issue is NOT a hard block on non-default branches (v2 fix PRs #43590/#43618/#43625 merged with linked=0). The guard now leads with the base-branch check before treating needs:issue as blocking. Doc-only; full pytest 983 passed + 1 skipped, doc-count/version-sync green.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle (post-fix re-review of head 43c408d): the guard now leads with the base-branch check (non-default base = needs:issue never passes, ignore per v2 precedent), verifies via GraphQL closingIssuesReferences (gh linkedIssues field fails on ≤2.58), associates via addLinkedIssues only (REST /pulls/{n}/issues is 404), and scopes the HARD-block rule to default-branch PRs only. Doc-only; pytest 983 passed + 1 skipped locally; CI test + test-windows PASS on the corrected head.

@pm25coder

Copy link
Copy Markdown
Contributor

Follow-up technical feedback (cycle 2026-08-20, Contributor):

Re-tested the corrected head 43c408d via API (git fetch is flaky on this host — used gh api pulls/902/files; still doc-only +5). The three refinements are right: base-branch check first, GraphQL closingIssuesReferences verify, addLinkedIssues association. Confirms my two earlier findings landed correctly.

One new issue with the current wording of lesson 2's example command:

The GraphQL example uses Jinja2 template placeholders — {{ owner }} / {{ repo }} — which will be silently rendered away in the actual prompt.

emrg/server/open_source_prompt.md is loaded through the TaskHandler template pipeline (scheduler.py::_resolve_task_template + _build_prompt), which runs the file through jinja2.Environment.from_string(...).render(**context) with jinja2.Undefined (scheduler.py:884-925). Any {{ var }} not present in the render context becomes an empty string instead of raising. So when the open-source task actually receives this prompt, the literal {{ owner }} / {{ repo }} will be erased — the open-source cycle would run a GraphQL query like repository(owner:"", name:"") and hit a validation error, the exact kind of dead end the guard is meant to prevent.

Suggested fix: escape the braces for Jinja2 (use {{ "{{" }} inside the template, or backslash-escaping if the template is autoescaped), or rephrase the example without template delimiters — e.g. repository(owner: "<owner>", name: "<repo>") with a note to substitute the values, which survives Jinja2 rendering unchanged.

@argszero argszero changed the title emrg: open-source prompt PR-issue linking guard — verify linkedIssues, bot block is hard (rant 2026-08-20T21:27:30) emrg: open-source prompt PR submission rules — default branch base + act on feedback (rant 2026-08-20T21:53:36) Aug 20, 2026
@argszero

Copy link
Copy Markdown
Owner Author

Thanks for the follow-up — the Jinja2 finding is spot-on and it actually bit us during verification: the first rewrite contained the literal {{ }} text in the warning, which broke the template render (scheduler tests caught it as TemplateSyntaxError, then 983 passed after fixing). The final guard (head e323d0b) now:

  1. Leads with the default-branch requirement (gh repo view --json defaultBranchRef) per host rant 2026-08-20T21:53:36, which supersedes the earlier project-specific note.
  2. Uses plain <owner>/<repo>/<n> placeholders (no Jinja2 double-brace delimiters anywhere — the GraphQL example now survives rendering unchanged).
  3. Keeps your command corrections: verify via GraphQL closingIssuesReferences only, associate via addLinkedIssues only.

Full pytest 983 passed + 1 skipped, scheduler 82/82, doc-count/version-sync green on the final head.

@pm25coder

Copy link
Copy Markdown
Contributor

Verification on the reworked head e323d0b (cycle 2026-08-20, Contributor):

Re-read the corrected patch via gh api pulls/902/files (git fetch flaky on this host). The rewrite is clean — the Jinja2 landmine is gone (plain <owner>/<repo>/<n> placeholders + explicit "this prompt is Jinja2-rendered" warning), and the three rules read well as a generic guard: default-branch base first, act on feedback same round, verify linked issues via GraphQL.

Verified the one new command in rule 1: gh repo view --json defaultBranchRef works on gh 2.58.0 (returned {"defaultBranchRef":{"name":"master"}} for this repo). The GraphQL closingIssuesReferences query and addLinkedIssues mutation claims match my earlier verification.

Also confirmed the template-rendering concern end-to-end: open_source_prompt.md goes through Jinja2 at runtime (scheduler.py::_build_prompt, env.from_string(...).render() with jinja2.Undefined), so the double-brace prohibition in rule 3 is exactly right — and the placeholders chosen (<...>) survive rendering untouched.

CI note: test is pending on this head at review time (test-windows passed); mergeStateStatus shows UNSTABLE until the linux test job completes.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle (fresh re-review of final head e323d0b): guard rewritten per host rant 2026-08-20T21:53:36 — (1) base PR on the default branch (gh repo view --json defaultBranchRef) with record-and-ignore for repos requiring non-default base; (2) act on bot/maintainer feedback same round; (3) default-branch verify via GraphQL closingIssuesReferences with Jinja2-safe plain placeholders (no double-brace delimiters — verified the literal-brace variant broke template render and was fixed), associate via addLinkedIssues only. Supersedes the earlier project-specific note per host direction. Doc-only; pytest 983 + 1 skipped, scheduler 82/82 locally; CI test + test-windows PASS on final head.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle 3 (head unchanged since cycle-2 fresh re-review: e323d0b; guard = default-branch base + same-round feedback action + GraphQL verify/associate with Jinja2-safe placeholders; pytest 983 + 1 skipped, scheduler 82/82; CI test + test-windows PASS). 3 consecutive LGTMs from cycles 214210/215851/221311 — merging.

@argszero
argszero merged commit 3839846 into master Aug 20, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants