keel-finish
Skill0 starsChanged 57 days agoMITView on GitHub
Use when implementation appears complete, before claiming done, committing final work, or opening a PR — enforces fresh verification evidence for every claim, exercises the change end-to-end, names the signals that would later show it worked or that the requirement was wrong, then integrates the branch. Stage 5 (final) of the keel pipeline.
- Reads credentials
In this repository9 other files it gives its agents.
- keel-auditSkill
- security-auditSkill
- keel-debugSkill
- keel-discoverSkill
- keel-executeSkill
- keel-plan-reviewSkill
- keel-planSkill
- keel-wayfindSkill
- keel-workflowSkill
The file
Show its rules and text
We split this skill into the rules it gives an agent
15 rules. About 0 are common, 7 are distinctive, and 8 are very specific to this project.
≈0
Common · estimate
Many files like this have a rule of this kind.
≈7
Distinctive · estimate
This author's own choices. Worth a look.
8
Very specific
Tied to their project's names, files or tools. Probably skip.
- Common
- Distinctive
- Very specific
- Distinctive: line 31
NO COMPLETION CLAIMS WITHOUT FRESH VERIFICATION EVIDENCE.
- Distinctive: line 32
If you haven't run the verification command in this session, in this state of the code, you cannot claim it passes.
- Distinctive: line 46
Run this before ANY status claim — "done", "fixed", "passing", "ready":
- Very specific: line 105
Which surface is not yours to choose here: Read the plan's ## Critical flows and drive every row, against its own Observable result column …
- Distinctive: line 128
Name the target environment before driving anything: If you cannot show it is local, preview, or otherwise ephemeral, stop and get explicit …
- Very specific: line 164
If CONTEXT.md exists: any new domain term this work introduced belongs in it, and public names in the diff should match its vocabulary.
- Very specific: line 169
If the plan file already has a matching ## ADR: <decision name> section — because keel-plan-review Step 5 already produced it at the moment …
- Distinctive: line 186
Every item gets one of two dispositions: resolved (with the evidence) or
- Very specific: line 258
When the user explicitly accepts a Critical finding instead of fixing it, record it the same way Part 2's deferred work is recorded: the …
- Very specific: line 284
Is anything instrumented to produce either?: If the answer is no and the change is worth watching, that is a TODOS.md entry, not a shrug.
- Very specific: line 287
All three answers go into the plan file under ## Signals, next to the Success Criteria they outlive — the instrumentation one as a TODOS.md …
- Very specific: line 309
Read findings.md and the ledger's parked/adjudicated findings.
- Distinctive: line 310
Keep only what is non-obvious AND reusable: needed more than one attempt, a framework/tool behaved counter to its docs, a reviewer finding …
- Very specific: line 405
Do not pass a model override at the call site — each agent file pins its own.
- Distinctive: line 408
All boxes checked, evidence fresh — put exactly these four to the user in
≈ “Common” is an estimate: at least 1 in 5 of the 350,495 skill files we read have a rule of the same kind, not the same words. “Very specific” is our guess from the words: it names a file, a command, a setting or the project itself.
Files
keel-finish — Verification & Branch Integration
INPUT implementation complete on a non-main branch; the plan's success
criteria; the ledger at .keel/progress.md
OUTPUT every claim backed by fresh evidence from this session; open items
reconciled (Part 2b); branch integrated by the user's chosen optionMissing INPUT → BLOCKED: 缺 <field> → 退回 <keel-plan for success criteria,
keel-execute for the ledger>. Part 2 opens the Success Criteria checklist
unconditionally; arriving without one means the gate has nothing to check
and will pass on an empty set.
<IRON-LAW>
NO COMPLETION CLAIMS WITHOUT FRESH VERIFICATION EVIDENCE.
If you haven't run the verification command in this session, in this state
of the code, you cannot claim it passes. Violating the letter of this rule
by paraphrase ("should work now", "looks good") violates the rule.
</IRON-LAW>On session start or after compaction: read the ledger (.keel/progress.md),
the plan header (Success Criteria, ## Signals, ## REVIEW REPORT), and
TODOS.md FIRST. Evidence gathered before the compaction now exists only
as a summarized claim, and the Iron Law treats it as stale: re-run the
verification commands for every box not yet presented to the user, and
re-confirm nothing from recollection.
Part 1: The Gate Function
Run this before ANY status claim — "done", "fixed", "passing", "ready":
1. IDENTIFY What command proves this claim?
Default for build/test/lint claims:
~/.claude/skills/learned/scripts/dod.sh <repo>
(stack-detected; a repo it cannot classify says so — that is
a "not executed — <why>" disclosure, not a pass)
2. RUN Execute the FULL command — fresh, complete, no cached result
3. READ The whole output. Exit code. Count the failures yourself.
4. VERIFY Does the output actually confirm the claim?
5. ONLY THEN Make the claim, with the evidence in hand.
Skipping any step is lying, not verifying.When step 4 comes back NO — the command ran but its output doesn't support
the claim — that is an undiagnosed bug, and it routes. Not "run it again,"
not "write it in the summary and move on": stop keel-finish and take the
command plus its output into keel-debug, which already wants exactly that as
its Phase 1 red loop. Come back and re-run the whole gate from the top when
it's fixed. This is keel-workflow's keel-finish → keel-debug backward route,
and it fires here.
Two cases that are not this route:
- The command produced a clear, specific failure you can fix in place — an ordinary red test. Fix it; that's a normal fix pass, not a stage handoff.
- The command cannot exist or cannot run at all — no test harness, no
non-production environment. That IS the summary-disclosure path: state
not executed — <why>plainly. A stated absence is honest; a claim resting on nothing is not.
Claim → required evidence
| Claim | Sufficient evidence | NOT sufficient |
| Tests pass | Test run output: 0 failures, this session | Previous run; "should pass" |
| Bug fixed | Red-green cycle (below) | The fix "addresses the cause" |
| Feature works | You drove the real flow and saw it | Unit tests green; typecheck clean |
| Agent completed X | VCS diff shows the changes | Agent reported "success" |
| Linter/build clean | Full command output, zero errors | Linter alone (linter ≠ compiler) |
Red-green regression rule (for every bug fix)
Write the regression test → run it (passes with fix) →
revert the fix → run it (MUST FAIL) →
restore the fix → run it (passes)A regression test that never went red proves nothing.
Drive the real flow
Tests exercise what the tests exercise. Additionally run the actual affected surface once: start the app/CLI/endpoint, perform the changed behavior, observe the result. Any change to product source has a runtime surface to drive; "all tests pass but the feature doesn't work" is a routine failure.
Which surface is not yours to choose here. Read the plan's
## Critical flows and drive every row, against its own Observable result
column — an agent picking the surface at this stage picks the one it can get
green, which is the shallowest, and the flows were named at discovery
precisely so the choice would already be made by someone not under pressure
to finish. The ledger's flow: lines say which were already driven during
execution; drive each one again if anything has changed since, and drive
every row the ledger never recorded at all.
Three ways this section ends, and they are not interchangeable:
- Every row driven, each matching its
Observable result— the claim "feature works" now has its evidence. - A row you cannot drive here (needs an environment this session has no
access to) →
not executed — <why>, the same honest-absence line the paragraph below grants. A stated absence is a caveat in the summary. - The plan has no
## Critical flowssection at all → that is a gap, not an exemption. Say so ascritical flows: no section — plan predates keel-discover 5c, fall back to the ad-hoc drive above, and know that you are now doing the thing this section exists to stop: choosing the surface yourself, at the stage least able to judge it. An explicitnone — <why>row is different and is a legitimate answer.
Name the target environment before driving anything. If you cannot show
it is local, preview, or otherwise ephemeral, stop and get explicit consent
naming that environment and the exact command (gate G9). Migrations, deploys,
and writes against a live third party are never authorized by the Iron Law
alone — not executed — no non-production environment available is a
legitimate evidence line, structurally identical to the tool-absent exemption
Part 2c grants the secrets scan.
This is the one place the Iron Law can be turned against the user. The
pressure it creates is real and deliberate — no claim without fresh evidence —
and the only instance the repo makes available is sometimes staging with real
data, or a wrangler config whose only configured env is production. An
honest missing-evidence line costs a caveat in the summary; the alternative
costs the data.
Rationalization table — excuses that mean STOP
| Excuse | Reality |
| "Agent said success" | Verify independently |
| "It worked before my change" | Your change is exactly what's unverified |
| "Just a small refactor" | Small refactors break builds daily |
| "Great! Done!" (before running anything) | Satisfaction is not evidence |
Part 2: Success criteria check
Open the plan's Success Criteria checklist (keel-plan header). Read each
criterion aloud with its evidence and only mark it done once the user confirms
it on the spot — a one-line "does this one check out?" is enough; the user's
on-the-spot confirmation is what closes a box, never the agent's own
assessment. If the user disagrees, that criterion follows the existing unmet
path below. Unmet criteria are reported plainly, with why, and never silently
dropped. A criterion the user agrees
to ship without is deferred work: record it in the repo's TODOS.md using
keel-plan-review's entry format — unwritten deferrals evaporate.
If CONTEXT.md exists: any new domain term this work introduced belongs in
it, and public names in the diff should match its vocabulary. One-line check,
report mismatches — don't rename code at this stage. Same check for ADRs:
a decision this work locked that is hard to reverse + surprising without
context + a real trade-off → offer a one-paragraph ADR before integrating.
If the plan file already has a matching ## ADR: <decision name> section —
because keel-plan-review Step 5 already produced it at the moment the
decision was made — skip the offer (found → skip, not found → offer as
before).
Part 2b: Open-items reconciliation
Loose ends live in three places that never talk to each other. Pull all three into ONE list before integrating, because a decision that exists in only one of them is a decision nobody will find again:
| Source | Where |
| Unresolved decisions | the plan's REVIEW REPORT section (keel-plan-review) |
| Deferred work | TODOS.md entries added during review |
| Flagged concerns | every DONE_WITH_CONCERNS in .keel/progress.md and the task report files |
Every item gets one of two dispositions: resolved (with the evidence) or explicitly deferred. An item with neither blocks completion. Present the reconciled list in the final summary even when everything is clean — "nothing outstanding" is a claim, and like every other claim here it needs to show its work.
Where a deferral lands depends on what the repo has. With a .learned/
(python3 ~/.claude/skills/learned/scripts/learned.py root), a confirmed
problem deferred to a later round goes to
learned.py backlog add <H|M|L> <狀態> <slug> "<中文>" — the backlog carries
priority, an overdue hook, and a close discipline (--rule <ID> or a stated
reason) that a TODO line cannot enforce — and the disposition cites the
backlog file it created. Without one, TODOS.md with a line number, as
before. Either way the reference must resolve: a deferral pointing at
nothing is the "neither" case and blocks.
Part 2c: Security exit gate
Runs every time, regardless of project type — even a plan that never
triggered keel-plan-lens-security or keel-exec-reviewer-security still
executes this section; "nothing to check" is a claim, and like every other
claim in this file it needs to show its work. Same Iron Law as Part 1: every
line below needs evidence produced this session, not a stale scan.
Checks 1–4 are verbatim from the security requirements doc's R6; check 0 is this pipeline's own addition, covering the case R6 assumed away — a branch on which the execution-stage axis never ran at all.
| # | Check | Evidence | Tool-absent handling |
| 0 | Branch-level security coverage. If no task on this branch got a keel-exec-reviewer-security pass — every ledger line reads security axis skipped, or there is no ledger at all (the Small/Medium shortcuts skip keel-execute entirely and still route here) — re-evaluate R4's five conditions against the whole-branch diff rather than per task. Any condition met at branch scope → dispatch keel-exec-reviewer-security once over the merge-base diff before continuing — by name, never general-purpose, and do not pass a model override: that agent pins its own | The branch-scope R4 evaluation, stated condition by condition, plus that reviewer's verdict if it fired | n/a — always runs |
| 1 | Full-branch secrets scan | Run gitleaks detect --source . -v (or semgrep if that's what's installed) this session and paste the summary line (leak count, or "no leaks found") | No such tool installed → state "secrets scan: not executed — no gitleaks/semgrep available" explicitly; never blocks (see BLOCKED condition below). |
| 2 | Execution-stage security axis findings closed | Every Critical/Important keel-exec-reviewer-security finding, read from the security: field of each task's ledger line in .keel/progress.md (keel-execute writes it whenever that axis runs), is either resolved (cite the fix commit) or has an explicit user risk-acceptance decision (format below). A ledger line with neither a security: field nor a documented skip is a gap, not a pass — say so and go get the answer | n/a — always runs |
| 3a | New-dependency CVE / maintenance-status scan | Scanner output if one is installed | Same tool-existence rule as (1): state "not executed" plainly if absent, never BLOCK on absence |
| 3b | New-dependency package-existence verification (anti-slopsquatting) | For every new dependency name introduced on this branch, confirm against its registry that the name actually exists and resolves to the intended package — the same check keel-exec-reviewer-security's checklist item 9 defines | Pure LLM + registry lookup, no external tool involved — no "not executed" exemption; this one must actually run |
| 4 | Plan-stage security lens findings | Read the plan file's ## SECURITY FINDINGS table. Every Critical row is resolved or explicitly risk-accepted — matched by its ## Task N tag, plan-global rows reconciled once, not per task | Section missing entirely (no plan file, or a plan that never went through keel-plan-review — the common case) → this check cannot fire; say so as plan-stage lens findings: no section — plan did not go through keel-plan-review, and rely on check 0, which is what covers that branch. Do not report it as passed |
BLOCKED condition
Any unresolved Critical finding from (0), (2), (3b), or (4) with no explicit user risk-acceptance decision → keel-finish returns:
BLOCKED: 資安 finding 未關閉and does not proceed to Part 3. (1) and (3a) do not trigger this BLOCKED path on their own — reporting "not executed" for those two IS the honest disclosure this file asks for everywhere else; it is a stated absence, not "pretending the check ran." The plan's Global Constraints rule out requiring an external/paid security service, and a hard requirement here would BLOCK every run in an environment without the tool, which is alarm fatigue, not signal — if a scanner IS installed, its real output upgrades that line past "not executed," but it is never required. Treating an unresolved Critical from (0)/(2)/(3b)/(4) as anything less than blocking is the failure this gate exists to prevent.
Where check 0's findings live. That reviewer runs inside this stage, so
there is no ledger line to write them to and no fix loop already scheduled.
Record them directly in the Part 2b reconciliation list as their own source
row, with the same two dispositions everything else there gets: resolved
(cite the fix commit) or explicitly risk-accepted (TODOS.md entry). A
Critical from check 0 blocks exactly like one from (2) — being discovered
late is not a reason to weigh it less.
Risk-acceptance decisions
One finding, one decision. A blanket "fine, ship it, I accept the risk" covering several findings is not a decision — re-ask each one separately, quoting that finding's own text. The user cannot weigh what they were never shown individually.
When the user explicitly accepts a Critical finding instead of fixing it,
record it the same way Part 2's deferred work is recorded: the repo's
TODOS.md, using keel-plan-review's existing entry format (What / Why
deferred / Effort / Priority), plus the finding's file:line appended. No
new ledger schema in .keel/progress.md for this — in this
single-user interactive pipeline the decider is always the user present at
the time, and the timestamp is recoverable from the commit or the TODOS.md
entry itself, so a dedicated "decider"/"time" field would be ceremony with no
reader.
Part 2d: Name the signal, before it becomes unaskable
Everything above verifies the change against a test environment. Nothing so far asks whether it works where it will actually live, and after integration nobody is holding the question any more — the context that knew what "working" meant is about to end.
So, in one line each, before integrating:
- What would tell us this worked? A metric, a log line, a support-ticket category that should shrink, a screen someone actually uses. Name where it is read, not just that it exists.
- What would tell us this was wrong? Not "a bug appears" — the specific
observation that would mean the requirement was mistaken rather than the
code. That is the signal that routes back to
keel-discover, and it is almost impossible to recognise later if nobody wrote it down now. - Is anything instrumented to produce either? If the answer is no and the
change is worth watching, that is a
TODOS.mdentry, not a shrug.
All three answers go into the plan file under ## Signals, next to the
Success Criteria they outlive — the instrumentation one as a TODOS.md
reference when the answer was "not instrumented yet".
No plan file? The Small and Medium shortcuts skip keel-plan and still
route here. Then the answers go in the PR body, or TODOS.md if there is no
PR — the question is about the work, not about which artifact happens to
exist. Part 2c check 0 makes the same allowance for its own missing input. One question, asked once — this is not a new gate
and it does not block integration; a user who answers "nothing to watch, it's
a docs fix" has answered it correctly.
Part 2e: Promote findings to the project rulebook
keel-execute wrote .keel/findings.md (what was learned) as session
memory; it dies with the branch unless promoted. First check the ledger's
findings: line: none — <why> means promotion has nothing to read and
this part is one quoted line in the final summary; a missing line, or a
missing/empty findings.md the line's count contradicts, is an execute-side
gap — record it in the final summary rather than silently skipping. Then,
before integrating, if the repo has a .learned/
(python3 ~/.claude/skills/learned/scripts/learned.py root):
- Read
findings.mdand the ledger's parked/adjudicated findings. - Keep only what is non-obvious AND reusable: needed more than one
attempt, a framework/tool behaved counter to its docs, a reviewer finding
that matched no existing rule, a plan premise the code refuted. Drop
one-off fixes, obvious usage, and anything about how the user likes to
work (that is auto-memory feedback, not a rule). A finding that is an
unfixed problem rather than a lesson is not a rule either — it is
work, and it goes through Part 2b's deferral lane (backlog), not here.
2a. Before writing any survivor as a rule, try to make it impossible
instead. A rule is advice the next agent may not read; ask, in this
order, whether the mistake can be closed by (a) the code itself — a type,
a structure, an API shape that cannot express the wrong thing — or (b) a
linter, compiler setting, or CI check. Either one in reach → propose it to
the user as that change (Part 2b's deferral lane if not done in this
branch), and write the rule only as a pointer to it until it lands. Only
what neither can enforce becomes a rule on its own: code a wrong version
cannot be written in, then a check that fails, then a rule, then a human
remembering in review — rules are third of four, not first.
2b. Check whether a bad pattern has already spread. For each Critical or
Important review finding on this branch that names a pattern — a smell,
a workaround shape, an anti-pattern — rather than a one-line slip, search
the repo for the same shape outside the diff. Agents copy what they read,
so a pattern the reviewer caught here was often copied from somewhere, and
every surviving copy is a template the next agent will extend. Found
elsewhere → report the count and paths, propose stopping the spread first
(step 2a's lint rule or structural change, so no new copy can land), then
the cleanup as Part 2b deferred work. Do not clean up the copies on this
branch: they are outside its scope, and a sweep belongs in its own
reviewed change. Found nowhere else → one line saying so. This step reads
the ledger's adjudicated findings, not the rulebook, so it runs whether or
not the repo has a
.learned/. - For each survivor:
learned.py search "<kw>"first — a near-duplicate gets a**補充(date)**paragraph on the existing rule, not a new ID. Otherwiselearned.py add <cat> "<title>"per the skill's rule-format. Unverified inferences are written as**待驗證**, never as the fix. - Report the IDs added or amended in one line; "no new rules" is a valid answer and must be said, not skipped.
Then ask the positive question once, because nothing else will. Steps 1-4 admit only failure shapes — needed more than one attempt, behaved counter to its docs, refuted a premise — so a branch where you learned a better way to do something that was never broken leaves no trace. Ask: did this branch teach an approach you would now reach for instead of what you would otherwise have written? Sources include the prior-art harvest from
keel-discoverstep 2b, a reviewer's suggestion you adopted, and code you read while working.Two gates before recording it, and they are stricter than steps 1-3's:
- Name what it replaces. State the inferior thing you would otherwise have written, concretely, and why it is worse. Cannot name it → do not record it: you admired something you do not yet understand well enough to reuse, and recording that is how a cargo cult starts.
- Name where it does not apply. Cross-project patterns are the most context-dependent knowledge there is — a shape that is right for a high-throughput service is wrong for a CLI — so the boundary is part of the rule, not a caveat on it.
Where it goes depends on reach: something true only of this repo is a local rule like any other; something that would change how you design in a different project belongs in the cross-project rulebook this repo mounts (its path is in
.learned/config.local.json), written there rather than here. "Nothing this round" is the common answer and is said, not skipped — a pattern invented to fill this step is worse than an empty one.
No .learned/ → one line offering learned.py init; do not block on it.
Step 2b still runs.
Rules are the only artifact from this stage that the next plan reads —
keel-plan-review and keel-execute inject them into every brief — so this
step closes the loop the whole pipeline otherwise leaves open.
Why Part 2d sits here rather than in a sixth stage: the pipeline ends at merge, which is where the code's life starts. A full post-release stage would be a scope this repo cannot honestly claim to run — but the cheap half of a product loop is writing down, while the context still exists, what reality would have to show for the work to count. Skipping that is what makes the loop unclosable, not the absence of a stage.
Part 3: Integrate the branch
Never merge, rebase, push, or force-push into main/master without
explicit user consent — the rule is defined in keel-workflow and binds this
stage exactly as it binds keel-execute. Integration is where the pipeline's
work becomes irreversible; the branch protection does not lapse at the finish
line.
keel-execute dispatches the final whole-branch code-reviewer, not this
stage — it holds the base commit and the ledger context that reviewer needs.
By the time work reaches here, that review and its one fix pass have already
happened. keel-finish's job is to confirm it did: read the final-review:
line from .keel/progress.md — verdict, findings with their file:line,
each one's disposition, and the coverage figure. No such line is a gap, not
a pass; say so and go get it, the same way Part 2c check (2) treats a
missing security: field. Confirming, not re-running; two whole-branch reviews at
inherit-strongest is the most expensive way to duplicate work in this
pipeline.
Do not pass a model override at the call site — each agent file pins its own.
Name the agent, not the role: announce each dispatch before it runs and broadcast each result when it lands, both using the agent's literal keel-* name.
All boxes checked, evidence fresh — put exactly these four to the user in
one AskUserQuestion call (single question, multiSelect: false), your
recommended option marked; do not print them as plain text and wait for a
typed reply:
- Merge back to the base branch locally
- Push + PR — PR body summarizes what/why, links spec + plan. If the project hits PROJECT-TYPE-GUIDE.md's "a real deploy step (not preview-only)" criterion, additionally produce a Release Runbook (pre-deploy checks, deploy command, post-deploy verification commands, rollback command) and write it into the PR body; otherwise skip it.
- Keep the branch — user integrates later
- Discard — the work was exploratory
The AskUserQuestion answer only selects among these four — it never
substitutes for a destructive-path confirmation. If the answer is
Discard, that selection is not the go-ahead: list exactly what will be
deleted (branch name, commit list, worktree path) as a follow-up message,
then require the user to type discard verbatim in reply. Wait for that
exact word — "yes", "ok", "sure", or re-selecting the option do not count.
Then clean up: ledger closed with a final line, plan file marked complete.
Removing a worktree is its own deletion path, and it runs under options
1–3 too. Before removing one, run git status --porcelain inside it. Any
uncommitted change or untracked file — a .env.local, downloaded fixtures,
the throwaway harness keel-debug asks you to build — is content git cannot
recover, so list it and require the user to type remove verbatim. Clean
worktree, nothing untracked → remove it without asking. Option 4's typed
confirmation covers the branch and its commits; it does not cover a dirty
worktree, and the pipeline routes risky work into worktrees on purpose
(keel-workflow's INFRA row).
Report outcomes faithfully — if anything was skipped or is unverified, that goes in the final summary, not under it.
Red flags
- Committing with a failing test "to fix in a follow-up" the user didn't ask for
- Writing the completion summary before running the verification commands
- Deleting the branch before the user chose an integration option
Does it work?
Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.
No reviews yet
People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.
- It worked
- Partly
- It didn't
Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.
Your trail
Files you open will collect here, so you can find them again.
Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.
Now do this for your own.
Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.

