feat(create-pr): enforce Watch out and Pending headings in PR body (#207)
* feat(create-pr): enforce Watch out and Pending headings * test(create-pr): cover Watch out and Pending headings * docs(handoff): add handoff and ADR for PR #207 --------- Co-authored-by: opencode-agent <agent@opencode.local>
This commit is contained in:
parent
85d7d62fcd
commit
0c1e3e7d39
5 changed files with 148 additions and 4 deletions
|
|
@ -11,13 +11,15 @@ const RULES = `Rules:
|
||||||
- Title in English only
|
- Title in English only
|
||||||
- Body must contain '## Что сделано' heading
|
- Body must contain '## Что сделано' heading
|
||||||
- Body must contain '## Почему' heading
|
- Body must contain '## Почему' heading
|
||||||
|
- Body must contain '## Watch out' heading
|
||||||
|
- Body must contain '## Pending' heading
|
||||||
- Body in Russian (must contain Cyrillic)`
|
- Body in Russian (must contain Cyrillic)`
|
||||||
|
|
||||||
export default tool({
|
export default tool({
|
||||||
description: "Create a GitHub PR with title/body validation. Validates: title format type(scope): description (<=72), English title, body headings (## Что сделано, ## Почему), body in Russian. If issue_number provided, appends 'Closes #N' to body. Returns PR URL on success.",
|
description: "Create a GitHub PR with title/body validation. Validates: title format type(scope): description (<=72), English title, body headings (## Что сделано, ## Почему, ## Watch out, ## Pending), body in Russian. If issue_number provided, appends 'Closes #N' to body. Returns PR URL on success.",
|
||||||
args: {
|
args: {
|
||||||
title: tool.schema.string().describe("PR title (conventional format: type(scope): description)"),
|
title: tool.schema.string().describe("PR title (conventional format: type(scope): description)"),
|
||||||
body: tool.schema.string().describe("PR body in Russian with ## Что сделано and ## Почему headings"),
|
body: tool.schema.string().describe("PR body in Russian with ## Что сделано, ## Почему, ## Watch out and ## Pending headings"),
|
||||||
issue_number: tool.schema.number().optional().describe("Issue number to link (appends 'Closes #N' to body)"),
|
issue_number: tool.schema.number().optional().describe("Issue number to link (appends 'Closes #N' to body)"),
|
||||||
repo: tool.schema.string().optional().describe("Optional repo (owner/name). If omitted, gh auto-detects from context.worktree."),
|
repo: tool.schema.string().optional().describe("Optional repo (owner/name). If omitted, gh auto-detects from context.worktree."),
|
||||||
},
|
},
|
||||||
|
|
@ -37,6 +39,12 @@ export default tool({
|
||||||
if (!body.includes("## Почему")) {
|
if (!body.includes("## Почему")) {
|
||||||
return `❌ PR body must contain '## Почему' heading\n\n${RULES}`
|
return `❌ PR body must contain '## Почему' heading\n\n${RULES}`
|
||||||
}
|
}
|
||||||
|
if (!body.includes("## Watch out")) {
|
||||||
|
return `❌ PR body must contain '## Watch out' heading\n\n${RULES}`
|
||||||
|
}
|
||||||
|
if (!body.includes("## Pending")) {
|
||||||
|
return `❌ PR body must contain '## Pending' heading\n\n${RULES}`
|
||||||
|
}
|
||||||
if (!CYRILLIC.test(body)) {
|
if (!CYRILLIC.test(body)) {
|
||||||
return `❌ PR body must be in Russian\n\n${RULES}`
|
return `❌ PR body must be in Russian\n\n${RULES}`
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,14 @@
|
||||||
|
# ADR-090: Enforce Watch out and Pending headings in PR body
|
||||||
|
|
||||||
|
## Статус
|
||||||
|
Accepted (2026-08-01)
|
||||||
|
|
||||||
|
## Контекст
|
||||||
|
Pipeline migration убирает DOCS phase. Handoff файлы удаляются. PR body должен стать единственным source of truth для review/memory. Раньше PR body валидировал только `## Что сделано` и `## Почему` — недостаточно для замены handoff.
|
||||||
|
|
||||||
|
## Решение
|
||||||
|
Расширить create-pr.ts validation до 4 heading'ов: `## Что сделано`, `## Почему`, `## Watch out` (required, em-dash если пусто), `## Pending` (required heading, content optional). Substring check (body.includes) — same as existing pattern.
|
||||||
|
|
||||||
|
## Альтернативы
|
||||||
|
1. Regex-based validation — отвергнута (несоответствует текущему pattern).
|
||||||
|
2. Опциональные heading'и — отвергнуто (цель — единый source of truth, requires enforcement).
|
||||||
16
docs/handoff/pr-207-enforce-watch-out-pending-headings.md
Normal file
16
docs/handoff/pr-207-enforce-watch-out-pending-headings.md
Normal file
|
|
@ -0,0 +1,16 @@
|
||||||
|
---
|
||||||
|
pr: 207
|
||||||
|
title: feat(create-pr): enforce Watch out and Pending headings in PR body
|
||||||
|
---
|
||||||
|
|
||||||
|
## Что сделано
|
||||||
|
Расширена валидация PR body в create-pr.ts с 2 heading'ов до 4 — добавлены обязательные `## Watch out` и `## Pending`. Обновлены RULES string, execute() validation, description. Добавлены тесты в test_create_pr_tool.py (3 теста) и test_create_pr_tool.ts (3 теста).
|
||||||
|
|
||||||
|
## Почему
|
||||||
|
Pipeline migration убирает DOCS phase — handoff файлы удаляются, PR body становится единственным source of truth. Нужно чтобы PR body содержал всю информацию ранее жившую в handoff (включая Watch out и Pending).
|
||||||
|
|
||||||
|
## Pending
|
||||||
|
Удаление handoff файлов и обновление pipeline-status.py — отдельный issue (out of scope).
|
||||||
|
|
||||||
|
## Watch out
|
||||||
|
`## Watch out: —` (em-dash) валиден если нет gotchas. `## Pending: —` валиден если нет follow-ups. Validation = substring check (body.includes), не regex. PR title validation не трогать.
|
||||||
|
|
@ -31,7 +31,12 @@ TS_FILE = REPO_ROOT / ".opencode" / "tools" / "create-pr.ts"
|
||||||
TS_FILE_REL = ".opencode/tools/create-pr.ts"
|
TS_FILE_REL = ".opencode/tools/create-pr.ts"
|
||||||
|
|
||||||
VALID_TITLE = "feat(tools): add create-pr tool validation"
|
VALID_TITLE = "feat(tools): add create-pr tool validation"
|
||||||
VALID_BODY = "## Что сделано\nДобавлен tool\n\n## Почему\nНужна валидация"
|
VALID_BODY = (
|
||||||
|
"## Что сделано\nДобавлен tool\n\n"
|
||||||
|
"## Почему\nНужна валидация\n\n"
|
||||||
|
"## Watch out\n—\n\n"
|
||||||
|
"## Pending\n—"
|
||||||
|
)
|
||||||
PR_URL = "https://github.com/slaid098/opencode-config/pull/38"
|
PR_URL = "https://github.com/slaid098/opencode-config/pull/38"
|
||||||
PR_OK_RESPONSE = {"status": 0, "stdout": PR_URL + "\n", "stderr": ""}
|
PR_OK_RESPONSE = {"status": 0, "stdout": PR_URL + "\n", "stderr": ""}
|
||||||
|
|
||||||
|
|
@ -107,6 +112,50 @@ def test_missing_pochemu():
|
||||||
assert "## Почему" in result, f"expected heading error, got: {result!r}"
|
assert "## Почему" in result, f"expected heading error, got: {result!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_missing_watch_out():
|
||||||
|
"""execute() with body missing '## Watch out' → error mentioning heading.
|
||||||
|
|
||||||
|
Body has the first two required headings (## Что сделано, ## Почему) so the
|
||||||
|
earlier checks pass; the third heading check (## Watch out) fires.
|
||||||
|
"""
|
||||||
|
body = "## Что сделано\nСделано\n\n## Почему\nПотому что\n\n## Pending\n—"
|
||||||
|
out = _run_exec({"title": VALID_TITLE, "body": body}, [PR_OK_RESPONSE])
|
||||||
|
result = out["result"]
|
||||||
|
assert "## Watch out" in result, f"expected heading error, got: {result!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_missing_pending():
|
||||||
|
"""execute() with body missing '## Pending' → error mentioning heading.
|
||||||
|
|
||||||
|
Body has the first three required headings (## Что сделано, ## Почему,
|
||||||
|
## Watch out) so the earlier checks pass; the fourth heading check
|
||||||
|
(## Pending) fires.
|
||||||
|
"""
|
||||||
|
body = "## Что сделано\nСделано\n\n## Почему\nПотому что\n\n## Watch out\n—"
|
||||||
|
out = _run_exec({"title": VALID_TITLE, "body": body}, [PR_OK_RESPONSE])
|
||||||
|
result = out["result"]
|
||||||
|
assert "## Pending" in result, f"expected heading error, got: {result!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_all_four_headings_success():
|
||||||
|
"""execute() with all 4 headings (incl. ## Watch out, ## Pending) succeeds.
|
||||||
|
|
||||||
|
A body with all four required headings (## Что сделано, ## Почему,
|
||||||
|
## Watch out, ## Pending) passes validation and returns the PR URL.
|
||||||
|
Uses em-dash placeholders for empty Watch out / Pending sections (ADR
|
||||||
|
pattern) — heading presence is sufficient, content may be '—'.
|
||||||
|
"""
|
||||||
|
body = (
|
||||||
|
"## Что сделано\nДобавлен enforcement\n\n"
|
||||||
|
"## Почему\nPipeline migration делает PR body single source of truth\n\n"
|
||||||
|
"## Watch out\n—\n\n"
|
||||||
|
"## Pending\n—"
|
||||||
|
)
|
||||||
|
out = _run_exec({"title": VALID_TITLE, "body": body}, [PR_OK_RESPONSE])
|
||||||
|
result = out["result"]
|
||||||
|
assert result == f"PR created: {PR_URL}", f"expected success, got: {result!r}"
|
||||||
|
|
||||||
|
|
||||||
def test_latin_only_body():
|
def test_latin_only_body():
|
||||||
"""execute() with body containing no Cyrillic → error.
|
"""execute() with body containing no Cyrillic → error.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,6 +17,9 @@
|
||||||
* - test_missing_scope — title without scope → error
|
* - test_missing_scope — title without scope → error
|
||||||
* - test_missing_chto_sdelano — body missing ## Что сделано → error
|
* - test_missing_chto_sdelano — body missing ## Что сделано → error
|
||||||
* - test_missing_pochemu — body missing ## Почему → error
|
* - test_missing_pochemu — body missing ## Почему → error
|
||||||
|
* - test_missing_watch_out — body missing ## Watch out → error
|
||||||
|
* - test_missing_pending — body missing ## Pending → error
|
||||||
|
* - test_all_four_headings_success — all 4 headings → success
|
||||||
* - test_latin_only_body — body without Cyrillic → error
|
* - test_latin_only_body — body without Cyrillic → error
|
||||||
* - test_issue_linkage — issue_number → body gets Closes #N
|
* - test_issue_linkage — issue_number → body gets Closes #N
|
||||||
*/
|
*/
|
||||||
|
|
@ -36,7 +39,12 @@ function ctx() {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const VALID_BODY = "## Что сделано\nДобавлен tool\n\n## Почему\nНужна валидация"
|
const VALID_BODY = (
|
||||||
|
"## Что сделано\nДобавлен tool\n\n" +
|
||||||
|
"## Почему\nНужна валидация\n\n" +
|
||||||
|
"## Watch out\n—\n\n" +
|
||||||
|
"## Pending\n—"
|
||||||
|
)
|
||||||
|
|
||||||
describe("create-pr tool", () => {
|
describe("create-pr tool", () => {
|
||||||
test("test_valid_pr — valid title + body succeeds", async () => {
|
test("test_valid_pr — valid title + body succeeds", async () => {
|
||||||
|
|
@ -87,6 +95,55 @@ describe("create-pr tool", () => {
|
||||||
expect(result).toContain("## Почему")
|
expect(result).toContain("## Почему")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("test_missing_watch_out — body missing ## Watch out → error", async () => {
|
||||||
|
// Body has the first two required headings (## Что сделано, ## Почему) so
|
||||||
|
// the earlier checks pass; the third heading check (## Watch out) fires.
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: () => ({ status: 0, stdout: "url\n", stderr: "" }),
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({
|
||||||
|
title: "feat(tools): valid title",
|
||||||
|
body: "## Что сделано\nСделано\n\n## Почему\nПотому что\n\n## Pending\n—",
|
||||||
|
}, ctx())
|
||||||
|
expect(result).toContain("## Watch out")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_missing_pending — body missing ## Pending → error", async () => {
|
||||||
|
// Body has the first three required headings (## Что сделано, ## Почему,
|
||||||
|
// ## Watch out) so the earlier checks pass; the fourth heading check
|
||||||
|
// (## Pending) fires.
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: () => ({ status: 0, stdout: "url\n", stderr: "" }),
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({
|
||||||
|
title: "feat(tools): valid title",
|
||||||
|
body: "## Что сделано\nСделано\n\n## Почему\nПотому что\n\n## Watch out\n—",
|
||||||
|
}, ctx())
|
||||||
|
expect(result).toContain("## Pending")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("test_all_four_headings_success — all 4 headings → success", async () => {
|
||||||
|
// A body with all four required headings passes validation and returns
|
||||||
|
// the PR URL. Uses em-dash placeholders for empty Watch out / Pending
|
||||||
|
// sections (ADR pattern) — heading presence is sufficient.
|
||||||
|
mock.module("child_process", () => ({
|
||||||
|
spawnSync: () => ({ status: 0, stdout: "https://github.com/x/y/pull/1\n", stderr: "" }),
|
||||||
|
}))
|
||||||
|
const mod = await import(TOOL_SRC + "?t=" + Date.now())
|
||||||
|
const result = await mod.default.execute({
|
||||||
|
title: "feat(tools): valid title",
|
||||||
|
body: (
|
||||||
|
"## Что сделано\nДобавлен enforcement\n\n" +
|
||||||
|
"## Почему\nPipeline migration делает PR body single source of truth\n\n" +
|
||||||
|
"## Watch out\n—\n\n" +
|
||||||
|
"## Pending\n—"
|
||||||
|
),
|
||||||
|
}, ctx())
|
||||||
|
expect(result).toBe("PR created: https://github.com/x/y/pull/1")
|
||||||
|
})
|
||||||
|
|
||||||
test("test_latin_only_body — body without Cyrillic → error", async () => {
|
test("test_latin_only_body — body without Cyrillic → error", async () => {
|
||||||
// NOTE: spec validation order checks headings (## Что сделано, ## Почему)
|
// NOTE: spec validation order checks headings (## Что сделано, ## Почему)
|
||||||
// BEFORE the Cyrillic check. Since the headings themselves are Cyrillic,
|
// BEFORE the Cyrillic check. Since the headings themselves are Cyrillic,
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue