forked from Neptune/mirror.kickstart.nvim
Release v1.30.0 — Workflow Call/Return & Agent Delegation #56
Mike Bros
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "release/1.30.0"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Release v1.30.0 — Workflow Call/Return & Agent Delegation
Changes
PR Review Fixes
Checklist
References
Version: v1.30.0 (Gravity PM ID: 154)
Add ### {Project Name} — v{version} header to cross-skill guidance output blocks in gr-refinement, gr-pr-review, and gr-pr-followup. These three skills were the only ones missing the standard header that was already present in gr-preflight, gr-implementation, gr-postflight, and gr-release. Skills invoked outside a version context (single-ticket refinement, ad-hoc PR review/followup) show project name only without version. Closes OP#2325 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>Add call-result-capture.md reference documenting: result file path convention (.gravity/results/{call_id}.result.json), called phase write protocol, conductor read and validation logic, missing file handling (synthetic error envelope), timeout handling, validation checks table, and cleanup after consumption. Closes OP#2337 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>Document agent target resolution (agent:{name} syntax), command construction from invoke templates, input validation against schemas, headless Claude and local CLI backends, output capture with envelope wrapping, failure handling, and backend extension point interface. Closes OP#2349 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>Define the handoff envelope schema at .gravity/handoffs/{call_id}.handoff.json for passing context between call/dispatch loop iterations. Documents field reference, lifecycle (write/read/cleanup), and relationship to result envelopes and heartbeat protocol. Closes OP#2475 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>PR Review
Verdict: Acceptable
This review was conducted from a fresh context against the current branch state (31 files, ~7900 lines added). First review cycle — no prior reviews or followups exist.
Findings
Test Results
Human Review Items
3 items flagged for human review — see items marked with HUMAN REVIEW below.
Key Findings
request_idformat between SKILL.md examples and conductor spec — multiple filesvalidate-goto-schema.shregex for ISO 8601 duration rejects valid durations — validate-goto-schema.sh:133agents.yamlinvoke templates — agents.yaml / SKILL.md(Grouped by file below)
configs/bin/git-cleanup
[MINOR] Lines 95-130 (
show_summary): Function is ~80 lines with mixed formatting, data collection, and sorting logic. Consider extracting the branch-collection loop and sort into a helper function (e.g.,collect_and_sort_branches). The worktree display section (lines 140-180) could similarly be extracted.[MINOR] Lines 295-330 (
cleanup_interactive): Stale worktree pruning logic is duplicated betweencleanup_interactiveandprune_stale_worktrees. The detection loop (lines 295-310 and 355-370) is nearly identical. Consider extractingdetect_stale_worktreesinto a shared function and calling it from both.[NITPICK] The script uses
printf '%b\n'inusage()butprintfwith direct format strings elsewhere. Consistent use of one style would improve readability.configs/claude/skills/gr-gravity/agents.yaml
[IMPORTANT] The
invoketemplates for AI agents use single-quote wrapping around{input}:If the serialized JSON input contains single quotes (e.g., in string values passed as params), the shell command will break or — worse — interpret unescaped content. The conductor's command construction section in SKILL.md does not document how single quotes in param values are escaped before substitution. This is a shell injection surface for local-cli agents (the
schema-validatorinvoke template passes{input}directly as a file path argument:npx ajv validate -s {schema_path} -d {input}).Suggested fix: Document the escaping protocol in the conductor's Agent Invocation section — e.g., mandate that
{input}is always properly shell-escaped before substitution, and that{placeholder}values from params are individually escaped. Alternatively, note that the conductor should write input to a temp file and pass the file path instead of inline JSON for AI agents.configs/claude/skills/gr-gravity/scripts/validate-goto-schema.sh
[IMPORTANT] Line 133 — The ISO 8601 duration regex:
This regex rejects valid ISO 8601 durations like
P1D(no time component) andP1DT2H(combined date and time). The first branch requiresTat the start (soP7Dwould fail), and the second branch allowsP7Dbut uses[DWMY]which includesY(years) andM(months, butMcollides with minutes in the time portion). The regex also does not handle multi-segment durations likePT1H30M.Suggested fix:
This handles the full ISO 8601 duration syntax: optional date segments followed by optional time segments.
[MINOR] Lines 97-99 — The
error()andwarn()functions use((ERRORS++))and((WARNINGS++))which will return exit code 1 when the variable transitions from 0 to 1 (because((0))is falsy in bash). Combined withset -e, this could cause the script to exit prematurely on the first error/warning. This is a latent bug — currently mitigated because the functions use||or are called in contexts where the return code is consumed, but it is fragile.Suggested fix: Use
ERRORS=$((ERRORS + 1))instead of((ERRORS++))to avoid the exit-code-1 issue underset -e.configs/claude/skills/gr-gravity/SKILL.md
[MINOR] The Result Branching section documents that
cancelledstatus routes toon_error, and the decision table includes acancelledrow. However, the result envelope schema (result-envelope.md) listscancelledas a valid status, but the Call Result Capture section (call-result-capture.md) and the error handling table do not mentioncancelledat all. The conductor's error handling table (line ~570 of the SKILL.md additions) coversnot found,timeout,crashes, andstack overflowbut omitscancelled. Consider adding a row for cancellation or documenting where cancellation originates.[NITPICK] Line 790 contains a rendering artifact:
{result*envelope.payload if non-empty, otherwise "*(none)\_"}— the asterisks and underscores appear to be escaped markdown that will render oddly. Should likely be{result_envelope.payload if non-empty, otherwise "_(none)_"}.configs/claude/skills/gr-pr-review/SKILL.md
[MINOR] The result envelope's
verdictfield uses"pass" | "warn" | "fail"values, while the conductor test cases and convergence conditions throughout the PR use"approved" | "needs_changes"as verdict values. The mapping table clarifies the translation, but having two different verdict vocabularies (one for the envelope, one for the convergence condition) creates a mismatch risk. A caller writingconvergence_condition: "result.data.verdict == approved"would never converge because the envelope writes"pass"not"approved".Suggested fix: Either align the envelope verdict values with the convergence examples (use
"approved"/"not_acceptable") or update all test cases and convergence examples to use"pass"/"fail"to match the actual envelope values.Cross-File Consistency
HUMAN REVIEW: The
request_idformat differs between specifications. The conductor's Call ID Generation section specifies the format{target}_{YYYYMMDD}_{HHMMSS}(e.g.,pr-review_20260318_143000), while the result envelope examples inresult-envelope.mduse a different format:goto-postflight-pr-review-001andgoto-impl-release-042. These are clearly different naming conventions. The test case files consistently use the{target}_{YYYYMMDD}_{HHMMSS}format. Please verify which format is canonical and updateresult-envelope.mdexamples to match.HUMAN REVIEW: The PR introduces three new
.gravity/subdirectories (results/,heartbeats/,handoffs/) that should be gitignored. The ADR notes this requirement (line 1710: "add.gravity/results/,.gravity/heartbeats/, and.gravity/handoffs/to.gitignore") but no.gitignorechange is included in the diff. This may be intentional if.gravity/is already gitignored at a higher level, but please verify.HUMAN REVIEW: The
schema-validatoragent inagents.yamlusesnpx ajv validatewhich requiresajv-clito be installed. This project is a generic neovim config ecosystem with nopackage.json. The agent definition is a reference example, but if it is intended to be functional, the dependency needs to be documented or the agent should note it is illustrative only.Scope Assessment
git-cleanup) — well-structured, comprehensive branch/worktree managementvalidate-goto-schema.sh) — solid schema validation with minor regex issueagents.yaml) — clean schema with 3 reference agentsArchitecture Notes
The three-mode goto extension (goto/call/dispatch) is well-designed with clear separation between protocol specification (SKILL.md conductor sections), schema definition (phase-manifest-schema.md), file format schemas (result-envelope.md, handoff-envelope.md, heartbeat-protocol.md), and behavioral test cases (6 test case files). The ADR provides strong rationale for design decisions including rejected alternatives. The protocol is file-based and dependency-free, consistent with the project's design philosophy.
The addition of result envelope writing to orchestration-aware skills (gr-implementation, gr-pr-review, gr-pr-followup, gr-release) follows a consistent pattern with clear detection, timing, and error handling. The bulk operation preference updates across gr-preflight, gr-refinement, gr-pr-followup, and gr-release are well-scoped incremental improvements.
Review by Gravity Bot — 2026-03-18 | Review cycle: 1
PR Review Followup
Responding to review posted on 2026-03-18 (review comment).
Triage Summary
Fixes Applied
All 3 important findings resolved in commit
72dc9b7:printf '%q'for CLI agents)^P([0-9]+[YWMD])*(T([0-9]+[HMS])+)?$pass/failvsapproved/needs_changes) — aligned gr-pr-review envelope to useapproved/needs_changesmatching all conductor convergence examplesDeferred to v1.31.0
The following items have been tracked for the next version:
((ERRORS++))exit code issue underset -eHuman Review Items (Unchanged)
These items require human judgment and were not auto-triaged:
request_idformat inconsistency between conductor spec and result-envelope.md examples — verify canonical format.gitignoreentries for.gravity/results/,.gravity/heartbeats/,.gravity/handoffs/— verify if.gravity/is already gitignored at a higher levelschema-validatoragent usesnpx ajvwhich requires Node.js — verify if this is illustrative only or intended to be functionalTest Results
This followup was generated by the gr-pr-followup skill.
Update result-envelope.md examples to use the conductor's canonical request_id format ({target}_{YYYYMMDD}_{HHMMSS}) instead of the ad-hoc goto-prefixed format. Refs OP#2357 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>