takedown-implement Runner commit omits design-placer's placed files

jared commented

2026-10-16 12:04:48 +00:00

Owner

Problem

The Runner commit at merge-ready stages too few files in a takedown-implement run.
The design-placer places the final design's files, but the commit leaves them out.
An unattended takedown implementation therefore cannot produce a complete commit.

Design

/home/jared/dev/cc-os/.claude/worktrees/prism-architect/docs/takedown/1018-20261016-094026/final-design.json

Approach

DesignPlacement returns every path it wrote or deleted, and the place-design script prints them as a ready items: line that the design-placer pastes into its handoff, so the unchanged ListedPaths & changed_paths rule stages them.

Required work:

Write plugins/os-sdlc/lib/os_sdlc/controller/dispatch/placing.rb
Write plugins/os-sdlc/agents/design-placer.md
Write plugins/os-sdlc/lib/os_sdlc/design_placement.rb
Write test plugins/os-sdlc/tests/controller/place_design_reports_items_test.rb

Acceptance criteria

tests mode, 2 entries: stdout items: ["a","b"], exit 0
code mode, code_files and deleted_files: stdout lists both sets, exit 0
Placement raises (existing path, missing deleted file): Fails as before. Nothing is printed.
Placer handoff carries the line: Placed paths are staged and committed

Owner sketch

Responsibility	Owner
Know which paths were written or deleted	DesignPlacement#place (returns them)
Report them on stdout as an `items:` line	Dispatch::Placing
Put the reported line in the handoff	design-placer.md template (paste the script's line verbatim)
Stage handoff items ∩ changed paths	Commit / ListedPaths (unchanged)

plugins/os-sdlc/tests/controller/place_design_reports_items_test.rb

# frozen_string_literal: true

require_relative "../test_helper"
require "yaml"

module OsSdlc
  class PlaceDesignReportsItemsTest < TmpdirTestCase
    def test_tests_mode_prints_an_items_line_listing_every_placed_test_path
      failing_tests = [
        { "path" => "tests/one_test.rb", "content" => "# one\n" },
        { "path" => "tests/deep/two_test.rb", "content" => "# two\n" }
      ]
      fixture = DesignPlacementFixture.new(@dir, design: { "failing_tests" => failing_tests })

      out, err, status = place(fixture, "tests")

      assert_equal 0, status.exitstatus, err
      assert_equal %w[tests/deep/two_test.rb tests/one_test.rb], items(out).sort
    end

    def test_code_mode_prints_written_and_deleted_paths
      design = {
        "code_files" => [{ "path" => "lib/new_one.rb", "content" => "# new\n" }],
        "deleted_files" => ["lib/old_one.rb"]
      }
      fixture = DesignPlacementFixture.new(@dir, design: design)
      fixture.write("lib/old_one.rb", "# old\n")

      out, err, status = place(fixture, "code")

      assert_equal 0, status.exitstatus, err
      assert_equal %w[lib/new_one.rb lib/old_one.rb], items(out).sort
    end

    private

    def items(out)
      line = out.lines.find { _1.start_with?("items:") } || flunk("no items: line in #{out.inspect}")
      YAML.safe_load(line).fetch("items")
    end

    def place(fixture, mode)
      InProcessDispatchScript.call(@dir, "place-design", mode, fixture.dispatch_id.to_s)
    end
  end
end

## Problem The Runner commit at merge-ready stages too few files in a takedown-implement run. The design-placer places the final design's files, but the commit leaves them out. An unattended takedown implementation therefore cannot produce a complete commit. ## Design /home/jared/dev/cc-os/.claude/worktrees/prism-architect/docs/takedown/1018-20261016-094026/final-design.json ## Approach DesignPlacement returns every path it wrote or deleted, and the place-design script prints them as a ready `items:` line that the design-placer pastes into its handoff, so the unchanged ListedPaths & changed_paths rule stages them. Required work: 1. Write `plugins/os-sdlc/lib/os_sdlc/controller/dispatch/placing.rb` 2. Write `plugins/os-sdlc/agents/design-placer.md` 3. Write `plugins/os-sdlc/lib/os_sdlc/design_placement.rb` 4. Write test `plugins/os-sdlc/tests/controller/place_design_reports_items_test.rb` ## Acceptance criteria - tests mode, 2 entries: stdout `items: ["a","b"]`, exit 0 - code mode, code_files and deleted_files: stdout lists both sets, exit 0 - Placement raises (existing path, missing deleted file): Fails as before. Nothing is printed. - Placer handoff carries the line: Placed paths are staged and committed ## Owner sketch | Responsibility | Owner | |---|---| | Know which paths were written or deleted | DesignPlacement#place (returns them) | | Report them on stdout as an `items:` line | Dispatch::Placing | | Put the reported line in the handoff | design-placer.md template (paste the script's line verbatim) | | Stage handoff items ∩ changed paths | Commit / ListedPaths (unchanged) | `plugins/os-sdlc/tests/controller/place_design_reports_items_test.rb` ```ruby # frozen_string_literal: true require_relative "../test_helper" require "yaml" module OsSdlc class PlaceDesignReportsItemsTest < TmpdirTestCase def test_tests_mode_prints_an_items_line_listing_every_placed_test_path failing_tests = [ { "path" => "tests/one_test.rb", "content" => "# one\n" }, { "path" => "tests/deep/two_test.rb", "content" => "# two\n" } ] fixture = DesignPlacementFixture.new(@dir, design: { "failing_tests" => failing_tests }) out, err, status = place(fixture, "tests") assert_equal 0, status.exitstatus, err assert_equal %w[tests/deep/two_test.rb tests/one_test.rb], items(out).sort end def test_code_mode_prints_written_and_deleted_paths design = { "code_files" => [{ "path" => "lib/new_one.rb", "content" => "# new\n" }], "deleted_files" => ["lib/old_one.rb"] } fixture = DesignPlacementFixture.new(@dir, design: design) fixture.write("lib/old_one.rb", "# old\n") out, err, status = place(fixture, "code") assert_equal 0, status.exitstatus, err assert_equal %w[lib/new_one.rb lib/old_one.rb], items(out).sort end private def items(out) line = out.lines.find { _1.start_with?("items:") } || flunk("no items: line in #{out.inspect}") YAML.safe_load(line).fetch("items") end def place(fixture, mode) InProcessDispatchScript.call(@dir, "place-design", mode, fixture.dispatch_id.to_s) end end end ```

jared added the

plugin/os-sdlc

filed-by/agent

category/code-change

labels

2026-10-16 12:04:48 +00:00

jared referenced this issue

2026-10-16 12:05:01 +00:00

Fix MutableCollectionLeakViaAttrReader: skip private readers, report every attr, decide by reader return #1010

jared commented

2026-10-16 13:16:29 +00:00

Author

Owner

Work started 2026-10-16: live takedown run (session 7, branch prism-architect).

jared commented

2026-10-16 13:16:42 +00:00

Author

Owner

Takedown relevance: needs a ruling

Harm: An unattended takedown-implement run reaches merge-ready with a Runner commit that omits the files the design-placer placed, so the orchestrator must commit them by hand (implementation 47 needed b51ef274 for 7 files).
Protects: A complete, reproducible runner-made commit of the implementation's change, while keeping ADR-0171's rule that the runner stages only paths the agent declared and never user edits in the working tree.
Harm path: The takedown-implement map routes merge-ready to implementation_complete (maps/takedown-implement.yaml:135). Landing#complete! constructs Implementation::Commit (command/implementation/landing.rb:33), which stages ListedPaths (handoff items) intersected with changed paths (command/implementation/commit.rb:34). The design-placer's handoff template has items: [] and nothing else reports its placed paths. (live: true)

Evidence:

plugins/os-sdlc/maps/takedown-implement.yaml:135: merge-ready maps to implementation_complete, so the takedown-implement run reaches the terminal Runner commit path.
plugins/os-sdlc/lib/os_sdlc/command/implementation/landing.rb:33: implementation_complete calls Implementation::Commit.new(...).call and records the result; the commit path is still wired.
plugins/os-sdlc/lib/os_sdlc/command/implementation/commit.rb:34: staged_paths = ListedPaths.new(@implementation).call & changed_paths, so only handoff-listed, changed paths are staged.
plugins/os-sdlc/lib/os_sdlc/query/implementation/commit/listed_paths.rb:12: ListedPaths is the flat union of handoff items' paths; it reads no placement record.
plugins/os-sdlc/agents/design-placer.md:36-40: The design-placer handoff template has items: [], so it reports no paths for the Runner to stage.
plugins/os-sdlc/agents/design-placer.md:18-22: The placer runs os-sdlc-place-design, which writes files; the placed paths are not returned in any handoff item.
plugins/os-sdlc/skills/implement/SKILL.md:153: The skill says the Runner already committed and the orchestrator must not commit again, so a manual commit is outside the contract.
.takedown/history.txt:43-46: Commit 56686026 is the refactorer's two-file commit, matching the ticket's observed paths. Commit b51ef274 is the later manual commit of the placed files.
.takedown/history.txt:1-5: Newest commit d77bbada and the following history touch no commit.rb or ListedPaths code, so no later commit fixes the path.

Conflicting ADRs:

ADR-0171: ADR-0171 fixes the staged set as the union of handoff items: paths intersected with changed paths. The ticket wants placed design files committed, which changes that staged-set rule or the handoff contract (parsed vocabulary).

The ticket is waiting for Jared's ruling on the ADRs above.

Takedown relevance: needs a ruling Harm: An unattended takedown-implement run reaches merge-ready with a Runner commit that omits the files the design-placer placed, so the orchestrator must commit them by hand (implementation 47 needed b51ef274 for 7 files). Protects: A complete, reproducible runner-made commit of the implementation's change, while keeping ADR-0171's rule that the runner stages only paths the agent declared and never user edits in the working tree. Harm path: The takedown-implement map routes merge-ready to implementation_complete (maps/takedown-implement.yaml:135). Landing#complete! constructs Implementation::Commit (command/implementation/landing.rb:33), which stages ListedPaths (handoff items) intersected with changed paths (command/implementation/commit.rb:34). The design-placer's handoff template has `items: []` and nothing else reports its placed paths. (live: true) Evidence: - plugins/os-sdlc/maps/takedown-implement.yaml:135: merge-ready maps to implementation_complete, so the takedown-implement run reaches the terminal Runner commit path. - plugins/os-sdlc/lib/os_sdlc/command/implementation/landing.rb:33: implementation_complete calls Implementation::Commit.new(...).call and records the result; the commit path is still wired. - plugins/os-sdlc/lib/os_sdlc/command/implementation/commit.rb:34: staged_paths = ListedPaths.new(@implementation).call & changed_paths, so only handoff-listed, changed paths are staged. - plugins/os-sdlc/lib/os_sdlc/query/implementation/commit/listed_paths.rb:12: ListedPaths is the flat union of handoff items' paths; it reads no placement record. - plugins/os-sdlc/agents/design-placer.md:36-40: The design-placer handoff template has `items: []`, so it reports no paths for the Runner to stage. - plugins/os-sdlc/agents/design-placer.md:18-22: The placer runs os-sdlc-place-design, which writes files; the placed paths are not returned in any handoff item. - plugins/os-sdlc/skills/implement/SKILL.md:153: The skill says the Runner already committed and the orchestrator must not commit again, so a manual commit is outside the contract. - .takedown/history.txt:43-46: Commit 56686026 is the refactorer's two-file commit, matching the ticket's observed paths. Commit b51ef274 is the later manual commit of the placed files. - .takedown/history.txt:1-5: Newest commit d77bbada and the following history touch no commit.rb or ListedPaths code, so no later commit fixes the path. Conflicting ADRs: - ADR-0171: ADR-0171 fixes the staged set as the union of handoff items: paths intersected with changed paths. The ticket wants placed design files committed, which changes that staged-set rule or the handoff contract (parsed vocabulary). The ticket is waiting for Jared's ruling on the ADRs above.

jared added the

waiting

label

2026-10-16 13:16:42 +00:00

jared commented

2026-10-16 13:23:34 +00:00

Author

Owner

Ruling 2026-10-16 (Jared): comply with ADR-0171. Use the existing handoff process: the design-placer handoff lists the paths it placed in items:, so ListedPaths & changed_paths includes them. No change to the Runner staged-set rule. Prefer the placement script reporting the paths over the model retyping them. Takedown run 1018-20261016-091539 stopped at relevance (needs ruling 0171); a rerun follows.

jared commented

2026-10-16 13:39:48 +00:00

Author

Owner

Takedown relevance: needs a ruling

Harm: An unattended takedown-implement run reaches implementation_complete with the design-placer's placed files left uncommitted, so the merge-ready commit is incomplete and an operator must commit them by hand. Implementation 47 is the cited instance.
Protects: ADR-0171's purpose: the Runner commits the implementation's own change deterministically at implementation_complete, so an unattended run yields a complete, reviewable commit without manual repair.
Harm path: Landing#complete! (command/implementation/landing.rb:31-33) calls Commit#call at implementation_complete, which builds its path set from ListedPaths over handoff items. The takedown-implement map reaches merge-ready: implementation_complete through the place-tests and place-code design-placer nodes, whose handoff template hard-codes items: []. (live: true)

Evidence:

plugins/os-sdlc/lib/os_sdlc/command/implementation/landing.rb:31-33: complete! runs @implementation.complete! and then record_commit(Commit.new(...).call) when the terminal type is implementation_complete.
plugins/os-sdlc/lib/os_sdlc/command/implementation/commit.rb:33-35: staged_paths is ListedPaths.new(@implementation).call & changed_paths, so only handoff-listed paths that git reports as changed are staged.
plugins/os-sdlc/lib/os_sdlc/query/implementation/commit/listed_paths.rb:12: ListedPaths is the union of Item paths over @implementation.handoffs items; placed files appear only if a handoff lists them.
plugins/os-sdlc/agents/design-placer.md:36-40: The design-placer handoff template is items: [] with no instruction to list placed paths, so placer handoffs supply no paths to ListedPaths.
plugins/os-sdlc/maps/takedown-implement.yaml:10-30,135: place-tests and place-code are design-placer nodes that lead to merge-ready: implementation_complete, so the path is still wired in the current map.
docs/adr/0171-the-runner-commits-deterministically-at-implementation-complete-staging-only-handoff-listed-paths-opt-in-per-project.md:4,19: ADR-0171 is Accepted and states the staged set is the union of handoff items paths intersected with git diff HEAD.
.takedown/history.txt:5616: The commit.rb staging path comes from commit 01f5edf9 (#837), and no later commit in the history listed removes or replaces ListedPaths or the design-placer items template.

Conflicting ADRs:

ADR-0171: ADR-0171 fixes the staged set to handoff items intersected with the git diff; the ticket's expected behavior (commit every placed or changed path) reverses that rule if the fix adds a path source other than handoff items. Having design-placer list its paths in items would not reverse it.

The ticket is waiting for Jared's ruling on the ADRs above.

Takedown relevance: needs a ruling Harm: An unattended takedown-implement run reaches implementation_complete with the design-placer's placed files left uncommitted, so the merge-ready commit is incomplete and an operator must commit them by hand. Implementation 47 is the cited instance. Protects: ADR-0171's purpose: the Runner commits the implementation's own change deterministically at implementation_complete, so an unattended run yields a complete, reviewable commit without manual repair. Harm path: Landing#complete! (command/implementation/landing.rb:31-33) calls Commit#call at implementation_complete, which builds its path set from ListedPaths over handoff items. The takedown-implement map reaches merge-ready: implementation_complete through the place-tests and place-code design-placer nodes, whose handoff template hard-codes `items: []`. (live: true) Evidence: - plugins/os-sdlc/lib/os_sdlc/command/implementation/landing.rb:31-33: complete! runs @implementation.complete! and then record_commit(Commit.new(...).call) when the terminal type is implementation_complete. - plugins/os-sdlc/lib/os_sdlc/command/implementation/commit.rb:33-35: staged_paths is ListedPaths.new(@implementation).call & changed_paths, so only handoff-listed paths that git reports as changed are staged. - plugins/os-sdlc/lib/os_sdlc/query/implementation/commit/listed_paths.rb:12: ListedPaths is the union of Item paths over @implementation.handoffs items; placed files appear only if a handoff lists them. - plugins/os-sdlc/agents/design-placer.md:36-40: The design-placer handoff template is `items: []` with no instruction to list placed paths, so placer handoffs supply no paths to ListedPaths. - plugins/os-sdlc/maps/takedown-implement.yaml:10-30,135: place-tests and place-code are design-placer nodes that lead to merge-ready: implementation_complete, so the path is still wired in the current map. - docs/adr/0171-the-runner-commits-deterministically-at-implementation-complete-staging-only-handoff-listed-paths-opt-in-per-project.md:4,19: ADR-0171 is Accepted and states the staged set is the union of handoff items paths intersected with git diff HEAD. - .takedown/history.txt:5616: The commit.rb staging path comes from commit 01f5edf9 (#837), and no later commit in the history listed removes or replaces ListedPaths or the design-placer items template. Conflicting ADRs: - ADR-0171: ADR-0171 fixes the staged set to handoff items intersected with the git diff; the ticket's expected behavior (commit every placed or changed path) reverses that rule if the fix adds a path source other than handoff items. Having design-placer list its paths in items would not reverse it. The ticket is waiting for Jared's ruling on the ADRs above.

jared commented

2026-10-16 13:48:15 +00:00

Author

Owner

Implementation started: /os-sdlc:implement 1018 takedown-implement on branch prism-architect (design docs/takedown/1018-20261016-094026).

jared commented

2026-10-16 13:51:17 +00:00

Author

Owner

Resolution

Done: os-sdlc-place-design prints an items: line listing every path it wrote or deleted; the design-placer copies it into its handoff, so the unchanged ADR-0171 rule (handoff items & changed paths) stages placed files. Built by takedown run 1018-20261016-094026 (1 round, grade cause_removed, $0.42) and implementation 48 (5 dispatches, all pass, 2 min). Also restored the apostrophe rule in design-placer.md and map-router.md to \u0027.

Evidence: Commit ec3665a6 on prism-architect. tests/controller/place_design_reports_items_test.rb: 2 runs, 0 failures (red at base: no items: line). Suite gate green at dispatch 429. Implementation 48's own Runner commit staged nothing, which reproduces the defect: the running code still had items: []; the fix was committed by hand.

Follow-ups: none; live check at the next takedown-implement run this session (#998)

## Resolution **Done:** os-sdlc-place-design prints an items: line listing every path it wrote or deleted; the design-placer copies it into its handoff, so the unchanged ADR-0171 rule (handoff items & changed paths) stages placed files. Built by takedown run 1018-20261016-094026 (1 round, grade cause_removed, $0.42) and implementation 48 (5 dispatches, all pass, 2 min). Also restored the apostrophe rule in design-placer.md and map-router.md to \u0027. **Evidence:** Commit ec3665a6 on prism-architect. tests/controller/place_design_reports_items_test.rb: 2 runs, 0 failures (red at base: no items: line). Suite gate green at dispatch 429. Implementation 48's own Runner commit staged nothing, which reproduces the defect: the running code still had items: []; the fix was committed by hand. **Follow-ups:** none; live check at the next takedown-implement run this session (#998)

jared closed this issue

2026-10-16 13:51:18 +00:00

jared referenced this issue from a commit

2026-10-16 16:51:23 +00:00

fix(os-sdlc): design-placer reports its placed paths as handoff items (#1018)

jared referenced this issue from a commit

2026-10-16 16:51:23 +00:00

docs(takedown): session 7 run records for #1018 and #998

jared referenced this issue from a commit

2026-10-16 16:51:23 +00:00

docs(takedown): session 7 results in the plan; session 8 handoff