feat(kanban): post-approve merger agent - #23
Merged
Merged
Conversation
Add DESIGN.md covering: - New 'merging' kanban status between human_review and done - approve_task() PR-detection routing (human_review -> merging vs done) - New post-approve-merger profile and skill - Idempotency via gh pr view state as source of truth - All 6 PR-state branches and their outcomes - dispatch_once() merging column (parallel to review column) - Gateway notifier events - Open Q on dashboard column order for Sahil No code yet — approval gate.
When kanban_approve is called on a human_review task that has an associated PR URL, instead of transitioning directly to done, the task is claimed for a post-approve-merger worker which merges the PR and transitions to done/blocked. Changes: - kanban_db.py: add _extract_pr_url(), claim_merger_task(), update approve_task() to return (bool, outcome, pr_url, task) tuple and route PR-bearing tasks via claim_merger_task - kanban_tools.py: update _handle_approve() to spawn post-approve-merger worker when outcome=merge_triggered - kanban.py: update _cmd_approve() to spawn merger from CLI path too, print descriptive message for merge-triggered outcome - gateway/run.py: add merge_requested to TERMINAL_KINDS, add notifier message for merge_requested events - skills/devops/post-approve-merger/SKILL.md: new skill for the merger worker with full PR state-machine (6 branches), auth pattern, and idempotency rules - tests/hermes_cli/test_kanban_merging.py: new tests for _extract_pr_url, claim_merger_task, approve_task routing (15 test cases) - tests/hermes_cli/test_kanban_human_review.py: update existing tests for new approve_task tuple return type - DESIGN.md: updated to reflect no-merging-status decision No new VALID_STATUSES added. No new dispatcher column needed. The post-approve-merger profile lives at ~/.hermes/profiles/post-approve-merger/.
|
Warning Review limit reached
More reviews will be available in 53 minutes and 25 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR implements a complete "post-approve merger" workflow for Kanban tasks: when a task in human_review has an associated GitHub PR URL, the system claims the task for a background merger worker, spawns that worker to handle CI waits and PR merging, and transitions the task to done or blocked based on merge success. ChangesPost-Approve Merger Workflow
Sequence DiagramsequenceDiagram
participant User
participant ApproveTask as approve_task()
participant KanbanDB as kanban_db
participant CLI as hermes_cli/kanban
participant ToolHandler as tools/kanban_tools
participant MergerSkill as post-approve-merger
User->>ApproveTask: approve(task_id, reason)
ApproveTask->>KanbanDB: _extract_pr_url(task_id)
KanbanDB-->>ApproveTask: pr_url found
ApproveTask->>KanbanDB: claim_merger_task(task_id, pr_url)
KanbanDB->>KanbanDB: transition human_review→running
KanbanDB->>KanbanDB: emit merge_requested event
KanbanDB-->>ApproveTask: (merge_triggered, pr_url, task)
ApproveTask-->>CLI: merge_triggered outcome
CLI->>CLI: resolve workspace, add skill
CLI->>ToolHandler: spawn merger worker
ToolHandler->>ToolHandler: prepare workspace
ToolHandler->>MergerSkill: start(post-approve-merger)
MergerSkill->>MergerSkill: gh pr view --json state
MergerSkill->>MergerSkill: wait for CI (polling)
MergerSkill->>MergerSkill: gh pr merge --squash
MergerSkill->>KanbanDB: kanban_complete()
KanbanDB-->>MergerSkill: task done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
auto-review: changes requested. 1 blocking finding — S1 positive imperatives in The skill has 3 negation-form directives outside fenced code blocks. Per the skill authoring convention, these must be rewritten as positive imperatives (LLMs pink-elephant on negations).
All other rules passed: in-scope files, no secrets, AC coverage (with Sahil's updated no-merging-status decision), CI green (CodeRabbit pending but non-required), type discipline clean, 15+2 tests, frontmatter valid, no system leaks. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@DESIGN.md`:
- Line 40: Add explicit language identifiers to all unlabeled fenced code blocks
in DESIGN.md that are triggering markdownlint MD040 (e.g., replace ``` with
```bash, ```mermaid, ```text or another appropriate language) for the instances
flagged (around lines noted: 40, 132, 222, 240, 252, 268) so each fenced block
has a language specifier; ensure the chosen identifier matches the block content
(shell snippets -> bash, diagrams -> mermaid, plain output -> text) to clear the
MD040 warnings.
- Line 67: The document references stale "merging" transitions and status values
that conflict with the approved model; update all occurrences of "human_review
-> merging", status='merging', and any text suggesting a new "merging" state to
instead use "human_review -> running" and status='running' so the contract
matches the approved approve_task() flow (no new status/column). Search for
occurrences of "merging" and "human_review -> merging" (including the mentions
around approve_task()) and replace them with "running"/"human_review ->
running", and verify the surrounding sentences no longer imply introducing a new
status/column.
In `@hermes_cli/kanban_db.py`:
- Around line 3939-3943: The code assumes payload is a dict and calls
payload.get("reason"), which will fail for JSON lists/strings; after
json.loads(row["payload"]) ensure payload is a dict (e.g., if not
isinstance(payload, dict) set reason = str(payload) or extract a "reason" if
present), then compute m = _RESPAWN_GUARD_PR_URL_RE.search(reason); update the
payload handling around payload, reason, and _RESPAWN_GUARD_PR_URL_RE usage so
non-dict JSONs are converted to a safe string instead of calling .get on them.
In `@skills/devops/post-approve-merger/SKILL.md`:
- Around line 24-26: Add language identifiers to the unlabeled fenced code
blocks in the SKILL.md content (the blocks containing lines like 'events where
kind == "merge_requested" → payload.pr_url' and the other occurrences at the
noted ranges) to satisfy markdownlint MD040; update each triple-backtick fence
to include an appropriate tag (e.g., ```text or ```bash) so the blocks are
explicitly labeled.
- Around line 20-165: The SKILL.md flow uses disallowed third-party CLIs (gh,
git, shell commands and GH_TOKEN usage) in sections like Trigger and context,
Auth identity, Idempotency rule, rebase steps, CI polling and Auth note; replace
these with calls to Hermes-native tools or explicitly state expected MCP
servers: update references to GH_TOKEN/GH_TOKEN_SAHILM_AI, gh pr view/merge, git
clone/rebase/push and the extraheader pattern to instead invoke the
corresponding Hermes APIs or documented MCP endpoints (or add prose that this
skill requires a specific MCP with exact API surface), and adjust examples in
the Idempotency rule, PR state machine (steps 1–7), rebase/checkout/push and
Cleanup to use the native tool names or MCP contract so the SKILL.md no longer
prescribes third‑party CLI usage while preserving the same state checks and
terminal calls (kanban_complete, kanban_block, kanban_show).
- Around line 12-165: The SKILL.md content must be reorganized to match the
required template: keep the title "post-approve-merger — PR Merge Worker" and
add a 2–3 sentence intro, then create the headings in this exact order: ## When
to Use, ## Prerequisites, ## How to Run, ## Quick Reference, ## Procedure, ##
Pitfalls, ## Verification; move relevant existing sections under those headings
(e.g., put auth and GH_TOKEN guidance under Prerequisites, call/sequence steps
including kanban_show(), idempotency gh pr view block, PR state machine and
merge steps under Procedure, the one-line success/termination rules with
kanban_complete and kanban_block under Quick Reference or Procedure as
appropriate), and ensure examples like the rebase flow, CI polling, and cleanup
rm -rf /tmp/merger-rebase-$$ are preserved in Procedure; keep content semantics
but reorganize headings and add the brief intro per guidelines.
- Line 3: The frontmatter 'description' value in SKILL.md is too long and
repeats the skill name; replace the current description key with a single,
non-marketing sentence of 60 characters or fewer that ends with a period and
does not include the skill name (e.g., shorten to a concise purpose line). Edit
the 'description' frontmatter field (the description: value) to meet these
constraints and ensure it's one sentence ≤60 chars and ends with a period.
In `@tools/kanban_tools.py`:
- Around line 979-982: Replace the two assert statements that check task and
pr_url (used after approve_task() when outcome == "merge_triggered") with
explicit runtime validations: verify task is not None and pr_url is not None and
raise a clear exception (e.g., RuntimeError or ValueError) or log an error and
return early if they are missing; include contextual information (e.g., outcome
value, result of approve_task(), expected task.id/task.skills) to aid debugging
before aborting/returning. Ensure the checks are added where outcome ==
"merge_triggered" is handled so subsequent uses of task.skills and task.id are
safe.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 04ec560f-7574-4372-b54c-0da70ab7b1cc
📒 Files selected for processing (8)
DESIGN.mdgateway/run.pyhermes_cli/kanban.pyhermes_cli/kanban_db.pyskills/devops/post-approve-merger/SKILL.mdtests/hermes_cli/test_kanban_human_review.pytests/hermes_cli/test_kanban_merging.pytools/kanban_tools.py
| **Recommendation: `sahilm-ai` OAuth token via `GH_TOKEN_SAHILM_AI` env var.** | ||
|
|
||
| Per the established pattern documented in session memory and the `github-pr-workflow` skill, autonomous workers push/merge using: | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks flagged by markdownlint.
The unlabeled fences trigger MD040; adding explicit languages (bash, mermaid, text, etc.) will clear these warnings.
Also applies to: 132-132, 222-222, 240-240, 252-252, 268-268
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DESIGN.md` at line 40, Add explicit language identifiers to all unlabeled
fenced code blocks in DESIGN.md that are triggering markdownlint MD040 (e.g.,
replace ``` with ```bash, ```mermaid, ```text or another appropriate language)
for the instances flagged (around lines noted: 40, 132, 222, 240, 252, 268) so
each fenced block has a language specifier; ensure the chosen identifier matches
the block content (shell snippets -> bash, diagrams -> mermaid, plain output ->
text) to clear the MD040 warnings.
|
|
||
| ### Q4 — No-PR `human_review` tasks? | ||
|
|
||
| **Recommendation: `approve_task()` checks for a PR URL. If none found → `human_review → done` (current behavior). If found → `human_review → merging`.** |
There was a problem hiding this comment.
Resolve stale merging-status references to match the approved running flow.
These sections still document human_review -> merging / status='merging', which conflicts with the stated approved model (human_review -> running, no new status/column). Please normalize these contract lines to running so implementation guidance is unambiguous.
Also applies to: 123-123, 126-126, 224-235, 250-264, 314-314, 334-334
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DESIGN.md` at line 67, The document references stale "merging" transitions
and status values that conflict with the approved model; update all occurrences
of "human_review -> merging", status='merging', and any text suggesting a new
"merging" state to instead use "human_review -> running" and status='running' so
the contract matches the approved approve_task() flow (no new status/column).
Search for occurrences of "merging" and "human_review -> merging" (including the
mentions around approve_task()) and replace them with "running"/"human_review ->
running", and verify the surrounding sentences no longer imply introducing a new
status/column.
Comment on lines
+3939
to
+3943
| payload = json.loads(row["payload"]) | ||
| except (ValueError, TypeError): | ||
| continue | ||
| reason = payload.get("reason") or "" | ||
| m = _RESPAWN_GUARD_PR_URL_RE.search(str(reason)) |
There was a problem hiding this comment.
Guard against non-dict JSON payloads in PR extraction.
If payload parses as a JSON list/string, Line 3942 calls .get(...) on a non-dict and can abort approval routing.
Suggested fix
try:
payload = json.loads(row["payload"])
except (ValueError, TypeError):
continue
+ if not isinstance(payload, dict):
+ continue
reason = payload.get("reason") or ""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hermes_cli/kanban_db.py` around lines 3939 - 3943, The code assumes payload
is a dict and calls payload.get("reason"), which will fail for JSON
lists/strings; after json.loads(row["payload"]) ensure payload is a dict (e.g.,
if not isinstance(payload, dict) set reason = str(payload) or extract a "reason"
if present), then compute m = _RESPAWN_GUARD_PR_URL_RE.search(reason); update
the payload handling around payload, reason, and _RESPAWN_GUARD_PR_URL_RE usage
so non-dict JSONs are converted to a safe string instead of calling .get on
them.
| @@ -0,0 +1,165 @@ | |||
| --- | |||
| name: post-approve-merger | |||
| description: "Load when spawned as a post-approve-merger kanban worker. Covers the full PR merge state-machine: check PR state, handle CI pending/failed, conflicts, drafts, already-merged, and the correct kanban_complete/kanban_block contract." | |||
There was a problem hiding this comment.
Frontmatter description violates SKILL.md constraints.
description is longer than 60 chars and repeats the skill name; please rewrite it to a single short sentence (≤60 chars) ending with a period.
As per coding guidelines, "Skill description in SKILL.md frontmatter must be ≤60 characters, one sentence, end with a period, and must not repeat the skill name or use marketing words."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/devops/post-approve-merger/SKILL.md` at line 3, The frontmatter
'description' value in SKILL.md is too long and repeats the skill name; replace
the current description key with a single, non-marketing sentence of 60
characters or fewer that ends with a period and does not include the skill name
(e.g., shorten to a concise purpose line). Edit the 'description' frontmatter
field (the description: value) to meet these constraints and ensure it's one
sentence ≤60 chars and ends with a period.
Comment on lines
+12
to
+165
| # post-approve-merger — PR Merge Worker | ||
|
|
||
| You are spawned automatically when `kanban_approve` is run on a task that | ||
| has an associated GitHub PR. Your job: merge that PR, then call | ||
| `kanban_complete` or `kanban_block`. | ||
|
|
||
| --- | ||
|
|
||
| ## Trigger and context | ||
|
|
||
| 1. Call `kanban_show()` at startup (no args — defaults to your task). | ||
| 2. Find the PR URL in the most recent `merge_requested` event payload: | ||
| ``` | ||
| events where kind == "merge_requested" → payload.pr_url | ||
| ``` | ||
| If no `merge_requested` event is found, scan task comments for a | ||
| `https://github.com/.../pull/N` URL as fallback. | ||
| 3. If you cannot find any PR URL after both passes → `kanban_complete` | ||
| with summary "approved, no PR to merge". Stop. | ||
|
|
||
| --- | ||
|
|
||
| ## Auth identity | ||
|
|
||
| All `gh` invocations must use `GH_TOKEN=$GH_TOKEN_SAHILM_AI`: | ||
|
|
||
| ```bash | ||
| TOKEN_VAR="GH_TOKEN_SAHILM_AI" | ||
| GH_TOKEN=$(printenv "$TOKEN_VAR") gh pr view "$PR_URL" --json state,... | ||
| ``` | ||
|
|
||
| Never use `sahilm-ti` credentials. Never use the bare `$GH_TOKEN` env var | ||
| unless it has already been set to `GH_TOKEN_SAHILM_AI` in this session. | ||
|
|
||
| --- | ||
|
|
||
| ## Idempotency rule (ALWAYS run first) | ||
|
|
||
| Before any merge attempt, call: | ||
| ```bash | ||
| GH_TOKEN=$(printenv GH_TOKEN_SAHILM_AI) gh pr view "$PR_URL" \ | ||
| --json state,merged,isDraft,mergeable,statusCheckRollup,baseRefName,headRefName | ||
| ``` | ||
|
|
||
| Treat the `gh pr view` output as the authoritative source of truth — | ||
| not local memory, not previous event history. | ||
|
|
||
| --- | ||
|
|
||
| ## PR state machine (handle every branch) | ||
|
|
||
| Parse the `gh pr view` JSON output and act on these cases, in order: | ||
|
|
||
| ### 1. Already merged (`merged == true`) | ||
| ``` | ||
| kanban_complete(summary="PR <url> was already merged — task done") | ||
| ``` | ||
| Stop. (Idempotency: safe if we crashed after the merge but before complete.) | ||
|
|
||
| ### 2. Closed (not merged) (`state == "CLOSED" and merged == false`) | ||
| ``` | ||
| kanban_block(reason="PR <url> was closed without merging — clarify intent before re-approving") | ||
| ``` | ||
| Stop. | ||
|
|
||
| ### 3. Draft (`isDraft == true`) | ||
| ``` | ||
| kanban_block(reason="PR <url> is still a draft — mark ready for review, then re-approve") | ||
| ``` | ||
| Stop. | ||
|
|
||
| ### 4. Merge conflict (`mergeable == "CONFLICTING"`) | ||
| Attempt one rebase: | ||
| ```bash | ||
| REPO=$(GH_TOKEN=$(printenv GH_TOKEN_SAHILM_AI) gh pr view "$PR_URL" \ | ||
| --json headRepositoryOwner,headRepository \ | ||
| --jq '.headRepositoryOwner.login + "/" + .headRepository.name') | ||
| HEAD_BRANCH=$(GH_TOKEN=$(printenv GH_TOKEN_SAHILM_AI) gh pr view "$PR_URL" \ | ||
| --json headRefName --jq '.headRefName') | ||
| BASE_BRANCH=$(GH_TOKEN=$(printenv GH_TOKEN_SAHILM_AI) gh pr view "$PR_URL" \ | ||
| --json baseRefName --jq '.baseRefName') | ||
|
|
||
| git clone "https://github.com/$REPO.git" /tmp/merger-rebase-$$ | ||
| cd /tmp/merger-rebase-$$ | ||
| git fetch origin | ||
| git checkout "$HEAD_BRANCH" | ||
| git rebase "origin/$BASE_BRANCH" | ||
| ``` | ||
| - If rebase succeeds → push (using `GH_TOKEN_SAHILM_AI` Basic auth extraheader, see §Auth) → proceed to §5. | ||
| - If rebase fails → `kanban_block(reason="Merge conflict on PR <url> — manual rebase needed. Conflicting files: <list>")` | ||
|
|
||
| ### 5. CI pending (`statusCheckRollup has items with status == "IN_PROGRESS" or "QUEUED"`) | ||
| Poll every 30 seconds, up to 10 minutes (20 polls). On each poll: | ||
| - If all checks are SUCCESS or SKIPPED → proceed to §6 (merge). | ||
| - If any check FAILED → proceed to §7 (CI failed). | ||
| - If still pending after 20 polls → `kanban_block(reason="CI still pending after 10 min on PR <url> — re-approve when CI passes")` | ||
|
|
||
| Check for a `ci_wait_timeout_minutes=N` annotation in the task body or | ||
| most recent comment. If found, override the 10-minute default. | ||
|
|
||
| ### 6. Open, mergeable, CI green | ||
| ```bash | ||
| GH_TOKEN=$(printenv GH_TOKEN_SAHILM_AI) gh pr merge "$PR_URL" \ | ||
| --squash --delete-branch | ||
| ``` | ||
| On success: | ||
| ``` | ||
| kanban_complete( | ||
| summary="Merged PR <url> (squash+delete-branch)", | ||
| metadata={"pr_url": "<url>", "merge_method": "squash"} | ||
| ) | ||
| ``` | ||
|
|
||
| On `gh` error → `kanban_block(reason="gh pr merge failed: <stderr>")` | ||
|
|
||
| ### 7. CI failed (`statusCheckRollup has items with conclusion == "FAILURE"`) | ||
| Collect the failing check names and their URLs: | ||
| ```bash | ||
| GH_TOKEN=$(printenv GH_TOKEN_SAHILM_AI) gh pr view "$PR_URL" \ | ||
| --json statusCheckRollup --jq \ | ||
| '[.statusCheckRollup[] | select(.conclusion == "FAILURE") | {name: .name, detailsUrl: .detailsUrl}]' | ||
| ``` | ||
| ``` | ||
| kanban_block(reason="CI failing on PR <url> — fix and re-approve. Failing checks: <list with URLs>") | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## kanban_complete / kanban_block contract | ||
|
|
||
| - `kanban_complete` is the ONLY success terminator. Do NOT call `kanban_review` or `kanban_human_review`. | ||
| - `kanban_block` is the ONLY failure terminator. Always include the PR URL and an actionable next step in the reason. | ||
| - One and only one terminal call per run. After it, stop — do not attempt anything else. | ||
|
|
||
| --- | ||
|
|
||
| ## Cleanup | ||
|
|
||
| After a successful merge, remove the local clone if you created one: | ||
| ```bash | ||
| rm -rf /tmp/merger-rebase-$$ 2>/dev/null || true | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Auth note: pushing during rebase | ||
|
|
||
| Use the Basic auth extraheader pattern (from session memory): | ||
| ```bash | ||
| TOKEN=$(printenv GH_TOKEN_SAHILM_AI) | ||
| git -c http.https://github.com/.extraheader="Authorization: Basic $(printf 'x-access-token:%s' "$TOKEN" | base64)" \ | ||
| -c credential.helper= \ | ||
| push origin "$HEAD_BRANCH" | ||
| ``` |
There was a problem hiding this comment.
SKILL.md body structure does not match the required section template.
Please reorganize to include the mandated headings and order: ## When to Use, ## Prerequisites, ## How to Run, ## Quick Reference, ## Procedure, ## Pitfalls, ## Verification, plus the required intro shape.
As per coding guidelines, "SKILL.md body structure must follow: title, 2-3 sentence intro, ## When to Use, ## Prerequisites, ## How to Run, ## Quick Reference, ## Procedure, ## Pitfalls, ## Verification."
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 72-72: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 118-118: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 134-134: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/devops/post-approve-merger/SKILL.md` around lines 12 - 165, The
SKILL.md content must be reorganized to match the required template: keep the
title "post-approve-merger — PR Merge Worker" and add a 2–3 sentence intro, then
create the headings in this exact order: ## When to Use, ## Prerequisites, ##
How to Run, ## Quick Reference, ## Procedure, ## Pitfalls, ## Verification; move
relevant existing sections under those headings (e.g., put auth and GH_TOKEN
guidance under Prerequisites, call/sequence steps including kanban_show(),
idempotency gh pr view block, PR state machine and merge steps under Procedure,
the one-line success/termination rules with kanban_complete and kanban_block
under Quick Reference or Procedure as appropriate), and ensure examples like the
rebase flow, CI polling, and cleanup rm -rf /tmp/merger-rebase-$$ are preserved
in Procedure; keep content semantics but reorganize headings and add the brief
intro per guidelines.
Comment on lines
+20
to
+165
| ## Trigger and context | ||
|
|
||
| 1. Call `kanban_show()` at startup (no args — defaults to your task). | ||
| 2. Find the PR URL in the most recent `merge_requested` event payload: | ||
| ``` | ||
| events where kind == "merge_requested" → payload.pr_url | ||
| ``` | ||
| If no `merge_requested` event is found, scan task comments for a | ||
| `https://github.com/.../pull/N` URL as fallback. | ||
| 3. If you cannot find any PR URL after both passes → `kanban_complete` | ||
| with summary "approved, no PR to merge". Stop. | ||
|
|
||
| --- | ||
|
|
||
| ## Auth identity | ||
|
|
||
| All `gh` invocations must use `GH_TOKEN=$GH_TOKEN_SAHILM_AI`: | ||
|
|
||
| ```bash | ||
| TOKEN_VAR="GH_TOKEN_SAHILM_AI" | ||
| GH_TOKEN=$(printenv "$TOKEN_VAR") gh pr view "$PR_URL" --json state,... | ||
| ``` | ||
|
|
||
| Never use `sahilm-ti` credentials. Never use the bare `$GH_TOKEN` env var | ||
| unless it has already been set to `GH_TOKEN_SAHILM_AI` in this session. | ||
|
|
||
| --- | ||
|
|
||
| ## Idempotency rule (ALWAYS run first) | ||
|
|
||
| Before any merge attempt, call: | ||
| ```bash | ||
| GH_TOKEN=$(printenv GH_TOKEN_SAHILM_AI) gh pr view "$PR_URL" \ | ||
| --json state,merged,isDraft,mergeable,statusCheckRollup,baseRefName,headRefName | ||
| ``` | ||
|
|
||
| Treat the `gh pr view` output as the authoritative source of truth — | ||
| not local memory, not previous event history. | ||
|
|
||
| --- | ||
|
|
||
| ## PR state machine (handle every branch) | ||
|
|
||
| Parse the `gh pr view` JSON output and act on these cases, in order: | ||
|
|
||
| ### 1. Already merged (`merged == true`) | ||
| ``` | ||
| kanban_complete(summary="PR <url> was already merged — task done") | ||
| ``` | ||
| Stop. (Idempotency: safe if we crashed after the merge but before complete.) | ||
|
|
||
| ### 2. Closed (not merged) (`state == "CLOSED" and merged == false`) | ||
| ``` | ||
| kanban_block(reason="PR <url> was closed without merging — clarify intent before re-approving") | ||
| ``` | ||
| Stop. | ||
|
|
||
| ### 3. Draft (`isDraft == true`) | ||
| ``` | ||
| kanban_block(reason="PR <url> is still a draft — mark ready for review, then re-approve") | ||
| ``` | ||
| Stop. | ||
|
|
||
| ### 4. Merge conflict (`mergeable == "CONFLICTING"`) | ||
| Attempt one rebase: | ||
| ```bash | ||
| REPO=$(GH_TOKEN=$(printenv GH_TOKEN_SAHILM_AI) gh pr view "$PR_URL" \ | ||
| --json headRepositoryOwner,headRepository \ | ||
| --jq '.headRepositoryOwner.login + "/" + .headRepository.name') | ||
| HEAD_BRANCH=$(GH_TOKEN=$(printenv GH_TOKEN_SAHILM_AI) gh pr view "$PR_URL" \ | ||
| --json headRefName --jq '.headRefName') | ||
| BASE_BRANCH=$(GH_TOKEN=$(printenv GH_TOKEN_SAHILM_AI) gh pr view "$PR_URL" \ | ||
| --json baseRefName --jq '.baseRefName') | ||
|
|
||
| git clone "https://github.com/$REPO.git" /tmp/merger-rebase-$$ | ||
| cd /tmp/merger-rebase-$$ | ||
| git fetch origin | ||
| git checkout "$HEAD_BRANCH" | ||
| git rebase "origin/$BASE_BRANCH" | ||
| ``` | ||
| - If rebase succeeds → push (using `GH_TOKEN_SAHILM_AI` Basic auth extraheader, see §Auth) → proceed to §5. | ||
| - If rebase fails → `kanban_block(reason="Merge conflict on PR <url> — manual rebase needed. Conflicting files: <list>")` | ||
|
|
||
| ### 5. CI pending (`statusCheckRollup has items with status == "IN_PROGRESS" or "QUEUED"`) | ||
| Poll every 30 seconds, up to 10 minutes (20 polls). On each poll: | ||
| - If all checks are SUCCESS or SKIPPED → proceed to §6 (merge). | ||
| - If any check FAILED → proceed to §7 (CI failed). | ||
| - If still pending after 20 polls → `kanban_block(reason="CI still pending after 10 min on PR <url> — re-approve when CI passes")` | ||
|
|
||
| Check for a `ci_wait_timeout_minutes=N` annotation in the task body or | ||
| most recent comment. If found, override the 10-minute default. | ||
|
|
||
| ### 6. Open, mergeable, CI green | ||
| ```bash | ||
| GH_TOKEN=$(printenv GH_TOKEN_SAHILM_AI) gh pr merge "$PR_URL" \ | ||
| --squash --delete-branch | ||
| ``` | ||
| On success: | ||
| ``` | ||
| kanban_complete( | ||
| summary="Merged PR <url> (squash+delete-branch)", | ||
| metadata={"pr_url": "<url>", "merge_method": "squash"} | ||
| ) | ||
| ``` | ||
|
|
||
| On `gh` error → `kanban_block(reason="gh pr merge failed: <stderr>")` | ||
|
|
||
| ### 7. CI failed (`statusCheckRollup has items with conclusion == "FAILURE"`) | ||
| Collect the failing check names and their URLs: | ||
| ```bash | ||
| GH_TOKEN=$(printenv GH_TOKEN_SAHILM_AI) gh pr view "$PR_URL" \ | ||
| --json statusCheckRollup --jq \ | ||
| '[.statusCheckRollup[] | select(.conclusion == "FAILURE") | {name: .name, detailsUrl: .detailsUrl}]' | ||
| ``` | ||
| ``` | ||
| kanban_block(reason="CI failing on PR <url> — fix and re-approve. Failing checks: <list with URLs>") | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## kanban_complete / kanban_block contract | ||
|
|
||
| - `kanban_complete` is the ONLY success terminator. Do NOT call `kanban_review` or `kanban_human_review`. | ||
| - `kanban_block` is the ONLY failure terminator. Always include the PR URL and an actionable next step in the reason. | ||
| - One and only one terminal call per run. After it, stop — do not attempt anything else. | ||
|
|
||
| --- | ||
|
|
||
| ## Cleanup | ||
|
|
||
| After a successful merge, remove the local clone if you created one: | ||
| ```bash | ||
| rm -rf /tmp/merger-rebase-$$ 2>/dev/null || true | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Auth note: pushing during rebase | ||
|
|
||
| Use the Basic auth extraheader pattern (from session memory): | ||
| ```bash | ||
| TOKEN=$(printenv GH_TOKEN_SAHILM_AI) | ||
| git -c http.https://github.com/.extraheader="Authorization: Basic $(printf 'x-access-token:%s' "$TOKEN" | base64)" \ | ||
| -c credential.helper= \ | ||
| push origin "$HEAD_BRANCH" | ||
| ``` |
There was a problem hiding this comment.
Tooling instructions rely on disallowed third-party CLIs.
The procedure is built around gh/git shell commands; this conflicts with the SKILL.md rule requiring native Hermes tools or explicitly expected MCP servers in prose.
As per coding guidelines, "Tools referenced in SKILL.md prose must be native Hermes tools or explicitly expected MCP servers, not shell utilities or third-party CLIs."
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 72-72: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 118-118: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 134-134: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/devops/post-approve-merger/SKILL.md` around lines 20 - 165, The
SKILL.md flow uses disallowed third-party CLIs (gh, git, shell commands and
GH_TOKEN usage) in sections like Trigger and context, Auth identity, Idempotency
rule, rebase steps, CI polling and Auth note; replace these with calls to
Hermes-native tools or explicitly state expected MCP servers: update references
to GH_TOKEN/GH_TOKEN_SAHILM_AI, gh pr view/merge, git clone/rebase/push and the
extraheader pattern to instead invoke the corresponding Hermes APIs or
documented MCP endpoints (or add prose that this skill requires a specific MCP
with exact API surface), and adjust examples in the Idempotency rule, PR state
machine (steps 1–7), rebase/checkout/push and Cleanup to use the native tool
names or MCP contract so the SKILL.md no longer prescribes third‑party CLI usage
while preserving the same state checks and terminal calls (kanban_complete,
kanban_block, kanban_show).
Comment on lines
+24
to
+26
| ``` | ||
| events where kind == "merge_requested" → payload.pr_url | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks to satisfy markdownlint.
These unlabeled fences trigger MD040; annotate them (for example text/bash) to clear lint warnings.
Also applies to: 66-68, 72-74, 78-80, 118-123, 134-136
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/devops/post-approve-merger/SKILL.md` around lines 24 - 26, Add
language identifiers to the unlabeled fenced code blocks in the SKILL.md content
(the blocks containing lines like 'events where kind == "merge_requested" →
payload.pr_url' and the other occurrences at the noted ranges) to satisfy
markdownlint MD040; update each triple-backtick fence to include an appropriate
tag (e.g., ```text or ```bash) so the blocks are explicitly labeled.
Comment on lines
+979
to
+982
| if outcome == "merge_triggered": | ||
| assert task is not None # claim_merger_task returned it | ||
| assert pr_url is not None | ||
| # Spawn the merger worker in the background. Import here to |
There was a problem hiding this comment.
Replace assert with explicit runtime checks for production safety.
Using assert for contract validation is stripped when Python runs with -O (optimize). If approve_task() returned unexpected values due to a bug, the subsequent task.skills and task.id accesses would raise AttributeError rather than failing cleanly.
Suggested fix
if outcome == "merge_triggered":
- assert task is not None # claim_merger_task returned it
- assert pr_url is not None
+ if task is None or pr_url is None:
+ return tool_error(
+ f"kanban_approve: internal error — merge_triggered "
+ f"but task or pr_url missing"
+ )
# Spawn the merger worker in the background. Import here to🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/kanban_tools.py` around lines 979 - 982, Replace the two assert
statements that check task and pr_url (used after approve_task() when outcome ==
"merge_triggered") with explicit runtime validations: verify task is not None
and pr_url is not None and raise a clear exception (e.g., RuntimeError or
ValueError) or log an error and return early if they are missing; include
contextual information (e.g., outcome value, result of approve_task(), expected
task.id/task.skills) to aid debugging before aborting/returning. Ensure the
checks are added where outcome == "merge_triggered" is handled so subsequent
uses of task.skills and task.id are safe.
…-merger SKILL.md S1 auto-review rejection fix: 3 negation-form lines rewritten as positive imperatives per sdlc-review rules. - 'Never use sahilm-ti credentials' -> 'Use sahilm-ai credentials exclusively' - 'Do NOT call kanban_review' -> 'The ONLY success terminator is kanban_complete' - 'After it, stop - do not attempt anything else' -> 'After the single terminal call, stop'
Owner
Author
|
auto-review: approved, awaiting human merge + kanban_approve. |
sahilm-ti
added a commit
that referenced
this pull request
May 26, 2026
…LL.md (#24) PR #23 had three negation-form directives rewritten as positive imperatives during the auto-reviewer pass, in a mistaken application of the BT-agent optimization-playbook S1 rule to a Hermes infrastructure skill. The S1/SD4 rules in sdlc-review have since been scoped BT-agent-only — but the rewrites in #23 landed before that scoping fix, so the load-bearing prohibitions are gone from the merged skill content. Restore them: 1. Auth: add explicit 'Never use sahilm-ti credentials' with the two concrete consequences (audit-trail misattribution + keychain prompt blocking the worker). The bare 'use sahilm-ai exclusively' positive form left the door open to drift, as evidenced by PR #23 itself — commit 7da3474 in that PR was authored as sahilm-ti. 2. Terminator contract: add explicit 'Do NOT call kanban_review' — without this prohibition the worker could re-loop the card through the auto-reviewer after Sahil has already approved, defeating the one-approval-equals-merged contract that motivated #23 in the first place. 3. Stop clause: 'After the single terminal call, stop — do not attempt any further gh/git/kanban_* operations.' A second terminal call corrupts the event log; the positive-only 'stop' form is too ambiguous (stop what? stop thinking? the model may interpret it as stop typing but keep tool-calling). No code changes. No test changes. Skill-content-only fix. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
May 27, 2026
…hook (#28) Part 1: Add pin_workspace_git_identity() to hermes_cli/kanban_db.py - Sets user.name/user.email in the workspace's local git config (survives subprocess shells, rebase resolution commits, auto-format follow-ups) - Installs a pre-commit hook in <workspace>/.hermes-hooks/ wired via core.hooksPath that hard-rejects commits whose GIT_AUTHOR_EMAIL does not match the configured worker identity - Opt-out: HERMES_KANBAN_ENFORCE_GIT_IDENTITY=false (env) or kanban.enforce_worker_git_identity: false (config.yaml) - Best-effort on non-git dirs: hook dir created; config write skipped Part 2: Call pin_workspace_git_identity from all workspace init paths - ensure_worktree(): called at end (both create and respawn) - dispatch_once(): called for scratch/dir workspace kinds in both the main and review dispatcher call sites Tests: 11 new tests in tests/hermes_cli/test_kanban_db.py covering worktree/scratch/dir workspace kinds, hook rejection, hook allowance, opt-out behaviour, and non-git dir noop. Also patches test_ensure_worktree_respawn_reuses_existing to opt out of identity enforcement (test is about respawn semantics, not identity). Fixes recurring C5 auto-reviewer failure where workers committed as sahilm-ti instead of sahilm-ai (PR #27 recurrence of PR #23 pattern). Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
May 28, 2026
* design: post-approve merger agent (t_5a521a19) Add DESIGN.md covering: - New 'merging' kanban status between human_review and done - approve_task() PR-detection routing (human_review -> merging vs done) - New post-approve-merger profile and skill - Idempotency via gh pr view state as source of truth - All 6 PR-state branches and their outcomes - dispatch_once() merging column (parallel to review column) - Gateway notifier events - Open Q on dashboard column order for Sahil No code yet — approval gate. * feat(kanban): post-approve merger agent (t_5a521a19) When kanban_approve is called on a human_review task that has an associated PR URL, instead of transitioning directly to done, the task is claimed for a post-approve-merger worker which merges the PR and transitions to done/blocked. Changes: - kanban_db.py: add _extract_pr_url(), claim_merger_task(), update approve_task() to return (bool, outcome, pr_url, task) tuple and route PR-bearing tasks via claim_merger_task - kanban_tools.py: update _handle_approve() to spawn post-approve-merger worker when outcome=merge_triggered - kanban.py: update _cmd_approve() to spawn merger from CLI path too, print descriptive message for merge-triggered outcome - gateway/run.py: add merge_requested to TERMINAL_KINDS, add notifier message for merge_requested events - skills/devops/post-approve-merger/SKILL.md: new skill for the merger worker with full PR state-machine (6 branches), auth pattern, and idempotency rules - tests/hermes_cli/test_kanban_merging.py: new tests for _extract_pr_url, claim_merger_task, approve_task routing (15 test cases) - tests/hermes_cli/test_kanban_human_review.py: update existing tests for new approve_task tuple return type - DESIGN.md: updated to reflect no-merging-status decision No new VALID_STATUSES added. No new dispatcher column needed. The post-approve-merger profile lives at ~/.hermes/profiles/post-approve-merger/. * fix(skill): rewrite negations as positive imperatives in post-approve-merger SKILL.md S1 auto-review rejection fix: 3 negation-form lines rewritten as positive imperatives per sdlc-review rules. - 'Never use sahilm-ti credentials' -> 'Use sahilm-ai credentials exclusively' - 'Do NOT call kanban_review' -> 'The ONLY success terminator is kanban_complete' - 'After it, stop - do not attempt anything else' -> 'After the single terminal call, stop' --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com> Co-authored-by: sahilm-ai <sahilm.ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
May 28, 2026
…LL.md (#24) PR #23 had three negation-form directives rewritten as positive imperatives during the auto-reviewer pass, in a mistaken application of the BT-agent optimization-playbook S1 rule to a Hermes infrastructure skill. The S1/SD4 rules in sdlc-review have since been scoped BT-agent-only — but the rewrites in #23 landed before that scoping fix, so the load-bearing prohibitions are gone from the merged skill content. Restore them: 1. Auth: add explicit 'Never use sahilm-ti credentials' with the two concrete consequences (audit-trail misattribution + keychain prompt blocking the worker). The bare 'use sahilm-ai exclusively' positive form left the door open to drift, as evidenced by PR #23 itself — commit 7da3474 in that PR was authored as sahilm-ti. 2. Terminator contract: add explicit 'Do NOT call kanban_review' — without this prohibition the worker could re-loop the card through the auto-reviewer after Sahil has already approved, defeating the one-approval-equals-merged contract that motivated #23 in the first place. 3. Stop clause: 'After the single terminal call, stop — do not attempt any further gh/git/kanban_* operations.' A second terminal call corrupts the event log; the positive-only 'stop' form is too ambiguous (stop what? stop thinking? the model may interpret it as stop typing but keep tool-calling). No code changes. No test changes. Skill-content-only fix. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
May 28, 2026
…hook (#28) Part 1: Add pin_workspace_git_identity() to hermes_cli/kanban_db.py - Sets user.name/user.email in the workspace's local git config (survives subprocess shells, rebase resolution commits, auto-format follow-ups) - Installs a pre-commit hook in <workspace>/.hermes-hooks/ wired via core.hooksPath that hard-rejects commits whose GIT_AUTHOR_EMAIL does not match the configured worker identity - Opt-out: HERMES_KANBAN_ENFORCE_GIT_IDENTITY=false (env) or kanban.enforce_worker_git_identity: false (config.yaml) - Best-effort on non-git dirs: hook dir created; config write skipped Part 2: Call pin_workspace_git_identity from all workspace init paths - ensure_worktree(): called at end (both create and respawn) - dispatch_once(): called for scratch/dir workspace kinds in both the main and review dispatcher call sites Tests: 11 new tests in tests/hermes_cli/test_kanban_db.py covering worktree/scratch/dir workspace kinds, hook rejection, hook allowance, opt-out behaviour, and non-git dir noop. Also patches test_ensure_worktree_respawn_reuses_existing to opt out of identity enforcement (test is about respawn semantics, not identity). Fixes recurring C5 auto-reviewer failure where workers committed as sahilm-ti instead of sahilm-ai (PR #27 recurrence of PR #23 pattern). Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
May 28, 2026
* design: post-approve merger agent (t_5a521a19) Add DESIGN.md covering: - New 'merging' kanban status between human_review and done - approve_task() PR-detection routing (human_review -> merging vs done) - New post-approve-merger profile and skill - Idempotency via gh pr view state as source of truth - All 6 PR-state branches and their outcomes - dispatch_once() merging column (parallel to review column) - Gateway notifier events - Open Q on dashboard column order for Sahil No code yet — approval gate. * feat(kanban): post-approve merger agent (t_5a521a19) When kanban_approve is called on a human_review task that has an associated PR URL, instead of transitioning directly to done, the task is claimed for a post-approve-merger worker which merges the PR and transitions to done/blocked. Changes: - kanban_db.py: add _extract_pr_url(), claim_merger_task(), update approve_task() to return (bool, outcome, pr_url, task) tuple and route PR-bearing tasks via claim_merger_task - kanban_tools.py: update _handle_approve() to spawn post-approve-merger worker when outcome=merge_triggered - kanban.py: update _cmd_approve() to spawn merger from CLI path too, print descriptive message for merge-triggered outcome - gateway/run.py: add merge_requested to TERMINAL_KINDS, add notifier message for merge_requested events - skills/devops/post-approve-merger/SKILL.md: new skill for the merger worker with full PR state-machine (6 branches), auth pattern, and idempotency rules - tests/hermes_cli/test_kanban_merging.py: new tests for _extract_pr_url, claim_merger_task, approve_task routing (15 test cases) - tests/hermes_cli/test_kanban_human_review.py: update existing tests for new approve_task tuple return type - DESIGN.md: updated to reflect no-merging-status decision No new VALID_STATUSES added. No new dispatcher column needed. The post-approve-merger profile lives at ~/.hermes/profiles/post-approve-merger/. * fix(skill): rewrite negations as positive imperatives in post-approve-merger SKILL.md S1 auto-review rejection fix: 3 negation-form lines rewritten as positive imperatives per sdlc-review rules. - 'Never use sahilm-ti credentials' -> 'Use sahilm-ai credentials exclusively' - 'Do NOT call kanban_review' -> 'The ONLY success terminator is kanban_complete' - 'After it, stop - do not attempt anything else' -> 'After the single terminal call, stop' --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com> Co-authored-by: sahilm-ai <sahilm.ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
May 28, 2026
…LL.md (#24) PR #23 had three negation-form directives rewritten as positive imperatives during the auto-reviewer pass, in a mistaken application of the BT-agent optimization-playbook S1 rule to a Hermes infrastructure skill. The S1/SD4 rules in sdlc-review have since been scoped BT-agent-only — but the rewrites in #23 landed before that scoping fix, so the load-bearing prohibitions are gone from the merged skill content. Restore them: 1. Auth: add explicit 'Never use sahilm-ti credentials' with the two concrete consequences (audit-trail misattribution + keychain prompt blocking the worker). The bare 'use sahilm-ai exclusively' positive form left the door open to drift, as evidenced by PR #23 itself — commit 7da3474 in that PR was authored as sahilm-ti. 2. Terminator contract: add explicit 'Do NOT call kanban_review' — without this prohibition the worker could re-loop the card through the auto-reviewer after Sahil has already approved, defeating the one-approval-equals-merged contract that motivated #23 in the first place. 3. Stop clause: 'After the single terminal call, stop — do not attempt any further gh/git/kanban_* operations.' A second terminal call corrupts the event log; the positive-only 'stop' form is too ambiguous (stop what? stop thinking? the model may interpret it as stop typing but keep tool-calling). No code changes. No test changes. Skill-content-only fix. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
May 28, 2026
…hook (#28) Part 1: Add pin_workspace_git_identity() to hermes_cli/kanban_db.py - Sets user.name/user.email in the workspace's local git config (survives subprocess shells, rebase resolution commits, auto-format follow-ups) - Installs a pre-commit hook in <workspace>/.hermes-hooks/ wired via core.hooksPath that hard-rejects commits whose GIT_AUTHOR_EMAIL does not match the configured worker identity - Opt-out: HERMES_KANBAN_ENFORCE_GIT_IDENTITY=false (env) or kanban.enforce_worker_git_identity: false (config.yaml) - Best-effort on non-git dirs: hook dir created; config write skipped Part 2: Call pin_workspace_git_identity from all workspace init paths - ensure_worktree(): called at end (both create and respawn) - dispatch_once(): called for scratch/dir workspace kinds in both the main and review dispatcher call sites Tests: 11 new tests in tests/hermes_cli/test_kanban_db.py covering worktree/scratch/dir workspace kinds, hook rejection, hook allowance, opt-out behaviour, and non-git dir noop. Also patches test_ensure_worktree_respawn_reuses_existing to opt out of identity enforcement (test is about respawn semantics, not identity). Fixes recurring C5 auto-reviewer failure where workers committed as sahilm-ti instead of sahilm-ai (PR #27 recurrence of PR #23 pattern). Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
May 28, 2026
* design: post-approve merger agent (t_5a521a19) Add DESIGN.md covering: - New 'merging' kanban status between human_review and done - approve_task() PR-detection routing (human_review -> merging vs done) - New post-approve-merger profile and skill - Idempotency via gh pr view state as source of truth - All 6 PR-state branches and their outcomes - dispatch_once() merging column (parallel to review column) - Gateway notifier events - Open Q on dashboard column order for Sahil No code yet — approval gate. * feat(kanban): post-approve merger agent (t_5a521a19) When kanban_approve is called on a human_review task that has an associated PR URL, instead of transitioning directly to done, the task is claimed for a post-approve-merger worker which merges the PR and transitions to done/blocked. Changes: - kanban_db.py: add _extract_pr_url(), claim_merger_task(), update approve_task() to return (bool, outcome, pr_url, task) tuple and route PR-bearing tasks via claim_merger_task - kanban_tools.py: update _handle_approve() to spawn post-approve-merger worker when outcome=merge_triggered - kanban.py: update _cmd_approve() to spawn merger from CLI path too, print descriptive message for merge-triggered outcome - gateway/run.py: add merge_requested to TERMINAL_KINDS, add notifier message for merge_requested events - skills/devops/post-approve-merger/SKILL.md: new skill for the merger worker with full PR state-machine (6 branches), auth pattern, and idempotency rules - tests/hermes_cli/test_kanban_merging.py: new tests for _extract_pr_url, claim_merger_task, approve_task routing (15 test cases) - tests/hermes_cli/test_kanban_human_review.py: update existing tests for new approve_task tuple return type - DESIGN.md: updated to reflect no-merging-status decision No new VALID_STATUSES added. No new dispatcher column needed. The post-approve-merger profile lives at ~/.hermes/profiles/post-approve-merger/. * fix(skill): rewrite negations as positive imperatives in post-approve-merger SKILL.md S1 auto-review rejection fix: 3 negation-form lines rewritten as positive imperatives per sdlc-review rules. - 'Never use sahilm-ti credentials' -> 'Use sahilm-ai credentials exclusively' - 'Do NOT call kanban_review' -> 'The ONLY success terminator is kanban_complete' - 'After it, stop - do not attempt anything else' -> 'After the single terminal call, stop' --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com> Co-authored-by: sahilm-ai <sahilm.ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
May 28, 2026
…LL.md (#24) PR #23 had three negation-form directives rewritten as positive imperatives during the auto-reviewer pass, in a mistaken application of the BT-agent optimization-playbook S1 rule to a Hermes infrastructure skill. The S1/SD4 rules in sdlc-review have since been scoped BT-agent-only — but the rewrites in #23 landed before that scoping fix, so the load-bearing prohibitions are gone from the merged skill content. Restore them: 1. Auth: add explicit 'Never use sahilm-ti credentials' with the two concrete consequences (audit-trail misattribution + keychain prompt blocking the worker). The bare 'use sahilm-ai exclusively' positive form left the door open to drift, as evidenced by PR #23 itself — commit 7da3474 in that PR was authored as sahilm-ti. 2. Terminator contract: add explicit 'Do NOT call kanban_review' — without this prohibition the worker could re-loop the card through the auto-reviewer after Sahil has already approved, defeating the one-approval-equals-merged contract that motivated #23 in the first place. 3. Stop clause: 'After the single terminal call, stop — do not attempt any further gh/git/kanban_* operations.' A second terminal call corrupts the event log; the positive-only 'stop' form is too ambiguous (stop what? stop thinking? the model may interpret it as stop typing but keep tool-calling). No code changes. No test changes. Skill-content-only fix. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
May 28, 2026
…hook (#28) Part 1: Add pin_workspace_git_identity() to hermes_cli/kanban_db.py - Sets user.name/user.email in the workspace's local git config (survives subprocess shells, rebase resolution commits, auto-format follow-ups) - Installs a pre-commit hook in <workspace>/.hermes-hooks/ wired via core.hooksPath that hard-rejects commits whose GIT_AUTHOR_EMAIL does not match the configured worker identity - Opt-out: HERMES_KANBAN_ENFORCE_GIT_IDENTITY=false (env) or kanban.enforce_worker_git_identity: false (config.yaml) - Best-effort on non-git dirs: hook dir created; config write skipped Part 2: Call pin_workspace_git_identity from all workspace init paths - ensure_worktree(): called at end (both create and respawn) - dispatch_once(): called for scratch/dir workspace kinds in both the main and review dispatcher call sites Tests: 11 new tests in tests/hermes_cli/test_kanban_db.py covering worktree/scratch/dir workspace kinds, hook rejection, hook allowance, opt-out behaviour, and non-git dir noop. Also patches test_ensure_worktree_respawn_reuses_existing to opt out of identity enforcement (test is about respawn semantics, not identity). Fixes recurring C5 auto-reviewer failure where workers committed as sahilm-ti instead of sahilm-ai (PR #27 recurrence of PR #23 pattern). Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
May 29, 2026
* design: post-approve merger agent (t_5a521a19) Add DESIGN.md covering: - New 'merging' kanban status between human_review and done - approve_task() PR-detection routing (human_review -> merging vs done) - New post-approve-merger profile and skill - Idempotency via gh pr view state as source of truth - All 6 PR-state branches and their outcomes - dispatch_once() merging column (parallel to review column) - Gateway notifier events - Open Q on dashboard column order for Sahil No code yet — approval gate. * feat(kanban): post-approve merger agent (t_5a521a19) When kanban_approve is called on a human_review task that has an associated PR URL, instead of transitioning directly to done, the task is claimed for a post-approve-merger worker which merges the PR and transitions to done/blocked. Changes: - kanban_db.py: add _extract_pr_url(), claim_merger_task(), update approve_task() to return (bool, outcome, pr_url, task) tuple and route PR-bearing tasks via claim_merger_task - kanban_tools.py: update _handle_approve() to spawn post-approve-merger worker when outcome=merge_triggered - kanban.py: update _cmd_approve() to spawn merger from CLI path too, print descriptive message for merge-triggered outcome - gateway/run.py: add merge_requested to TERMINAL_KINDS, add notifier message for merge_requested events - skills/devops/post-approve-merger/SKILL.md: new skill for the merger worker with full PR state-machine (6 branches), auth pattern, and idempotency rules - tests/hermes_cli/test_kanban_merging.py: new tests for _extract_pr_url, claim_merger_task, approve_task routing (15 test cases) - tests/hermes_cli/test_kanban_human_review.py: update existing tests for new approve_task tuple return type - DESIGN.md: updated to reflect no-merging-status decision No new VALID_STATUSES added. No new dispatcher column needed. The post-approve-merger profile lives at ~/.hermes/profiles/post-approve-merger/. * fix(skill): rewrite negations as positive imperatives in post-approve-merger SKILL.md S1 auto-review rejection fix: 3 negation-form lines rewritten as positive imperatives per sdlc-review rules. - 'Never use sahilm-ti credentials' -> 'Use sahilm-ai credentials exclusively' - 'Do NOT call kanban_review' -> 'The ONLY success terminator is kanban_complete' - 'After it, stop - do not attempt anything else' -> 'After the single terminal call, stop' --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com> Co-authored-by: sahilm-ai <sahilm.ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
May 29, 2026
…LL.md (#24) PR #23 had three negation-form directives rewritten as positive imperatives during the auto-reviewer pass, in a mistaken application of the BT-agent optimization-playbook S1 rule to a Hermes infrastructure skill. The S1/SD4 rules in sdlc-review have since been scoped BT-agent-only — but the rewrites in #23 landed before that scoping fix, so the load-bearing prohibitions are gone from the merged skill content. Restore them: 1. Auth: add explicit 'Never use sahilm-ti credentials' with the two concrete consequences (audit-trail misattribution + keychain prompt blocking the worker). The bare 'use sahilm-ai exclusively' positive form left the door open to drift, as evidenced by PR #23 itself — commit 7da3474 in that PR was authored as sahilm-ti. 2. Terminator contract: add explicit 'Do NOT call kanban_review' — without this prohibition the worker could re-loop the card through the auto-reviewer after Sahil has already approved, defeating the one-approval-equals-merged contract that motivated #23 in the first place. 3. Stop clause: 'After the single terminal call, stop — do not attempt any further gh/git/kanban_* operations.' A second terminal call corrupts the event log; the positive-only 'stop' form is too ambiguous (stop what? stop thinking? the model may interpret it as stop typing but keep tool-calling). No code changes. No test changes. Skill-content-only fix. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
May 29, 2026
…hook (#28) Part 1: Add pin_workspace_git_identity() to hermes_cli/kanban_db.py - Sets user.name/user.email in the workspace's local git config (survives subprocess shells, rebase resolution commits, auto-format follow-ups) - Installs a pre-commit hook in <workspace>/.hermes-hooks/ wired via core.hooksPath that hard-rejects commits whose GIT_AUTHOR_EMAIL does not match the configured worker identity - Opt-out: HERMES_KANBAN_ENFORCE_GIT_IDENTITY=false (env) or kanban.enforce_worker_git_identity: false (config.yaml) - Best-effort on non-git dirs: hook dir created; config write skipped Part 2: Call pin_workspace_git_identity from all workspace init paths - ensure_worktree(): called at end (both create and respawn) - dispatch_once(): called for scratch/dir workspace kinds in both the main and review dispatcher call sites Tests: 11 new tests in tests/hermes_cli/test_kanban_db.py covering worktree/scratch/dir workspace kinds, hook rejection, hook allowance, opt-out behaviour, and non-git dir noop. Also patches test_ensure_worktree_respawn_reuses_existing to opt out of identity enforcement (test is about respawn semantics, not identity). Fixes recurring C5 auto-reviewer failure where workers committed as sahilm-ti instead of sahilm-ai (PR #27 recurrence of PR #23 pattern). Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
Jul 3, 2026
* design: post-approve merger agent (t_5a521a19) Add DESIGN.md covering: - New 'merging' kanban status between human_review and done - approve_task() PR-detection routing (human_review -> merging vs done) - New post-approve-merger profile and skill - Idempotency via gh pr view state as source of truth - All 6 PR-state branches and their outcomes - dispatch_once() merging column (parallel to review column) - Gateway notifier events - Open Q on dashboard column order for Sahil No code yet — approval gate. * feat(kanban): post-approve merger agent (t_5a521a19) When kanban_approve is called on a human_review task that has an associated PR URL, instead of transitioning directly to done, the task is claimed for a post-approve-merger worker which merges the PR and transitions to done/blocked. Changes: - kanban_db.py: add _extract_pr_url(), claim_merger_task(), update approve_task() to return (bool, outcome, pr_url, task) tuple and route PR-bearing tasks via claim_merger_task - kanban_tools.py: update _handle_approve() to spawn post-approve-merger worker when outcome=merge_triggered - kanban.py: update _cmd_approve() to spawn merger from CLI path too, print descriptive message for merge-triggered outcome - gateway/run.py: add merge_requested to TERMINAL_KINDS, add notifier message for merge_requested events - skills/devops/post-approve-merger/SKILL.md: new skill for the merger worker with full PR state-machine (6 branches), auth pattern, and idempotency rules - tests/hermes_cli/test_kanban_merging.py: new tests for _extract_pr_url, claim_merger_task, approve_task routing (15 test cases) - tests/hermes_cli/test_kanban_human_review.py: update existing tests for new approve_task tuple return type - DESIGN.md: updated to reflect no-merging-status decision No new VALID_STATUSES added. No new dispatcher column needed. The post-approve-merger profile lives at ~/.hermes/profiles/post-approve-merger/. * fix(skill): rewrite negations as positive imperatives in post-approve-merger SKILL.md S1 auto-review rejection fix: 3 negation-form lines rewritten as positive imperatives per sdlc-review rules. - 'Never use sahilm-ti credentials' -> 'Use sahilm-ai credentials exclusively' - 'Do NOT call kanban_review' -> 'The ONLY success terminator is kanban_complete' - 'After it, stop - do not attempt anything else' -> 'After the single terminal call, stop' --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com> Co-authored-by: sahilm-ai <sahilm.ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
Jul 3, 2026
…LL.md (#24) PR #23 had three negation-form directives rewritten as positive imperatives during the auto-reviewer pass, in a mistaken application of the BT-agent optimization-playbook S1 rule to a Hermes infrastructure skill. The S1/SD4 rules in sdlc-review have since been scoped BT-agent-only — but the rewrites in #23 landed before that scoping fix, so the load-bearing prohibitions are gone from the merged skill content. Restore them: 1. Auth: add explicit 'Never use sahilm-ti credentials' with the two concrete consequences (audit-trail misattribution + keychain prompt blocking the worker). The bare 'use sahilm-ai exclusively' positive form left the door open to drift, as evidenced by PR #23 itself — commit 7da3474 in that PR was authored as sahilm-ti. 2. Terminator contract: add explicit 'Do NOT call kanban_review' — without this prohibition the worker could re-loop the card through the auto-reviewer after Sahil has already approved, defeating the one-approval-equals-merged contract that motivated #23 in the first place. 3. Stop clause: 'After the single terminal call, stop — do not attempt any further gh/git/kanban_* operations.' A second terminal call corrupts the event log; the positive-only 'stop' form is too ambiguous (stop what? stop thinking? the model may interpret it as stop typing but keep tool-calling). No code changes. No test changes. Skill-content-only fix. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 3, 2026
…hook (#28) Part 1: Add pin_workspace_git_identity() to hermes_cli/kanban_db.py - Sets user.name/user.email in the workspace's local git config (survives subprocess shells, rebase resolution commits, auto-format follow-ups) - Installs a pre-commit hook in <workspace>/.hermes-hooks/ wired via core.hooksPath that hard-rejects commits whose GIT_AUTHOR_EMAIL does not match the configured worker identity - Opt-out: HERMES_KANBAN_ENFORCE_GIT_IDENTITY=false (env) or kanban.enforce_worker_git_identity: false (config.yaml) - Best-effort on non-git dirs: hook dir created; config write skipped Part 2: Call pin_workspace_git_identity from all workspace init paths - ensure_worktree(): called at end (both create and respawn) - dispatch_once(): called for scratch/dir workspace kinds in both the main and review dispatcher call sites Tests: 11 new tests in tests/hermes_cli/test_kanban_db.py covering worktree/scratch/dir workspace kinds, hook rejection, hook allowance, opt-out behaviour, and non-git dir noop. Also patches test_ensure_worktree_respawn_reuses_existing to opt out of identity enforcement (test is about respawn semantics, not identity). Fixes recurring C5 auto-reviewer failure where workers committed as sahilm-ti instead of sahilm-ai (PR #27 recurrence of PR #23 pattern). Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
Jul 9, 2026
* design: post-approve merger agent (t_5a521a19) Add DESIGN.md covering: - New 'merging' kanban status between human_review and done - approve_task() PR-detection routing (human_review -> merging vs done) - New post-approve-merger profile and skill - Idempotency via gh pr view state as source of truth - All 6 PR-state branches and their outcomes - dispatch_once() merging column (parallel to review column) - Gateway notifier events - Open Q on dashboard column order for Sahil No code yet — approval gate. * feat(kanban): post-approve merger agent (t_5a521a19) When kanban_approve is called on a human_review task that has an associated PR URL, instead of transitioning directly to done, the task is claimed for a post-approve-merger worker which merges the PR and transitions to done/blocked. Changes: - kanban_db.py: add _extract_pr_url(), claim_merger_task(), update approve_task() to return (bool, outcome, pr_url, task) tuple and route PR-bearing tasks via claim_merger_task - kanban_tools.py: update _handle_approve() to spawn post-approve-merger worker when outcome=merge_triggered - kanban.py: update _cmd_approve() to spawn merger from CLI path too, print descriptive message for merge-triggered outcome - gateway/run.py: add merge_requested to TERMINAL_KINDS, add notifier message for merge_requested events - skills/devops/post-approve-merger/SKILL.md: new skill for the merger worker with full PR state-machine (6 branches), auth pattern, and idempotency rules - tests/hermes_cli/test_kanban_merging.py: new tests for _extract_pr_url, claim_merger_task, approve_task routing (15 test cases) - tests/hermes_cli/test_kanban_human_review.py: update existing tests for new approve_task tuple return type - DESIGN.md: updated to reflect no-merging-status decision No new VALID_STATUSES added. No new dispatcher column needed. The post-approve-merger profile lives at ~/.hermes/profiles/post-approve-merger/. * fix(skill): rewrite negations as positive imperatives in post-approve-merger SKILL.md S1 auto-review rejection fix: 3 negation-form lines rewritten as positive imperatives per sdlc-review rules. - 'Never use sahilm-ti credentials' -> 'Use sahilm-ai credentials exclusively' - 'Do NOT call kanban_review' -> 'The ONLY success terminator is kanban_complete' - 'After it, stop - do not attempt anything else' -> 'After the single terminal call, stop' --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com> Co-authored-by: sahilm-ai <sahilm.ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
Jul 9, 2026
…LL.md (#24) PR #23 had three negation-form directives rewritten as positive imperatives during the auto-reviewer pass, in a mistaken application of the BT-agent optimization-playbook S1 rule to a Hermes infrastructure skill. The S1/SD4 rules in sdlc-review have since been scoped BT-agent-only — but the rewrites in #23 landed before that scoping fix, so the load-bearing prohibitions are gone from the merged skill content. Restore them: 1. Auth: add explicit 'Never use sahilm-ti credentials' with the two concrete consequences (audit-trail misattribution + keychain prompt blocking the worker). The bare 'use sahilm-ai exclusively' positive form left the door open to drift, as evidenced by PR #23 itself — commit 7da3474 in that PR was authored as sahilm-ti. 2. Terminator contract: add explicit 'Do NOT call kanban_review' — without this prohibition the worker could re-loop the card through the auto-reviewer after Sahil has already approved, defeating the one-approval-equals-merged contract that motivated #23 in the first place. 3. Stop clause: 'After the single terminal call, stop — do not attempt any further gh/git/kanban_* operations.' A second terminal call corrupts the event log; the positive-only 'stop' form is too ambiguous (stop what? stop thinking? the model may interpret it as stop typing but keep tool-calling). No code changes. No test changes. Skill-content-only fix. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 9, 2026
…hook (#28) Part 1: Add pin_workspace_git_identity() to hermes_cli/kanban_db.py - Sets user.name/user.email in the workspace's local git config (survives subprocess shells, rebase resolution commits, auto-format follow-ups) - Installs a pre-commit hook in <workspace>/.hermes-hooks/ wired via core.hooksPath that hard-rejects commits whose GIT_AUTHOR_EMAIL does not match the configured worker identity - Opt-out: HERMES_KANBAN_ENFORCE_GIT_IDENTITY=false (env) or kanban.enforce_worker_git_identity: false (config.yaml) - Best-effort on non-git dirs: hook dir created; config write skipped Part 2: Call pin_workspace_git_identity from all workspace init paths - ensure_worktree(): called at end (both create and respawn) - dispatch_once(): called for scratch/dir workspace kinds in both the main and review dispatcher call sites Tests: 11 new tests in tests/hermes_cli/test_kanban_db.py covering worktree/scratch/dir workspace kinds, hook rejection, hook allowance, opt-out behaviour, and non-git dir noop. Also patches test_ensure_worktree_respawn_reuses_existing to opt out of identity enforcement (test is about respawn semantics, not identity). Fixes recurring C5 auto-reviewer failure where workers committed as sahilm-ti instead of sahilm-ai (PR #27 recurrence of PR #23 pattern). Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
Jul 10, 2026
* design: post-approve merger agent (t_5a521a19) Add DESIGN.md covering: - New 'merging' kanban status between human_review and done - approve_task() PR-detection routing (human_review -> merging vs done) - New post-approve-merger profile and skill - Idempotency via gh pr view state as source of truth - All 6 PR-state branches and their outcomes - dispatch_once() merging column (parallel to review column) - Gateway notifier events - Open Q on dashboard column order for Sahil No code yet — approval gate. * feat(kanban): post-approve merger agent (t_5a521a19) When kanban_approve is called on a human_review task that has an associated PR URL, instead of transitioning directly to done, the task is claimed for a post-approve-merger worker which merges the PR and transitions to done/blocked. Changes: - kanban_db.py: add _extract_pr_url(), claim_merger_task(), update approve_task() to return (bool, outcome, pr_url, task) tuple and route PR-bearing tasks via claim_merger_task - kanban_tools.py: update _handle_approve() to spawn post-approve-merger worker when outcome=merge_triggered - kanban.py: update _cmd_approve() to spawn merger from CLI path too, print descriptive message for merge-triggered outcome - gateway/run.py: add merge_requested to TERMINAL_KINDS, add notifier message for merge_requested events - skills/devops/post-approve-merger/SKILL.md: new skill for the merger worker with full PR state-machine (6 branches), auth pattern, and idempotency rules - tests/hermes_cli/test_kanban_merging.py: new tests for _extract_pr_url, claim_merger_task, approve_task routing (15 test cases) - tests/hermes_cli/test_kanban_human_review.py: update existing tests for new approve_task tuple return type - DESIGN.md: updated to reflect no-merging-status decision No new VALID_STATUSES added. No new dispatcher column needed. The post-approve-merger profile lives at ~/.hermes/profiles/post-approve-merger/. * fix(skill): rewrite negations as positive imperatives in post-approve-merger SKILL.md S1 auto-review rejection fix: 3 negation-form lines rewritten as positive imperatives per sdlc-review rules. - 'Never use sahilm-ti credentials' -> 'Use sahilm-ai credentials exclusively' - 'Do NOT call kanban_review' -> 'The ONLY success terminator is kanban_complete' - 'After it, stop - do not attempt anything else' -> 'After the single terminal call, stop' --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com> Co-authored-by: sahilm-ai <sahilm.ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
Jul 10, 2026
…LL.md (#24) PR #23 had three negation-form directives rewritten as positive imperatives during the auto-reviewer pass, in a mistaken application of the BT-agent optimization-playbook S1 rule to a Hermes infrastructure skill. The S1/SD4 rules in sdlc-review have since been scoped BT-agent-only — but the rewrites in #23 landed before that scoping fix, so the load-bearing prohibitions are gone from the merged skill content. Restore them: 1. Auth: add explicit 'Never use sahilm-ti credentials' with the two concrete consequences (audit-trail misattribution + keychain prompt blocking the worker). The bare 'use sahilm-ai exclusively' positive form left the door open to drift, as evidenced by PR #23 itself — commit 7da3474 in that PR was authored as sahilm-ti. 2. Terminator contract: add explicit 'Do NOT call kanban_review' — without this prohibition the worker could re-loop the card through the auto-reviewer after Sahil has already approved, defeating the one-approval-equals-merged contract that motivated #23 in the first place. 3. Stop clause: 'After the single terminal call, stop — do not attempt any further gh/git/kanban_* operations.' A second terminal call corrupts the event log; the positive-only 'stop' form is too ambiguous (stop what? stop thinking? the model may interpret it as stop typing but keep tool-calling). No code changes. No test changes. Skill-content-only fix. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 10, 2026
…hook (#28) Part 1: Add pin_workspace_git_identity() to hermes_cli/kanban_db.py - Sets user.name/user.email in the workspace's local git config (survives subprocess shells, rebase resolution commits, auto-format follow-ups) - Installs a pre-commit hook in <workspace>/.hermes-hooks/ wired via core.hooksPath that hard-rejects commits whose GIT_AUTHOR_EMAIL does not match the configured worker identity - Opt-out: HERMES_KANBAN_ENFORCE_GIT_IDENTITY=false (env) or kanban.enforce_worker_git_identity: false (config.yaml) - Best-effort on non-git dirs: hook dir created; config write skipped Part 2: Call pin_workspace_git_identity from all workspace init paths - ensure_worktree(): called at end (both create and respawn) - dispatch_once(): called for scratch/dir workspace kinds in both the main and review dispatcher call sites Tests: 11 new tests in tests/hermes_cli/test_kanban_db.py covering worktree/scratch/dir workspace kinds, hook rejection, hook allowance, opt-out behaviour, and non-git dir noop. Also patches test_ensure_worktree_respawn_reuses_existing to opt out of identity enforcement (test is about respawn semantics, not identity). Fixes recurring C5 auto-reviewer failure where workers committed as sahilm-ti instead of sahilm-ai (PR #27 recurrence of PR #23 pattern). Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
Jul 11, 2026
* design: post-approve merger agent (t_5a521a19) Add DESIGN.md covering: - New 'merging' kanban status between human_review and done - approve_task() PR-detection routing (human_review -> merging vs done) - New post-approve-merger profile and skill - Idempotency via gh pr view state as source of truth - All 6 PR-state branches and their outcomes - dispatch_once() merging column (parallel to review column) - Gateway notifier events - Open Q on dashboard column order for Sahil No code yet — approval gate. * feat(kanban): post-approve merger agent (t_5a521a19) When kanban_approve is called on a human_review task that has an associated PR URL, instead of transitioning directly to done, the task is claimed for a post-approve-merger worker which merges the PR and transitions to done/blocked. Changes: - kanban_db.py: add _extract_pr_url(), claim_merger_task(), update approve_task() to return (bool, outcome, pr_url, task) tuple and route PR-bearing tasks via claim_merger_task - kanban_tools.py: update _handle_approve() to spawn post-approve-merger worker when outcome=merge_triggered - kanban.py: update _cmd_approve() to spawn merger from CLI path too, print descriptive message for merge-triggered outcome - gateway/run.py: add merge_requested to TERMINAL_KINDS, add notifier message for merge_requested events - skills/devops/post-approve-merger/SKILL.md: new skill for the merger worker with full PR state-machine (6 branches), auth pattern, and idempotency rules - tests/hermes_cli/test_kanban_merging.py: new tests for _extract_pr_url, claim_merger_task, approve_task routing (15 test cases) - tests/hermes_cli/test_kanban_human_review.py: update existing tests for new approve_task tuple return type - DESIGN.md: updated to reflect no-merging-status decision No new VALID_STATUSES added. No new dispatcher column needed. The post-approve-merger profile lives at ~/.hermes/profiles/post-approve-merger/. * fix(skill): rewrite negations as positive imperatives in post-approve-merger SKILL.md S1 auto-review rejection fix: 3 negation-form lines rewritten as positive imperatives per sdlc-review rules. - 'Never use sahilm-ti credentials' -> 'Use sahilm-ai credentials exclusively' - 'Do NOT call kanban_review' -> 'The ONLY success terminator is kanban_complete' - 'After it, stop - do not attempt anything else' -> 'After the single terminal call, stop' --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com> Co-authored-by: sahilm-ai <sahilm.ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
Jul 11, 2026
…LL.md (#24) PR #23 had three negation-form directives rewritten as positive imperatives during the auto-reviewer pass, in a mistaken application of the BT-agent optimization-playbook S1 rule to a Hermes infrastructure skill. The S1/SD4 rules in sdlc-review have since been scoped BT-agent-only — but the rewrites in #23 landed before that scoping fix, so the load-bearing prohibitions are gone from the merged skill content. Restore them: 1. Auth: add explicit 'Never use sahilm-ti credentials' with the two concrete consequences (audit-trail misattribution + keychain prompt blocking the worker). The bare 'use sahilm-ai exclusively' positive form left the door open to drift, as evidenced by PR #23 itself — commit 7da3474 in that PR was authored as sahilm-ti. 2. Terminator contract: add explicit 'Do NOT call kanban_review' — without this prohibition the worker could re-loop the card through the auto-reviewer after Sahil has already approved, defeating the one-approval-equals-merged contract that motivated #23 in the first place. 3. Stop clause: 'After the single terminal call, stop — do not attempt any further gh/git/kanban_* operations.' A second terminal call corrupts the event log; the positive-only 'stop' form is too ambiguous (stop what? stop thinking? the model may interpret it as stop typing but keep tool-calling). No code changes. No test changes. Skill-content-only fix. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 11, 2026
…hook (#28) Part 1: Add pin_workspace_git_identity() to hermes_cli/kanban_db.py - Sets user.name/user.email in the workspace's local git config (survives subprocess shells, rebase resolution commits, auto-format follow-ups) - Installs a pre-commit hook in <workspace>/.hermes-hooks/ wired via core.hooksPath that hard-rejects commits whose GIT_AUTHOR_EMAIL does not match the configured worker identity - Opt-out: HERMES_KANBAN_ENFORCE_GIT_IDENTITY=false (env) or kanban.enforce_worker_git_identity: false (config.yaml) - Best-effort on non-git dirs: hook dir created; config write skipped Part 2: Call pin_workspace_git_identity from all workspace init paths - ensure_worktree(): called at end (both create and respawn) - dispatch_once(): called for scratch/dir workspace kinds in both the main and review dispatcher call sites Tests: 11 new tests in tests/hermes_cli/test_kanban_db.py covering worktree/scratch/dir workspace kinds, hook rejection, hook allowance, opt-out behaviour, and non-git dir noop. Also patches test_ensure_worktree_respawn_reuses_existing to opt out of identity enforcement (test is about respawn semantics, not identity). Fixes recurring C5 auto-reviewer failure where workers committed as sahilm-ti instead of sahilm-ai (PR #27 recurrence of PR #23 pattern). Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
This was referenced Jul 13, 2026
sahilm-ti
added a commit
that referenced
this pull request
Jul 13, 2026
* design: post-approve merger agent (t_5a521a19) Add DESIGN.md covering: - New 'merging' kanban status between human_review and done - approve_task() PR-detection routing (human_review -> merging vs done) - New post-approve-merger profile and skill - Idempotency via gh pr view state as source of truth - All 6 PR-state branches and their outcomes - dispatch_once() merging column (parallel to review column) - Gateway notifier events - Open Q on dashboard column order for Sahil No code yet — approval gate. * feat(kanban): post-approve merger agent (t_5a521a19) When kanban_approve is called on a human_review task that has an associated PR URL, instead of transitioning directly to done, the task is claimed for a post-approve-merger worker which merges the PR and transitions to done/blocked. Changes: - kanban_db.py: add _extract_pr_url(), claim_merger_task(), update approve_task() to return (bool, outcome, pr_url, task) tuple and route PR-bearing tasks via claim_merger_task - kanban_tools.py: update _handle_approve() to spawn post-approve-merger worker when outcome=merge_triggered - kanban.py: update _cmd_approve() to spawn merger from CLI path too, print descriptive message for merge-triggered outcome - gateway/run.py: add merge_requested to TERMINAL_KINDS, add notifier message for merge_requested events - skills/devops/post-approve-merger/SKILL.md: new skill for the merger worker with full PR state-machine (6 branches), auth pattern, and idempotency rules - tests/hermes_cli/test_kanban_merging.py: new tests for _extract_pr_url, claim_merger_task, approve_task routing (15 test cases) - tests/hermes_cli/test_kanban_human_review.py: update existing tests for new approve_task tuple return type - DESIGN.md: updated to reflect no-merging-status decision No new VALID_STATUSES added. No new dispatcher column needed. The post-approve-merger profile lives at ~/.hermes/profiles/post-approve-merger/. * fix(skill): rewrite negations as positive imperatives in post-approve-merger SKILL.md S1 auto-review rejection fix: 3 negation-form lines rewritten as positive imperatives per sdlc-review rules. - 'Never use sahilm-ti credentials' -> 'Use sahilm-ai credentials exclusively' - 'Do NOT call kanban_review' -> 'The ONLY success terminator is kanban_complete' - 'After it, stop - do not attempt anything else' -> 'After the single terminal call, stop' --------- Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com> Co-authored-by: sahilm-ai <sahilm.ai@users.noreply.github.com>
sahilm-ti
added a commit
that referenced
this pull request
Jul 13, 2026
…LL.md (#24) PR #23 had three negation-form directives rewritten as positive imperatives during the auto-reviewer pass, in a mistaken application of the BT-agent optimization-playbook S1 rule to a Hermes infrastructure skill. The S1/SD4 rules in sdlc-review have since been scoped BT-agent-only — but the rewrites in #23 landed before that scoping fix, so the load-bearing prohibitions are gone from the merged skill content. Restore them: 1. Auth: add explicit 'Never use sahilm-ti credentials' with the two concrete consequences (audit-trail misattribution + keychain prompt blocking the worker). The bare 'use sahilm-ai exclusively' positive form left the door open to drift, as evidenced by PR #23 itself — commit 7da3474 in that PR was authored as sahilm-ti. 2. Terminator contract: add explicit 'Do NOT call kanban_review' — without this prohibition the worker could re-loop the card through the auto-reviewer after Sahil has already approved, defeating the one-approval-equals-merged contract that motivated #23 in the first place. 3. Stop clause: 'After the single terminal call, stop — do not attempt any further gh/git/kanban_* operations.' A second terminal call corrupts the event log; the positive-only 'stop' form is too ambiguous (stop what? stop thinking? the model may interpret it as stop typing but keep tool-calling). No code changes. No test changes. Skill-content-only fix. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 13, 2026
…hook (#28) Part 1: Add pin_workspace_git_identity() to hermes_cli/kanban_db.py - Sets user.name/user.email in the workspace's local git config (survives subprocess shells, rebase resolution commits, auto-format follow-ups) - Installs a pre-commit hook in <workspace>/.hermes-hooks/ wired via core.hooksPath that hard-rejects commits whose GIT_AUTHOR_EMAIL does not match the configured worker identity - Opt-out: HERMES_KANBAN_ENFORCE_GIT_IDENTITY=false (env) or kanban.enforce_worker_git_identity: false (config.yaml) - Best-effort on non-git dirs: hook dir created; config write skipped Part 2: Call pin_workspace_git_identity from all workspace init paths - ensure_worktree(): called at end (both create and respawn) - dispatch_once(): called for scratch/dir workspace kinds in both the main and review dispatcher call sites Tests: 11 new tests in tests/hermes_cli/test_kanban_db.py covering worktree/scratch/dir workspace kinds, hook rejection, hook allowance, opt-out behaviour, and non-git dir noop. Also patches test_ensure_worktree_respawn_reuses_existing to opt out of identity enforcement (test is about respawn semantics, not identity). Fixes recurring C5 auto-reviewer failure where workers committed as sahilm-ti instead of sahilm-ai (PR #27 recurrence of PR #23 pattern). Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wire a post-approve merger agent into the kanban lifecycle. When
kanban_approveis called on ahuman_reviewtask that has an associated PR, the PR is automatically merged (with judgment for edge cases) instead of leaving the merge as a separate manual step.Motivating incident: PR #22 was "approved on kanban" but not merged on GitHub, causing the skills_sync fix to sit undeployed while the dispatcher kept crashing workers.
Design (no new status, no new dashboard column)
Per Sahil's decision: the task stays in
human_reviewwhile the merger works, then transitions directly todoneorblocked.approve_task()detects a PR URL (events → comments fallback)claim_merger_task()moveshuman_review → running, spawns thepost-approve-mergerworker inlinehuman_review → done(unchanged behavior)Changes
hermes_cli/kanban_db.py_extract_pr_url(),claim_merger_task(), updatedapprove_task()→ tuple returntools/kanban_tools.py_handle_approve()spawns merger on merge_triggeredhermes_cli/kanban.py_cmd_approve()spawns merger from CLI, prints descriptive messagegateway/run.pymerge_requestedadded to notifier TERMINAL_KINDS + messageskills/devops/post-approve-merger/SKILL.mdtests/hermes_cli/test_kanban_merging.pytests/hermes_cli/test_kanban_human_review.pyDESIGN.mdTest evidence
Acceptance criteria checklist
_extract_pr_url: event path, comment path, no-URL path testedclaim_merger_task: human_review → running, merge_requested event, idempotency testedapprove_taskroutes correctly: PR → merge_triggered, no-PR → donepost-approve-mergerskill covers all 6 PR-state branches~/.hermes/profiles/post-approve-merger/config.yamlFixes the motivating incident:
kanban_approvenow guarantees the PR lands before the card is done.Summary by CodeRabbit
Release Notes
New Features
Documentation