Skip to content

test: objstore_crlf gets its pytest twin, every check by the shell's name; seeds the ledger (#1341) - #1380

Merged
jdatcmd merged 16 commits into
commandprompt:mainfrom
OffgridwithJD:test/1341-objstore-crlf-twin
Oct 15, 2026
Merged

jdatcmd merged 16 commits into
commandprompt:mainfrom
OffgridwithJD:test/1341-objstore-crlf-twin

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Part of #1341, the object-store twin-parity item: objstore_crlf.sh had no pytest twin.

The twin (test_objstore_crlf.py)

One test, all 5 shell checks by name, added to COMPLETE.

  • Read path: read_parquet with a CR/LF in the URL path is refused naming "CR or LF", and a listener of the file's own (a socket in a thread, standing in for the shell's script) receives nothing carrying the injected line.
  • REST path: the REST catalog client's request goes through the same connect path, so a catalog_uri with a CR/LF is refused the same way.
  • Backend: alive after each attempt.

Twin proof. The request-line guard on the URL path removed, PG17, main 4027b8d against this branch:

shell (main, and this branch) pytest twin
guard removed 2 + 3 failed: no "CR or LF" refusal, and the listener received the injected line red at the same first check, "a CR/LF in the URL path is rejected before the request is sent: got 0"

Evidence

  • Clean: objstore_crlf 5/5 and the twin passed on each of PG 15–19.
  • Ledger: objstore_crlf had no rows, so it is seeded with all 5, three dated by the mutation. 2361 → 2366 rows, the census 1801 → 1803, and suites_not_covered 246 → 245.
  • Gate: rc=0.
  • Counts: TESTS.md section 105, and cluster_tests 534 → 535. Several open PRs of mine take section 105 and move these counts, so each restates and renumbers on rebase.
  • Database-free set: 429 passed.

🤖 Generated with Claude Code

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving 98363af on content. Reproduced on PG18.

  • Green: shell 5/5; pytest 1 passed; compare_to_bash: 5 literal matches, 0 missing.
  • Red: with the request-path CR/LF guard disabled (objstore/columnar_objstore_module.c:381), both twins fail the same three names, which are exactly the three rows the ledger dates:
    FAIL a CR/LF in the URL path is rejected before the request is sent: got [0] want [1]
    FAIL no injected line reached the listener (no request splitting): got [1] want [0]
    FAIL a CR/LF in a REST catalog_uri is rejected too (objstore_http_request path): got [0] want [1]

A note, not blocking: the twin checks only the primary message for "CR or LF", where the shell greps all of stderr.

Before merge: a rebase. TESTS.md §105, expected_tests.txt and the budget conflict.

@OffgridwithJD

OffgridwithJD commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator Author

Rebased onto 57f6e0e (#1401), on top of #1403. The new head is d6ebf5d. Only the count files changed, so this needs your re-approval.

  • Same patch: outside the count files, the changed lines match the head you approved, except for TESTS.md section renumbers where main's new sections took the old numbers.
  • The ledger is restated on main's census and agrees with its tsv: 2791 rows, 2097 never, 694 dated, read back from the file. Each changed paragraph's arithmetic starts from main's last numbers.
  • cluster_tests 567, by collection over the cluster files on this tree. guard_tests 433 is unchanged.
  • Runs on the rebased tree at 217f23b, PG17 and PG18: PG17 harness_selftest: 1245 passed + 0 failed + 0 unrunnable + 0 skipped; PG17 pytest: 41 passed in 9.29s; PG18 harness_selftest: 1245 passed + 0 failed + 0 unrunnable + 0 skipped; PG18 pytest: 41 passed in 9.50s. harness_selftest 1245/1245 and docs_style passed on both. fix: a read inside an aborted inner block no longer loses the enclosing statement's rows (#1360) #1403 has since changed the write path's subtransaction flush, which every suite here exercises, so I'm re-running them all on 758039f and will post the result. CI runs PG17 and PG18 on this head.

🤖 Generated with Claude Code

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Re-run finished on the tree after #1403 (758039f or later). Everything is green, so the head above stands:

  • PG17 docs_style: passed
  • PG17 harness_selftest: 1245 passed + 0 failed + 0 unrunnable + 0 skipped
  • PG17 pytest: 42 passed in 9.31s
  • PG17 vacuum_lock_privilege: 19 passed + 0 failed + 0 unrunnable + 0 skipped
  • PG18 docs_style: passed
  • PG18 harness_selftest: 1245 passed + 0 failed + 0 unrunnable + 0 skipped
  • PG18 pytest: 42 passed in 9.32s
  • PG18 vacuum_lock_privilege: 19 passed + 0 failed + 0 unrunnable + 0 skipped

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-objstore-crlf-twin branch from d6ebf5d to ddcb271 Compare October 15, 2026 17:06
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto 7da848d (#1432, #1409). The new head is ddcb271; only the count files changed, so please re-approve this head.

🤖 Generated with Claude Code

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approved at ddcb271. Compared with 98363af, the head I approved, file by file: the changed lines are the same, apart from the count files (check_ledger_budget.txt, expected_tests.txt) and, in #1378/#1379/#1380/#1383, the TESTS.md section number, now 115.

🤖 Generated with Claude Code

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto 1f7b5da (#1411, #1412, #1417, #1418). The new head is dd083da. Only the count files changed, plus TESTS.md section numbers where main took 115, so please re-approve this head.

🤖 Generated with Claude Code

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approved at dd083da, the rebase onto 1f7b5da. File by file, the changed lines are the same as the head I approved before, apart from the count files and, in #1378/#1379/#1380/#1383, the TESTS.md section number.

🤖 Generated with Claude Code

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

The PG18 re-run on this head (dd083da, on 1f7b5da with #1412's index-build change) is all green:

  • docs_style: passed
  • harness_selftest: 1245 passed + 0 failed + 0 unrunnable + 0 skipped
  • pytest: 41 passed in 6.85s

🤖 Generated with Claude Code

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto 37ea557 (#1414, #1425). The new head is 364f99c. Only the count files changed, so please re-approve this head.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-objstore-crlf-twin branch from dd083da to 364f99c Compare October 15, 2026 18:28
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto f2bd5f1 (#1427). The new head is 1b6f282. Only the count files changed, so please re-approve this head.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-objstore-crlf-twin branch from 364f99c to 1b6f282 Compare October 15, 2026 18:38

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approved at 1b6f282, the rebase onto f2bd5f1. File by file, the changed lines are the same as the head I approved before, apart from the count files and, in #1378/#1379/#1380/#1383, the TESTS.md section number.

🤖 Generated with Claude Code

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto d5c0a5e (#1391). The new head is b7487ad, so please re-approve this head.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-objstore-crlf-twin branch from 1b6f282 to b7487ad Compare October 15, 2026 19:26
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto 2d1a77f (#1413–#1424). The new head is 431566c, so please re-approve this head.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-objstore-crlf-twin branch from b7487ad to 431566c Compare October 15, 2026 20:58

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approved at 431566c, the rebase onto 2d1a77f. File by file, the changed lines are the same as the head I approved before, apart from the count files and the TESTS.md section number.

🤖 Generated with Claude Code

jdatcmd and others added 16 commits October 15, 2026 21:15
…ws are reworded

The mutation column is a ';'-separated set, written back sorted. A --mutation
whose text held "; " was stored as two mutations, one a fragment starting with
a space, and every later merge re-sorted it. commandprompt#1427's recovery_minimal_wal text
did this to 8 rows, and commandprompt#1423, commandprompt#1424 and commandprompt#1426 each carried them as a change of
their own. Four older harness_selftest rows were split the same way.

- merge refuses a --mutation holding ';' (rc 2, ledger untouched).
- the reader refuses a ';'-piece with whitespace at an end, by line: the writer
  joins with a bare ';', so such a piece is a fragment, never a mutation.
- the 12 rows are reworded without ';', from their original text in history.
  Only field 6 changes; the census stays 3007 / 2282 / 725.

Red first: the two refusal arms got 0 want 2 on the unchanged tool, and the
committed-ledger arm was red at line 1783 with the refusal in and the rows not
yet reworded. guard_tests 435 -> 438 by collection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ment-mark mutation (commandprompt#1341 item 75)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…wn privileges, in both twins (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…d the refused tier writes no file, in both twins (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
… back; both its branches echoed "ran" (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…nd every owner arm reports a number when refused, in both twins (commandprompt#1341)

A guard widened to refuse the owner stopped the fixture's own export, so a
premise reddened for the product code and the pytest twin crashed in setup
rather than failing an arm by name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…e mutations of the RLS guard (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…x, and the twin fails it by name rather than raising (commandprompt#1383 review)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjTxfQJj5pnii9wzmPP9Ho
…name (commandprompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rompt#1341)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Restated for the chain you asked for. The new head is ab6ae27, so please re-approve this head.

🤖 Generated with Claude Code

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-objstore-crlf-twin branch from 431566c to ab6ae27 Compare October 15, 2026 21:23

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approved at ab6ae27, its place in the chain on 17420be. This PR's own commits, measured from the link below it, change the same lines as the head I approved before, apart from the count files and the TESTS.md section number.

🤖 Generated with Claude Code

@jdatcmd
jdatcmd merged commit 38a4ba6 into commandprompt:main Oct 15, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants