Redesign RawFileContentAssertion into OwnedFileReadInTest (owner-index cop) #1008

Open
opened 2026-10-15 15:38:00 +00:00 by jared · 5 comments
Owner

Problem

The cop Sdlc/Structural/RawFileContentAssertion fires on File.read compared to a literal. It gives a false positive on opaque bytes, and it misses real re-parses of files that an object already owns. Analysis: docs/research/lint-lessons/sdlc-structural-raw-file-content-assertion.md. Three cases from #1004:

  1. False positive. eval/ticket-takedown/test/command/final_suite_test.rb:95 has assert_equal "y", File.read(path) with path = lib/y.rb. Takedown::FilePatch restores opaque bytes and has no reader, so the test must check bytes.
  2. Real miss. def row_names = File.readlines(File.join(@run_dir, "metrics.tsv"), chomp: true).drop(1).map { _1.split("\t", -1)[2] } re-parses metrics.tsv. Takedown::RunSteps (eval/ticket-takedown/lib/takedown/query/run_steps.rb, @path = File.join(dir, "metrics.tsv")) already decodes that file.
  3. Evasion miss. assert_equal ["boom", "y"], [error.message, File.read(path)] hides the read inside an array literal.

Reproduce

n/a: design ticket. The cop source is plugins/os-sdlc/lib/os_sdlc/cops/raw_file_content_assertion.rb.

Approach

Design ruled by Jared on 2026-10-15. One cop, no hand-kept list.

  • Rename the cop to Sdlc/Structural/OwnedFileReadInTest.
  • At run time, build an owner index from the AST of lib/ with RuboCop's own parser. Use no ast-grep subprocess.
  • A reader is a class with a method that reads a file and decodes it (File.readlines plus split, JSON.parse(File.read(...)), YAML.safe_load(File.read(...)), CSV).
  • File-name source 1: a literal in the reader class. Source 2: a literal at a call site of the reader, for example Artifact.load(File.join(run_dir, "final-design.json")).
  • In test files, fire when the test reads an indexed file directly, in any shape (arrays, helpers). The message names the owner: "Read metrics.tsv through Takedown::RunSteps."
  • Stay silent on files with no reader (opaque bytes). Stay silent on before/after byte equality on the same path (original = File.read(path) then assert_equal original, File.read(path)).
  • Known misses, accepted: the path passes through 2 or more objects; the decode call is not on the list.

Precondition: verify the RuboCop result-cache risk BEFORE building

RuboCop caches results per source file. A cross-file index means a change in lib/ does not invalidate the cached result of a test file. Findings:

  • plugins/os-sdlc/.rubocop.yml:41-44 AllCops sets only TargetRubyVersion, NewCops, DisabledByDefault. No UseCache or CacheRootDirectory exists in the repo. The RuboCop default is UseCache: true. The cache directory ~/.cache/rubocop_cache exists on this machine.
  • No invocation passes --cache false or -C:
    • plugins/os-sdlc/lib/os_sdlc/lint/lint.rb:13 (RUBOCOP_COMMAND, run at :28-33; the test-lint gate)
    • plugins/os-sdlc/lib/os_sdlc/query/worklist/report.rb:18 (bin/lint-worklist, rubocop --format json --config)
    • plugins/os-sdlc/lib/os_sdlc/command/autocorrect_prepass.rb:15 and :17 (AUTOCORRECT_COMMAND, REPORT_COMMAND)
    • plugins/os-sdlc/lib/os_sdlc/adapter/lint_changed/autocorrect.rb:15 (rubocop -a)
  • The config passed is a generated lint-worklist.rubocop.yml (plugins/os-sdlc/lib/os_sdlc/adapter/worklist_rubocop_config.rb) that inherits the plugin .rubocop.yml.
  • Conclusion: the cache is on in every gate. The build must first prove or disprove staleness (edit a lib/ reader, re-run on an unchanged test file). Then it must choose a fix, for example the cop's external_dependency_checksum or disabling the cache for this cop's runs.
  • ADR find (2026-10-15): no Accepted ADR covers cross-file cops, this cop, its Excludes, or the cache. Related: ADR-0061, ADR-0066, ADR-0194 (cops/ is ignored by Zeitwerk), ADR-0219 (rubocop server under procman).

Rename consequences

  • plugins/os-sdlc/lib/os_sdlc/cops/raw_file_content_assertion.rb and the directory plugins/os-sdlc/lib/os_sdlc/cops/raw_file_content_assertion/ (assignment_finder.rb, raw_content_call.rb)
  • plugins/os-sdlc/tests/cops/raw_file_content_assertion_test.rb
  • plugins/os-sdlc/.rubocop.yml: the require: line for the cop file, and the block at lines 303-315 (Sdlc/Structural/RawFileContentAssertion) with 10 Exclude lines:
    • tests/lint_worklist_report_test.rb
    • tests/lint_worklist_residual_dispatch_test.rb
    • ../os-adr/tests/repository/initialization_test.rb
    • ../os-adr/tests/record/creation_test.rb
    • ../os-adr/tests/index_test.rb
    • ../os-adr/tests/migration_test.rb
    • ../os-aidd-lint/tests/aidd_lint_bin_test.rb
    • ../os-aidd-lint/tests/report_test.rb
    • ../os-aidd-lint/tests/hooks/post_tool_use_hook_test.rb
    • ../os-aidd-lint/tests/hooks/session_end_hook_test.rb
  • Decide per Exclude line whether it still applies. Files with no reader should not need an Exclude.
  • Docs that name the old cop: docs/plans/os-sdlc-reachability.md:68, docs/crc/os-sdlc/lens-audit/tests.md:126, docs/handoffs/2026-10-15-lint-lesson-analysis.md, docs/research/lint-lessons/sdlc-structural-raw-file-content-assertion.md.
  • The lesson file key becomes sdlc-structural-owned-file-read-in-test. The lesson is drafted separately. Do not write it here.

Required work:

  1. Add the renamed cop file — check: present OwnedFileReadInTest in plugins/os-sdlc/lib/os_sdlc/cops/*.rb
  2. Remove the old cop name from the config — check: absent RawFileContentAssertion in plugins/os-sdlc/.rubocop.yml

Orchestrator work: prove or disprove cache staleness before the build, and record the cache fix as an ADR via /os-adr:create if it is a design choice with consequences.

Acceptance criteria

  1. Given lib/ holds a reader of metrics.tsv and a test hand-parses it with File.readlines(File.join(@run_dir, "metrics.tsv")) plus split, when the cop runs, then it reports an offense.
  2. Given lib/ holds a reader of an owned file and a test reads it inside an array literal such as assert_equal [error.message, File.read(owned_path)], when the cop runs, then it reports an offense.
  3. Given no class in lib/ reads lib/y.rb, when a test asserts assert_equal "y", File.read(path), then the cop reports nothing.
  4. Given a test reads one path before and after an action and compares the two reads, when the cop runs, then it reports nothing.
  5. Given an offense on an owned file, when the cop reports it, then the message reads "Read metrics.tsv through Takedown::RunSteps."

Owner sketch

class OwnedFileReadInTest < RuboCop::Cop::Base
  def on_send(node)
end

Non-goals

Fixes at the two #1004 sites. If the new cop fires on them in the current tree, list them in the PR.

Origin

  • Skeptic: VERDICT: CREATE (user-requested ticket)
  • Trigger: lint-lesson analysis of the RawFileContentAssertion offense in #1004 (docs/handoffs/2026-10-15-lint-lesson-analysis.md)
  • Improvised this session: none
  • Chain: false positive on final_suite_test.rb:95 ← cop checks the literal, not file ownership ← DESIGN (docs/research/lint-lessons/sdlc-structural-raw-file-content-assertion.md)
  • Root candidate: none
  • Where: plugins/os-sdlc/lib/os_sdlc/cops/raw_file_content_assertion.rb
  • Session: 72a34749-4d4e-4dd8-9fc0-33bb5bc39ae0
  • Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os--claude-worktrees-prism-architect/72a34749-4d4e-4dd8-9fc0-33bb5bc39ae0.jsonl
## Problem The cop `Sdlc/Structural/RawFileContentAssertion` fires on `File.read` compared to a literal. It gives a false positive on opaque bytes, and it misses real re-parses of files that an object already owns. Analysis: docs/research/lint-lessons/sdlc-structural-raw-file-content-assertion.md. Three cases from #1004: 1. False positive. `eval/ticket-takedown/test/command/final_suite_test.rb:95` has `assert_equal "y", File.read(path)` with `path` = `lib/y.rb`. `Takedown::FilePatch` restores opaque bytes and has no reader, so the test must check bytes. 2. Real miss. `def row_names = File.readlines(File.join(@run_dir, "metrics.tsv"), chomp: true).drop(1).map { _1.split("\t", -1)[2] }` re-parses `metrics.tsv`. `Takedown::RunSteps` (`eval/ticket-takedown/lib/takedown/query/run_steps.rb`, `@path = File.join(dir, "metrics.tsv")`) already decodes that file. 3. Evasion miss. `assert_equal ["boom", "y"], [error.message, File.read(path)]` hides the read inside an array literal. ## Reproduce n/a: design ticket. The cop source is plugins/os-sdlc/lib/os_sdlc/cops/raw_file_content_assertion.rb. ## Approach Design ruled by Jared on 2026-10-15. One cop, no hand-kept list. - Rename the cop to `Sdlc/Structural/OwnedFileReadInTest`. - At run time, build an owner index from the AST of `lib/` with RuboCop's own parser. Use no ast-grep subprocess. - A reader is a class with a method that reads a file and decodes it (`File.readlines` plus `split`, `JSON.parse(File.read(...))`, `YAML.safe_load(File.read(...))`, `CSV`). - File-name source 1: a literal in the reader class. Source 2: a literal at a call site of the reader, for example `Artifact.load(File.join(run_dir, "final-design.json"))`. - In test files, fire when the test reads an indexed file directly, in any shape (arrays, helpers). The message names the owner: "Read `metrics.tsv` through `Takedown::RunSteps`." - Stay silent on files with no reader (opaque bytes). Stay silent on before/after byte equality on the same path (`original = File.read(path)` then `assert_equal original, File.read(path)`). - Known misses, accepted: the path passes through 2 or more objects; the decode call is not on the list. ### Precondition: verify the RuboCop result-cache risk BEFORE building RuboCop caches results per source file. A cross-file index means a change in `lib/` does not invalidate the cached result of a test file. Findings: - plugins/os-sdlc/.rubocop.yml:41-44 `AllCops` sets only `TargetRubyVersion`, `NewCops`, `DisabledByDefault`. No `UseCache` or `CacheRootDirectory` exists in the repo. The RuboCop default is `UseCache: true`. The cache directory `~/.cache/rubocop_cache` exists on this machine. - No invocation passes `--cache false` or `-C`: - plugins/os-sdlc/lib/os_sdlc/lint/lint.rb:13 (`RUBOCOP_COMMAND`, run at :28-33; the test-lint gate) - plugins/os-sdlc/lib/os_sdlc/query/worklist/report.rb:18 (bin/lint-worklist, `rubocop --format json --config`) - plugins/os-sdlc/lib/os_sdlc/command/autocorrect_prepass.rb:15 and :17 (`AUTOCORRECT_COMMAND`, `REPORT_COMMAND`) - plugins/os-sdlc/lib/os_sdlc/adapter/lint_changed/autocorrect.rb:15 (`rubocop -a`) - The config passed is a generated `lint-worklist.rubocop.yml` (plugins/os-sdlc/lib/os_sdlc/adapter/worklist_rubocop_config.rb) that inherits the plugin `.rubocop.yml`. - Conclusion: the cache is on in every gate. The build must first prove or disprove staleness (edit a `lib/` reader, re-run on an unchanged test file). Then it must choose a fix, for example the cop's `external_dependency_checksum` or disabling the cache for this cop's runs. - ADR find (2026-10-15): no Accepted ADR covers cross-file cops, this cop, its Excludes, or the cache. Related: ADR-0061, ADR-0066, ADR-0194 (cops/ is ignored by Zeitwerk), ADR-0219 (rubocop server under procman). ### Rename consequences - plugins/os-sdlc/lib/os_sdlc/cops/raw_file_content_assertion.rb and the directory plugins/os-sdlc/lib/os_sdlc/cops/raw_file_content_assertion/ (`assignment_finder.rb`, `raw_content_call.rb`) - plugins/os-sdlc/tests/cops/raw_file_content_assertion_test.rb - plugins/os-sdlc/.rubocop.yml: the `require:` line for the cop file, and the block at lines 303-315 (`Sdlc/Structural/RawFileContentAssertion`) with 10 Exclude lines: - tests/lint_worklist_report_test.rb - tests/lint_worklist_residual_dispatch_test.rb - ../os-adr/tests/repository/initialization_test.rb - ../os-adr/tests/record/creation_test.rb - ../os-adr/tests/index_test.rb - ../os-adr/tests/migration_test.rb - ../os-aidd-lint/tests/aidd_lint_bin_test.rb - ../os-aidd-lint/tests/report_test.rb - ../os-aidd-lint/tests/hooks/post_tool_use_hook_test.rb - ../os-aidd-lint/tests/hooks/session_end_hook_test.rb - Decide per Exclude line whether it still applies. Files with no reader should not need an Exclude. - Docs that name the old cop: docs/plans/os-sdlc-reachability.md:68, docs/crc/os-sdlc/lens-audit/tests.md:126, docs/handoffs/2026-10-15-lint-lesson-analysis.md, docs/research/lint-lessons/sdlc-structural-raw-file-content-assertion.md. - The lesson file key becomes `sdlc-structural-owned-file-read-in-test`. The lesson is drafted separately. Do not write it here. Required work: 1. Add the renamed cop file — check: present `OwnedFileReadInTest` in `plugins/os-sdlc/lib/os_sdlc/cops/*.rb` 2. Remove the old cop name from the config — check: absent `RawFileContentAssertion` in `plugins/os-sdlc/.rubocop.yml` Orchestrator work: prove or disprove cache staleness before the build, and record the cache fix as an ADR via /os-adr:create if it is a design choice with consequences. ## Acceptance criteria 1. Given `lib/` holds a reader of `metrics.tsv` and a test hand-parses it with `File.readlines(File.join(@run_dir, "metrics.tsv"))` plus `split`, when the cop runs, then it reports an offense. 2. Given `lib/` holds a reader of an owned file and a test reads it inside an array literal such as `assert_equal [error.message, File.read(owned_path)]`, when the cop runs, then it reports an offense. 3. Given no class in `lib/` reads `lib/y.rb`, when a test asserts `assert_equal "y", File.read(path)`, then the cop reports nothing. 4. Given a test reads one path before and after an action and compares the two reads, when the cop runs, then it reports nothing. 5. Given an offense on an owned file, when the cop reports it, then the message reads "Read `metrics.tsv` through `Takedown::RunSteps`." ## Owner sketch ```ruby class OwnedFileReadInTest < RuboCop::Cop::Base def on_send(node) end ``` ## Non-goals Fixes at the two #1004 sites. If the new cop fires on them in the current tree, list them in the PR. ## Origin - Skeptic: VERDICT: CREATE (user-requested ticket) - Trigger: lint-lesson analysis of the `RawFileContentAssertion` offense in #1004 (docs/handoffs/2026-10-15-lint-lesson-analysis.md) - Improvised this session: none - Chain: false positive on `final_suite_test.rb:95` ← cop checks the literal, not file ownership ← DESIGN (docs/research/lint-lessons/sdlc-structural-raw-file-content-assertion.md) - Root candidate: none - Where: plugins/os-sdlc/lib/os_sdlc/cops/raw_file_content_assertion.rb - Session: 72a34749-4d4e-4dd8-9fc0-33bb5bc39ae0 - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os--claude-worktrees-prism-architect/72a34749-4d4e-4dd8-9fc0-33bb5bc39ae0.jsonl
Author
Owner

Occurrence: RawFileContentAssertion still has a config Exclude list
Seen: .rubocop.yml:305 (10 Exclude entries; 5 sites still offend after fast-batch-5 narrowed the cop)
Run: Session 03befa1b-ac60-4013-8433-1e4b189d9a85, fast-batch-5 (524f99c5)
Adds: the cop now skips assert_equal of a literal the test wrote at the same path (cops/raw_file_content_assertion/fixture_writes.rb), so 5 of the 10 Exclude entries are no longer needed; the other 5 still need the redesign.

Occurrence: RawFileContentAssertion still has a config Exclude list Seen: .rubocop.yml:305 (10 Exclude entries; 5 sites still offend after fast-batch-5 narrowed the cop) Run: Session 03befa1b-ac60-4013-8433-1e4b189d9a85, fast-batch-5 (524f99c5) Adds: the cop now skips assert_equal of a literal the test wrote at the same path (cops/raw_file_content_assertion/fixture_writes.rb), so 5 of the 10 Exclude entries are no longer needed; the other 5 still need the redesign.
Author
Owner

Occurrence: Sdlc/Structural/RawFileContentAssertion: carve out checked-in documents by shape and remove the Exclude list
Seen: plugins/os-sdlc/lib/os_sdlc/cops/raw_file_content_assertion.rb
Run: session eb98cdcd-6783-46a0-b7df-cf18a8fe2a56 (#1325 zero-false-positive audit, docs/plans/class-length-family/sdlc-cop-audit.md)
Adds: counterexample plugins/os-backlog/tests/category_documentation_test.rb:25 reads a checked-in SKILL.md that has no owning reader; the MSG prescribes "or exclude this file"; the fix is a matcher carve-out for constant-rooted repository paths, with the .rubocop.yml:317-327 Exclude deleted.

Occurrence: Sdlc/Structural/RawFileContentAssertion: carve out checked-in documents by shape and remove the Exclude list Seen: plugins/os-sdlc/lib/os_sdlc/cops/raw_file_content_assertion.rb Run: session eb98cdcd-6783-46a0-b7df-cf18a8fe2a56 (#1325 zero-false-positive audit, docs/plans/class-length-family/sdlc-cop-audit.md) Adds: counterexample plugins/os-backlog/tests/category_documentation_test.rb:25 reads a checked-in SKILL.md that has no owning reader; the MSG prescribes "or exclude this file"; the fix is a matcher carve-out for constant-rooted repository paths, with the .rubocop.yml:317-327 Exclude deleted.
Author
Owner

Occurrence: Sdlc/Structural/RawFileContentAssertion: fire only when a reader class exists; drop the exclude advice
Seen: plugins/os-me/test/scorer_test.rb:43 (OsMe::Scorer writes context-0.txt for the model to read by path; no class reads it back). Cop message at plugins/os-sdlc/lib/os_sdlc/cops/structural/raw_file_content_assertion.rb:14 still says 'or exclude this file'.
Run: Session 0b4d7950-df1c-4eba-a561-ccc19b386a9a, fix-lints on os-me 2026-10-27 (lint-cops track)
Adds: A live FP on a model-read prompt file, left unfixed on main; the other hit in the same run (moment_extractor_cursor_test.rb:150, owner ExtractCursor) was a true positive, so the two make a ready test pair. The 'exclude this file' clause contradicts the no-Exclude rule.

Occurrence: Sdlc/Structural/RawFileContentAssertion: fire only when a reader class exists; drop the exclude advice Seen: plugins/os-me/test/scorer_test.rb:43 (OsMe::Scorer writes context-0.txt for the model to read by path; no class reads it back). Cop message at plugins/os-sdlc/lib/os_sdlc/cops/structural/raw_file_content_assertion.rb:14 still says 'or exclude this file'. Run: Session 0b4d7950-df1c-4eba-a561-ccc19b386a9a, fix-lints on os-me 2026-10-27 (lint-cops track) Adds: A live FP on a model-read prompt file, left unfixed on main; the other hit in the same run (moment_extractor_cursor_test.rb:150, owner ExtractCursor) was a true positive, so the two make a ready test pair. The 'exclude this file' clause contradicts the no-Exclude rule.
Author
Owner

Occurrence: RawFileContentAssertion false positives on os-backlog (no reader class)
Seen: plugins/os-backlog/tests/category_documentation_test.rb:27,28 (File.read of skills/capture/SKILL.md, assert_match on prose; no class reads the skill doc) and plugins/os-backlog/tests/issue_edit_cli_test.rb:52 (assert_equal on the --body-file the CLI hands to gh; an outside program reads it).
Run: Session 0b4d7950-df1c-4eba-a561-ccc19b386a9a, fix-lints on os-backlog 2026-10-27
Adds: Two more no-reader shapes (doc prose read by models; a file handed to an external program). In the same run, 10 hits had a reader (Backlog::FileIssueParser) and were true positives; those tests now assert through the parser. Ready true-positive/false-positive pairs for the owner-index matcher.

Occurrence: RawFileContentAssertion false positives on os-backlog (no reader class) Seen: plugins/os-backlog/tests/category_documentation_test.rb:27,28 (File.read of skills/capture/SKILL.md, assert_match on prose; no class reads the skill doc) and plugins/os-backlog/tests/issue_edit_cli_test.rb:52 (assert_equal on the --body-file the CLI hands to gh; an outside program reads it). Run: Session 0b4d7950-df1c-4eba-a561-ccc19b386a9a, fix-lints on os-backlog 2026-10-27 Adds: Two more no-reader shapes (doc prose read by models; a file handed to an external program). In the same run, 10 hits had a reader (Backlog::FileIssueParser) and were true positives; those tests now assert through the parser. Ready true-positive/false-positive pairs for the owner-index matcher.
Author
Owner

Survey 2026-10-27: with Exclude removed, the cop fires 48 times in 19 test files (os-adr 24, os-aidd-lint 16, os-sdlc 5, os-backlog 3, os-me 1). About 9 are true positives. All are os-adr record tests that read ADR text where Adr::Record.parse exists, for example os-adr/tests/record/amendment_test.rb:40, challenge_test.rb:66, reconcile_test.rb:14-15, creation_gate_test.rb:46.

Gap 1: the owner index finds file names only from literals. ADR paths are dynamic (adr_path(root), NNNN-slug.md). The index must treat a reader whose decode method gets text from a path argument (Record.parse(File.read(path))) as owning that path family. Add the os-adr sites above as acceptance cases.

Gap 2: the redesign drops the catch of tests that string-match skill or agent prose (PROMPT-TEXT). That catch moves to its own cop ticket: #1444.

Keep this correct false positive as a negative acceptance case: os-backlog/tests/issue_edit_cli_test.rb:52 (bytes for the external gh).

Exclude list in plugins/os-sdlc/.rubocop.yml:278-293: 3 of 10 entries name deleted files (aidd_lint_bin_test.rb, hooks/post_tool_use_hook_test.rb, hooks/session_end_hook_test.rb). record/creation_test.rb is excluded, but its assertions moved to support/record_creation_assertions.rb.

Survey 2026-10-27: with Exclude removed, the cop fires 48 times in 19 test files (os-adr 24, os-aidd-lint 16, os-sdlc 5, os-backlog 3, os-me 1). About 9 are true positives. All are os-adr record tests that read ADR text where Adr::Record.parse exists, for example os-adr/tests/record/amendment_test.rb:40, challenge_test.rb:66, reconcile_test.rb:14-15, creation_gate_test.rb:46. Gap 1: the owner index finds file names only from literals. ADR paths are dynamic (adr_path(root), NNNN-slug.md). The index must treat a reader whose decode method gets text from a path argument (Record.parse(File.read(path))) as owning that path family. Add the os-adr sites above as acceptance cases. Gap 2: the redesign drops the catch of tests that string-match skill or agent prose (PROMPT-TEXT). That catch moves to its own cop ticket: #1444. Keep this correct false positive as a negative acceptance case: os-backlog/tests/issue_edit_cli_test.rb:52 (bytes for the external gh). Exclude list in plugins/os-sdlc/.rubocop.yml:278-293: 3 of 10 entries name deleted files (aidd_lint_bin_test.rb, hooks/post_tool_use_hook_test.rb, hooks/session_end_hook_test.rb). record/creation_test.rb is excluded, but its assertions moved to support/record_creation_assertions.rb.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
jared/cc-os#1008
No description provided.