Implementation Plan: open-pr Skill Strips PART X ONLY Suffix from PR Title#492
Merged
Trecek merged 4 commits intointegrationfrom Mar 23, 2026
Conversation
Pipes the BASE_TITLE extraction through an additional sed that removes the trailing '— PART X ONLY' scope marker before it is used as the PR title. Updates Step 2 prose to document the stripping for the multi-plan synthesis path. Adds two contract tests to guard against regression. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Trecek
commented
Mar 23, 2026
Collaborator
Author
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: changes_requested (COMMENT event used — cannot request_changes on own PR)
Trecek
commented
Mar 23, 2026
Collaborator
Author
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review found 3 blocking issues (changes_requested verdict). See inline comments above for details.
Trecek
commented
Mar 23, 2026
Collaborator
Author
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review found 3 blocking issues (verdict: changes_requested). See inline comments for details.
…regex The first assert checked that "PART [A-Z] ONLY" appeared anywhere in SKILL.md, which was trivially satisfied by the Step 2 prose (line 120) rather than proving bash-block presence. The regex pattern lacked a pipe requirement, so sed and the PART pattern could appear on the same line in an unrelated context and still pass. Removes the redundant first assert and tightens the pattern to r"BASE_TITLE=.*\|.*sed.*PART \[A-Z\] ONLY" which requires the pipe from head, matching the actual SKILL.md bash block. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Checking "PART" and "ONLY" as independent substrings could pass if the words appeared in unrelated sentences. Replace with re.search to require the phrase as a unit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The sed pattern uses a literal U+2014 em-dash. On systems with non-UTF-8 locale (LANG=C), GNU sed processes bytes and may fail to match the multi-byte sequence, silently leaving the PART X ONLY suffix in the PR title. Prefixing with LC_ALL=en_US.UTF-8 ensures consistent UTF-8 byte interpretation regardless of the ambient locale. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced Mar 23, 2026
This was referenced Apr 2, 2026
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
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
The
open-prskill's Step 2 extracts the first#heading from the plan file verbatim and uses it as the PR title. Becausemake-planandrectifymandate that multi-part plan files include— PART X ONLYin their heading (e.g.,# Implementation Plan: Foo — PART A ONLY), this internal scope marker leaks directly into the PR title — making PRs appear partial when all parts are implemented.The fix is two-part: (1) update the bash block at Step 2 to pipe through a
sedthat strips the suffix from the extracted heading, and (2) update the Step 2 prose to explicitly instruct stripping the suffix before passing headings to the multi-plan subagent synthesis path. A contract test is added to guard against regression.Architecture Impact
Process Flow Diagram
%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 60, 'curve': 'basis'}}}%% flowchart TB %% CLASS DEFINITIONS %% classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff; classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff; classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff; classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff; classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff; START([START]) END([END]) subgraph Extract ["● Step 2: Title Extraction (open-pr/SKILL.md)"] direction TB HeadExtract["Extract heading<br/>━━━━━━━━━━<br/>head -1 {plan_path}<br/>sed 's/^# //'"] StripSuffix["● Strip PART X ONLY suffix<br/>━━━━━━━━━━<br/>sed 's/ *— *PART [A-Z] ONLY$//'<br/>guards against scope-marker leakage"] end subgraph PlanRoute ["Plan Count Routing"] PlanCount{"single or<br/>multiple plans?"} SingleUse["Use directly<br/>━━━━━━━━━━<br/>BASE_TITLE = stripped heading"] MultiSynth["Subagent synthesis<br/>━━━━━━━━━━<br/>sonnet subagent<br/>synthesizes clean title"] end subgraph PrefixApply ["Step 2b: run_name Prefix"] RunNameCheck{"run_name<br/>switch"} FeaturePrefix["[FEATURE] BASE_TITLE<br/>━━━━━━━━━━<br/>run_name starts with 'feature'"] FixPrefix["[FIX] BASE_TITLE<br/>━━━━━━━━━━<br/>run_name starts with 'fix'"] NoPrefix["BASE_TITLE unchanged<br/>━━━━━━━━━━<br/>any other run_name (e.g. 'impl')"] end PRCreate["gh pr create<br/>━━━━━━━━━━<br/>--title TITLE"] ContractTests["● Contract Tests<br/>━━━━━━━━━━<br/>test_part_suffix_stripped_in_bash_block<br/>test_step2_prose_instructs_suffix_stripping"] START --> HeadExtract HeadExtract --> StripSuffix StripSuffix --> PlanCount PlanCount -->|"single"| SingleUse PlanCount -->|"multiple"| MultiSynth SingleUse --> RunNameCheck MultiSynth --> RunNameCheck RunNameCheck -->|"feature*"| FeaturePrefix RunNameCheck -->|"fix*"| FixPrefix RunNameCheck -->|"other"| NoPrefix FeaturePrefix --> PRCreate FixPrefix --> PRCreate NoPrefix --> PRCreate PRCreate --> END ContractTests -.->|"guards"| StripSuffix class START,END terminal; class HeadExtract handler; class StripSuffix newComponent; class PlanCount,RunNameCheck detector; class SingleUse,MultiSynth phase; class FeaturePrefix,FixPrefix,NoPrefix stateNode; class PRCreate output; class ContractTests newComponent;Color Legend:
Closes #488
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/impl-20260322-203748-209804/temp/make-plan/open_pr_part_suffix_plan_2026-03-22_000000.md🤖 Generated with Claude Code via AutoSkillit