Redesign RawFileContentAssertion into OwnedFileReadInTest (owner-index cop) #1008
Labels
No labels
P0
P1
P2
P3
bug
category/code-change
category/cross-repo
category/eval-run
category/investigation
category/no-test-target
category/owner-ruling
category/prose-artifact
category/recurring-chore
category/structure
category/umbrella
crc-refactor
create
delete
enhancement
filed-by/agent
filed-by/user
frozen
lint-rule
needs-info
needs-triage
next
plugin/cc-architect
plugin/os
plugin/os-adr
plugin/os-aidd-lint
plugin/os-backlog
plugin/os-context
plugin/os-doc-hygiene
plugin/os-sdlc
plugin/os-vault
project/cc-os
ready-for-agent
ready-for-human
recurring
review
rule-proposal
update
waiting
wayfinder:grilling
wayfinder:map
wayfinder:map
wayfinder:research
wayfinder:task
wayfinder:task
wontfix
worklist/deviations
worklist/lint-rule
worklist/new-implement-build
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
jared/cc-os#1008
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
The cop
Sdlc/Structural/RawFileContentAssertionfires onFile.readcompared 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:eval/ticket-takedown/test/command/final_suite_test.rb:95hasassert_equal "y", File.read(path)withpath=lib/y.rb.Takedown::FilePatchrestores opaque bytes and has no reader, so the test must check bytes.def row_names = File.readlines(File.join(@run_dir, "metrics.tsv"), chomp: true).drop(1).map { _1.split("\t", -1)[2] }re-parsesmetrics.tsv.Takedown::RunSteps(eval/ticket-takedown/lib/takedown/query/run_steps.rb,@path = File.join(dir, "metrics.tsv")) already decodes that file.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.
Sdlc/Structural/OwnedFileReadInTest.lib/with RuboCop's own parser. Use no ast-grep subprocess.File.readlinesplussplit,JSON.parse(File.read(...)),YAML.safe_load(File.read(...)),CSV).Artifact.load(File.join(run_dir, "final-design.json")).metrics.tsvthroughTakedown::RunSteps."original = File.read(path)thenassert_equal original, File.read(path)).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:AllCopssets onlyTargetRubyVersion,NewCops,DisabledByDefault. NoUseCacheorCacheRootDirectoryexists in the repo. The RuboCop default isUseCache: true. The cache directory~/.cache/rubocop_cacheexists on this machine.--cache falseor-C:RUBOCOP_COMMAND, run at :28-33; the test-lint gate)rubocop --format json --config)AUTOCORRECT_COMMAND,REPORT_COMMAND)rubocop -a)lint-worklist.rubocop.yml(plugins/os-sdlc/lib/os_sdlc/adapter/worklist_rubocop_config.rb) that inherits the plugin.rubocop.yml.lib/reader, re-run on an unchanged test file). Then it must choose a fix, for example the cop'sexternal_dependency_checksumor disabling the cache for this cop's runs.Rename consequences
assignment_finder.rb,raw_content_call.rb)require:line for the cop file, and the block at lines 303-315 (Sdlc/Structural/RawFileContentAssertion) with 10 Exclude lines:sdlc-structural-owned-file-read-in-test. The lesson is drafted separately. Do not write it here.Required work:
OwnedFileReadInTestinplugins/os-sdlc/lib/os_sdlc/cops/*.rbRawFileContentAssertioninplugins/os-sdlc/.rubocop.ymlOrchestrator 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
lib/holds a reader ofmetrics.tsvand a test hand-parses it withFile.readlines(File.join(@run_dir, "metrics.tsv"))plussplit, when the cop runs, then it reports an offense.lib/holds a reader of an owned file and a test reads it inside an array literal such asassert_equal [error.message, File.read(owned_path)], when the cop runs, then it reports an offense.lib/readslib/y.rb, when a test assertsassert_equal "y", File.read(path), then the cop reports nothing.metrics.tsvthroughTakedown::RunSteps."Owner sketch
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
RawFileContentAssertionoffense in #1004 (docs/handoffs/2026-10-15-lint-lesson-analysis.md)final_suite_test.rb:95← cop checks the literal, not file ownership ← DESIGN (docs/research/lint-lessons/sdlc-structural-raw-file-content-assertion.md)jared referenced this issue2026-10-15 16:36:56 +00:00
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: 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: 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: 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.
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.