AM-1323 - Carry the plan into BUILD and GATE as a file, not a digest - #25
Conversation
Acceptance evidence: live read-back fidelity
Substituted with the reviewer-checkable read-only proof below, at zero tracker writes — every live call is a Script Established live
Not proven live, stated plainlyRecovered-equals-posted inside a single run. No pre-post source text for this plan exists anywhere locally, so a byte comparison of recovered vs. what was originally posted is unavailable and is not claimed here. That leg stays covered by:
Also unexercised live by this run: the legacy-separator fallback, and the zero/several-ACTIVE refusals (both offline-tested). |
…wn tracker
The header `plan_file` writes stated the rich-text escape transformation as
unconditional fact, while the tool's own docstring already worded it
conditionally. `skills/tracker/github/ADAPTER.md:51` is explicit that GitHub is
"Markdown passthrough — no conversion step", and `github/adapter.py`'s
`post_comment` hands the body to `gh` unconverted, so on a GitHub-backed tracker
every `plan-v{N}.md` carried a false claim about its own contents. Now states the
condition instead of asserting it, and is true either way.
Also drops a stray double space in test_jira.py's fixed-point assertion.
There was a problem hiding this comment.
Pull request overview
Adds a new MCP tool (plan_file) to materialize the agent-facing half of the sole ACTIVE execution plan comment into an on-disk file and pass later /sy:ship phases a path + pin, reducing tracker reads and avoiding loading plan text in phases that don’t need it.
Changes:
- Introduces
plan_fileinsy_tools/server.py, including plan selection (heading/status), boundary splitting (current + legacy), and file materialization. - Adds/updates tests and smoke coverage to pin “tracker read-back shape” and exercise
plan_fileend-to-end. - Updates
/sy:shipand tracker contract docs, agents, and validation checks to use a plan file path/pin instead of a plan digest.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sy_tools/server.py | Adds plan_file, plan parsing, boundary extraction, and writes the plan half to a scratch file. |
| sy_tools/tests/test_server.py | Adds fixtures and tests covering plan_file behavior and invariants. |
| sy_tools/tests/tracker/test_jira.py | Pins Jira ADF↔Markdown read-back shapes used by plan_file fixtures. |
| skills/tracker/jira/ADAPTER.md | Documents Jira comment read-back normalization/escaping relied on by plan_file. |
| skills/tracker/CONTRACT.md | Updates “Exactly one ACTIVE plan” contract to note plan_file enforcement. |
| skills/spec/SKILL.md | Adds guidance about backticking identifiers/paths/URLs to avoid read-back escapes. |
| skills/ship/SKILL.md | Updates state router/invariants to materialize plan per session via plan_file. |
| skills/ship/references/start-resume.md | Updates START contract/state fields to carry plan_path/plan_comment_id/plan_version. |
| skills/ship/references/implementation.md | Updates BUILD guidance to read the plan from the plan file (no tracker read). |
| skills/ship/references/immutable-gate.md | Updates GATE guidance to compose criteria from the plan file and pass its path. |
| skills/shared/references/context-economy.md | Updates context guidance to reflect plan-half file materialization. |
| scripts/validate.py | Adds validation assertions ensuring docs/agents mention the plan file and new state fields. |
| docs/smoke_mcp.py | Smokes plan_file live (as a non-verb tool) without changing verb tool counts. |
| agents/ship-start.md | Updates START agent contract to use plan file path/pin instead of reading tracker/digest. |
| agents/ship-build.md | Updates BUILD agent contract to treat the plan file as authority for the plan. |
| .claude-plugin/plugin.json | Bumps plugin version to 1.22.0. |
Suppressed comments (1)
sy_tools/server.py:525
plan_fileuses the rawissuestring as the scratch directory identifier (config.scratch_dir(issue)). On the GitHub tracker,get_issue/create_issuereturn the issue URL as the id (seesy_tools/tracker/github/adapter.py:84-86), which includes:and/. That will create a deeply nested directory tree on POSIX and is likely to fail on Windows filesystems (invalid:), makingplan_fileunreliable across trackers/platforms.
try:
destination = config.scratch_dir(issue) / f"plan-v{version}.md"
except config.ConfigError as exc:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…s, silent details-truncation, CRLF heading match, three vacuous validate.py checks, start-resume doc accuracy Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…y suffix not substring, disclose the smoke-script escape-shape gap, pin the validate.py scoping heading
…eck on the heading line itself (probe-verified), replace the unguarded _agent_half tests, dedupe smoke-script shape check Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eview Matches the existing pattern in usage_summarize/export_transcript: a filesystem failure now surfaces as a normal ToolError rather than an uncaught exception.
GATE fix-cycle summaryFive
Disclosed, deferred, non-blocking — for a follow-up ticket, not this PR:
A follow-up ticket will be filed at HANDOFF covering all three, plus the previously-known |
Review coverage |
Copilot review (PRRT_kwDOThwh386XMDXG): plan_file's success path could hand out an empty comment_id when the sole ACTIVE comment's id reads back empty, silently defeating the pin comparison a resume relies on for staleness detection. The tool's own zero-or-many branch already treats a missing id as "(no id)"; the success path now refuses the same condition instead of returning an unusable pin.
…-6 nit sy:gate (LOW, b67c81d): the refusal cross-referenced its own source position ("names above") which a ToolError recipient cannot see, and overstated the consequence as making staleness detection wholly no-op when only one of the pin's two moving fields (comment_id) is affected. Reworded to name the actual fault (a tracker read fault, not a plan fault) and the action to take (report it, don't repost the plan), matching the neighbouring refusals' action-ending convention.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (1)
skills/shared/references/context-economy.md:26
- The wording “Exactly one tracker comment is carried forward” is misleading now that later phases receive a file path (materialised from the plan comment) and no phase after the parent reads the ticket. Consider rephrasing to make it clear that it’s the plan comment’s ship half that’s carried forward as a file, not the comment itself.
In Shipyard that has a hard edge. Exactly one tracker comment is carried forward, and only half of it: the `/sy:ship` parent materialises the sole ACTIVE plan's own `## For /sy:ship` half to a file once per session and hands later phases that file's path (`${CLAUDE_PLUGIN_ROOT}/skills/ship/SKILL.md` § State router), and no `/sy:ship` phase reads the ticket itself. Every *other* comment still resolves to nothing for every phase — so a fact a later phase needs is in the plan's ship half, or in that phase's own dispatch brief, or it is not available at all, and "it's in the investigation comment" is not a way to keep detail without paying for it. Either the detail earns its place in the plan's ship half, or it goes in a companion comment the plan does not depend on.
Adds a
plan_fileMCP tool so/sy:ship's parent materialises the sole ACTIVE execution plan's## For /sy:shiphalf to a file once per session and hands later phases its path plus a pin (comment_id,version). Nothing after the parent reads the tracker for the plan, and no phase loads plan text it does not need — the tool returns a path, never the body.plan_file(sy_tools/server.py), boundary derived from_AGENT_DETAIL_TAG, legacy separator still resolves, zero/several ACTIVE refused by count and comment id/sy:shipcontract prose: materialisation in § State router (every session), resume-only pin comparison routed asbail-to-spec, START no longer reads the ticket, BUILD and GATE named the plan fileship-state.yamlgainsplan_path/plan_comment_id/plan_version; all three read asnullon an older filecheck_invariantsassertions, each demonstrated failing against a broken copydocs/smoke_mcp.pyexercises the tool live viaNON_VERB_SMOKED_TOOLS;REQUIRED_TOOLSand its count of 17 verb-serving tools are unchangedskills/tracker/github/ADAPTER.md:51)Live read-back fidelity was proven read-only, not by the live smoke run —
SMOKE_LIVE=1 smoke_mcp.py runwrites, and the only configured project is the productionAMboard. Its code is landed unchanged. Substituted at zero tracker writes:plan_filerun against this ticket's own live ACTIVE plan comment8891727. What that does and does not establish — including that recovered-equals-posted inside a single run stays offline-covered only — is in the acceptance-evidence comment below.🤖 Generated with Claude Code