os-adr: three copies of path and file-name knowledge (A12)

jared commented

2026-10-05 12:28:55 +00:00

Owner

Problem

Three places in os-adr copy path and file-name knowledge that another object already owns. This is heuristic A12 (one rule, one owner). A change to the rule must then change several files.

Observed

plugins/os-adr/bin/adr-migrate:23 builds the report path: File.join('docs', 'adr', Adr::Migration::Report::FILENAME). Report#path (lib/adr/migration/report.rb:15) owns the file name, and Repository#dir (lib/adr/repository.rb:15) owns 'docs', 'adr'.
plugins/os-adr/lib/adr/migration/inventory.rb:7 holds GENERATED_BASENAMES = %w[README.md migration-report.md].freeze. Repository::INDEX_FILENAME (repository.rb:5) and Report::FILENAME (report.rb:6) already define both names.
plugins/os-adr/lib/adr/record/creation.rb:36 builds File.join(repo.dir, "#{id}-#{Adr.slugify(title)}.md"). Repository#write (repository.rb:44) has the same rule.

Reproduce

n/a: design defect. No command fails. The three copies are visible in the cited lines.

Approach

Each caller asks the owner. Show the code first.

# bin/adr-migrate: Migration answers the report path
puts JSON.pretty_generate('applied' => true, 'report' => migrator.apply_fills!(fills)) 
# inventory.rb
GENERATED_BASENAMES = [Adr::Repository::INDEX_FILENAME, Adr::Migration::Report::FILENAME].freeze
# creation.rb: Repository answers the path for an id and title
path = repo.path_for(id, title)

Required work:

bin/adr-migrate takes the report path from the owner (Migration or Report#path) and does not name Report::FILENAME or 'docs', 'adr'. — check: absent Adr::Migration::Report::FILENAME in plugins/os-adr/bin/adr-migrate
Inventory builds GENERATED_BASENAMES from Repository::INDEX_FILENAME and Report::FILENAME. — check: absent migration-report.md in plugins/os-adr/lib/adr/migration/inventory.rb
Record::Creation asks Repository for the file path, and Repository#write uses the same method. — check: absent #{id}-#{Adr.slugify(title)}.md in plugins/os-adr/lib/adr/record/creation.rb

Acceptance criteria

Given a new ADR with an id and title, when Repository#path_for(id, title) answers the path and Repository#write writes the record, then the record file is at the path that path_for answered.

Owner sketch

Repository owns the ADR file path. Migration::Report owns its file name and path. Inventory owns only its candidate list.

class Adr::Repository
  def path_for(id, title)  # File.join(dir, "#{id}-#{Adr.slugify(title)}.md")
end
class Adr::Migration::Report
  def path  # already exists: File.join(@repository.dir, FILENAME)
end

Adr::Repository#write calls path_for(record.id, record.title).
Adr::Record::Creation#write_adr calls repo.path_for(id, title).
bin/adr-migrate prints the report path from Report#path (through Migration), not from literals.
Adr::Migration::Inventory sets GENERATED_BASENAMES = [Repository::INDEX_FILENAME, Report::FILENAME].freeze.

Non-goals

Moving Record::Creation to Repository::Creation (G4, a separate ruling).
No new test for the report path printed by bin/adr-migrate; Required-work check R1 proves it, and the output must stay the relative path docs/adr/migration-report.md (Report#path is absolute, so Migration answers the relative path).
No new test for Inventory's generated-file skip list; Required-work check R2 proves it, and the existing inventory tests cover the skip.

Origin

Trigger: lens challenge G3 (commit 4b8a1730) found the bin/adr-migrate:23 copy and listed the other two. Lens challenge G1 (same commit) gave the same view of bin/ scripts.
Improvised this session: none
Chain: three copies of one rule ← each caller wrote its own copy ← no accessor on the owner (DESIGN: docs/crc/os-adr/lens-challenge-g3-public-surface.md, A12).
Root candidate: none
Where: plugins/os-adr/bin/adr-migrate, plugins/os-adr/lib/adr/migration/inventory.rb, plugins/os-adr/lib/adr/record/creation.rb
Session: ab599e53-dbf5-4953-b8bf-c598a9c5c16e
Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os--claude-worktrees-crc-os-adr-2026-10-04/ab599e53-dbf5-4953-b8bf-c598a9c5c16e.jsonl

## Problem Three places in os-adr copy path and file-name knowledge that another object already owns. This is heuristic A12 (one rule, one owner). A change to the rule must then change several files. ## Observed - `plugins/os-adr/bin/adr-migrate:23` builds the report path: `File.join('docs', 'adr', Adr::Migration::Report::FILENAME)`. `Report#path` (`lib/adr/migration/report.rb:15`) owns the file name, and `Repository#dir` (`lib/adr/repository.rb:15`) owns `'docs', 'adr'`. - `plugins/os-adr/lib/adr/migration/inventory.rb:7` holds `GENERATED_BASENAMES = %w[README.md migration-report.md].freeze`. `Repository::INDEX_FILENAME` (`repository.rb:5`) and `Report::FILENAME` (`report.rb:6`) already define both names. - `plugins/os-adr/lib/adr/record/creation.rb:36` builds `File.join(repo.dir, "#{id}-#{Adr.slugify(title)}.md")`. `Repository#write` (`repository.rb:44`) has the same rule. ## Reproduce n/a: design defect. No command fails. The three copies are visible in the cited lines. ## Approach Each caller asks the owner. Show the code first. ```ruby # bin/adr-migrate: Migration answers the report path puts JSON.pretty_generate('applied' => true, 'report' => migrator.apply_fills!(fills)) # inventory.rb GENERATED_BASENAMES = [Adr::Repository::INDEX_FILENAME, Adr::Migration::Report::FILENAME].freeze # creation.rb: Repository answers the path for an id and title path = repo.path_for(id, title) ``` Required work: 1. `bin/adr-migrate` takes the report path from the owner (`Migration` or `Report#path`) and does not name `Report::FILENAME` or `'docs', 'adr'`. — check: absent `Adr::Migration::Report::FILENAME` in `plugins/os-adr/bin/adr-migrate` 2. `Inventory` builds `GENERATED_BASENAMES` from `Repository::INDEX_FILENAME` and `Report::FILENAME`. — check: absent `migration-report.md` in `plugins/os-adr/lib/adr/migration/inventory.rb` 3. `Record::Creation` asks `Repository` for the file path, and `Repository#write` uses the same method. — check: absent `#{id}-#{Adr.slugify(title)}.md` in `plugins/os-adr/lib/adr/record/creation.rb` ## Acceptance criteria 1. Given a new ADR with an id and title, when `Repository#path_for(id, title)` answers the path and `Repository#write` writes the record, then the record file is at the path that `path_for` answered. ## Owner sketch `Repository` owns the ADR file path. `Migration::Report` owns its file name and path. `Inventory` owns only its candidate list. ```ruby class Adr::Repository def path_for(id, title) # File.join(dir, "#{id}-#{Adr.slugify(title)}.md") end class Adr::Migration::Report def path # already exists: File.join(@repository.dir, FILENAME) end ``` - `Adr::Repository#write` calls `path_for(record.id, record.title)`. - `Adr::Record::Creation#write_adr` calls `repo.path_for(id, title)`. - `bin/adr-migrate` prints the report path from `Report#path` (through `Migration`), not from literals. - `Adr::Migration::Inventory` sets `GENERATED_BASENAMES = [Repository::INDEX_FILENAME, Report::FILENAME].freeze`. ## Non-goals Moving `Record::Creation` to `Repository::Creation` (G4, a separate ruling). No new test for the report path printed by bin/adr-migrate; Required-work check R1 proves it, and the output must stay the relative path `docs/adr/migration-report.md` (Report#path is absolute, so Migration answers the relative path). No new test for Inventory's generated-file skip list; Required-work check R2 proves it, and the existing inventory tests cover the skip. ## Origin - Trigger: lens challenge G3 (commit 4b8a1730) found the `bin/adr-migrate:23` copy and listed the other two. Lens challenge G1 (same commit) gave the same view of `bin/` scripts. - Improvised this session: none - Chain: three copies of one rule ← each caller wrote its own copy ← no accessor on the owner (DESIGN: docs/crc/os-adr/lens-challenge-g3-public-surface.md, A12). - Root candidate: none - Where: plugins/os-adr/bin/adr-migrate, plugins/os-adr/lib/adr/migration/inventory.rb, plugins/os-adr/lib/adr/record/creation.rb - Session: ab599e53-dbf5-4953-b8bf-c598a9c5c16e - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os--claude-worktrees-crc-os-adr-2026-10-04/ab599e53-dbf5-4953-b8bf-c598a9c5c16e.jsonl

jared added the

plugin/os-adr

filed-by/user

labels

2026-10-05 12:28:55 +00:00

jared commented

2026-10-05 12:55:37 +00:00

Author

Owner

Work started on branch crc-os-adr-2026-10-04 (persistent worktree) via /os-sdlc:implement.

jared commented

2026-10-05 13:04:56 +00:00

Author

Owner

Implementation 30 failed (verdict:error after an orchestrator reroute into test-reviewer with no red_tests evidence). AC1 and AC2 moved to Non-goals: they already hold and are proven by Required-work checks R1/R2. Reopening on a fresh implementation.

jared referenced this issue

2026-10-05 13:05:47 +00:00

No approved map fits a behavior-preserving refactor #952

jared referenced this issue

2026-10-05 13:05:48 +00:00

diff-test gate accepts a round as red when any test is red but test-reviewer requires all new tests red #953

jared commented

2026-10-05 13:05:48 +00:00

Author

Owner

Draft dropped (symptom, not root): map-router offers test-reviewer as a reroute entry node with empty red_tests (impl 30, dispatch 282). Same root as the no-map-for-refactor ticket; a red_tests precondition would only move the problem. Note this entry-node case on that ticket.

jared commented

2026-10-05 13:05:49 +00:00

Author

Owner

Draft dropped (symptom, not root): test-reviewer FAIL with CORRECTION NONE routes back to test-writer and burns the bound (dispatches 276, 278, 280). It stems from running a refactor through a red-tests map. #951 covers escalation recovery.

jared commented

2026-10-05 13:09:51 +00:00

Author

Owner

Resolution

Done: Repository#path_for owns the ADR file path; Repository#write and Record::Creation call it. Inventory::GENERATED_BASENAMES is built from Repository::INDEX_FILENAME and Migration::Report::FILENAME. bin/adr-migrate takes the report path from Report#path and prints it relative to the root.

Evidence: Commit on branch crc-os-adr-2026-10-04 (implementation 31, 8 dispatches, 0 retries, after failed implementation 30: map misfit for a behavior-preserving refactor). Required-work checks R1-R3 pass at suite-check; os-adr suite 52 runs, 0 failures; new test repository_test.rb covers path_for/write agreement.

Follow-ups: #952 no map for behavior-preserving refactors; #953 diff-test vs test-reviewer red rule mismatch; reroute-into-test-reviewer and reviewer no-correction loop DROPPED by skeptic as symptoms of #952/#951. Dropped: AC1/AC2 preservation pins (kept only in session scratchpad, not committed; R1/R2 text checks cover them). Branch not yet merged to main: waits for the user's decision on the whole crc-os-adr branch.

## Resolution **Done:** Repository#path_for owns the ADR file path; Repository#write and Record::Creation call it. Inventory::GENERATED_BASENAMES is built from Repository::INDEX_FILENAME and Migration::Report::FILENAME. bin/adr-migrate takes the report path from Report#path and prints it relative to the root. **Evidence:** Commit on branch crc-os-adr-2026-10-04 (implementation 31, 8 dispatches, 0 retries, after failed implementation 30: map misfit for a behavior-preserving refactor). Required-work checks R1-R3 pass at suite-check; os-adr suite 52 runs, 0 failures; new test repository_test.rb covers path_for/write agreement. **Follow-ups:** #952 no map for behavior-preserving refactors; #953 diff-test vs test-reviewer red rule mismatch; reroute-into-test-reviewer and reviewer no-correction loop DROPPED by skeptic as symptoms of #952/#951. Dropped: AC1/AC2 preservation pins (kept only in session scratchpad, not committed; R1/R2 text checks cover them). Branch not yet merged to main: waits for the user's decision on the whole crc-os-adr branch.

jared closed this issue

2026-10-05 13:09:51 +00:00

jared referenced this issue from a commit

2026-10-05 13:50:50 +00:00

feat(os-adr): Audited POODR boundaries for #948: Repository#path_for is the one owner of the record path; R1-R3 present; pass. (#948)

jared referenced this issue from a commit

2026-10-05 13:50:50 +00:00

test(os-adr): pin record path and adr-migrate report path (#948)

jared referenced this issue

2026-10-18 18:58:41 +00:00

implement skill names no map for a behavior-preserving refactor; ticket-implementation map escalates on unexpected_pass #663