From 8ec73f535df800a54ee75f0dd0c7c986380890ad Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Thu, 3 Sep 2026 18:28:48 -0400 Subject: [PATCH 1/2] docs(fiqh): the cash posture residual is recorded, not merely acknowledged (#691, part 2 of 2) Closes #691. The last two acceptance boxes: `keel brokers list` and the `docs/fiqh-basis.md` open question. THE PIN DID NOT FAIL WHEN IT SHOULD HAVE, AND THAT IS THE FINDING. `test_the_unprovable_half_of_the_cash_posture_is_stated_not_hidden` promised in its own docstring that "closing that needs a human attestation, not another adapter read -- and if one is ever built, this fails again". #691 built one. **The pin passed**, and the issue's own acceptance criterion predicted a failure that never came. Nothing in the pin observed the attestation, so it repeated its FIRST failure mode exactly: that form pinned `_sell_base_size`'s docstring as a proxy and held while the paragraph it guarded went false. A promise in a docstring is not a pin, and prose that matches is not prose that is true. So this re-pointing reaches for the MECHANISM rather than for sentences. Four assertions that each go false if something real is removed: * `CashPostureState` must carry no `CONFIRMED` -- adding one would mean keel believes something can AFFIRM a spot cash account, which is the claim the doc spends a section refuting. * `ATTESTATION_TTL_SEC > 0` -- without expiry, "the operator states it and keel records it" degrades to "keel remembers what someone said once". * `cash_posture` must appear in `guards.py` -- with no rail reading the record, "an unattested posture vetoes live entries" is a sentence about nothing. * the stale "a human attestation this repository has not yet built" must be GONE -- the specific prose regression, caught by string rather than hoped about. THE DOC. Two passages were wrong on their facts, not merely dated: one said keel "has no cash-account posture check on Coinbase", which #666 added, and the other said the attestation was unbuilt. Both are corrected, and the section now states the three load-bearing properties (no confirmable state, a 90-day expiry, a claim bound to a credential) followed by the sentence to keep if only one survives: **keel still cannot verify this, and neither can the venue.** What changed is that the trust is now dated, revocable and written down, rather than assumed. THE BLOCK. `keel brokers list` gains a THIRD block after declarations and readiness, which follows this codebase's own argument rather than a preference: `venue_readiness` renders separately because merging two questions "would re-blur exactly the distinction #233 exists to draw". Posture is a third question -- readiness asks whether this CREDENTIAL may trade, posture asks whether this ACCOUNT can borrow. It ends on its own honesty line, and a test forbids the words "verified", "confirmed", "checked" and "proven" anywhere in it: every other readiness surface reports something it checked, and this one cannot. Read-only and best-effort, `_readiness_rows`' discipline -- a display command must not create a database, and a pre-v18 file renders an empty block rather than failing. Tests written first: 9 on the block, plus the re-pointed fiqh pin. Mutation-verified, 6 mutants. One SURVIVED: replacing `sorted` with `reversed` produced identical output because the fixture had only two venues in reverse alphabetical order. Three venues now, in an order where reversing is not sorting. Closes #691 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NzuKAe2RVrPt9acVAWjRyL --- docs/fiqh-basis.md | 40 ++++-- keel/commands/brokers.py | 87 +++++++++++++ .../test_brokers_cash_posture_block.py | 114 ++++++++++++++++++ tests/test_fiqh_basis.py | 64 ++++++++-- 4 files changed, 290 insertions(+), 15 deletions(-) create mode 100644 tests/commands/test_brokers_cash_posture_block.py diff --git a/docs/fiqh-basis.md b/docs/fiqh-basis.md index dfa5b62b..151f3aad 100644 --- a/docs/fiqh-basis.md +++ b/docs/fiqh-basis.md @@ -233,9 +233,12 @@ found from. The trigger is the venue's own statement, never keel's ledger: cance over a position that really exists strips a live holding of its only protection, and the ledger can be stale in exactly that direction. -What remains open at the venue boundary is #666: on a cash account every case above is a -rejected order rather than a short, and keel has no cash-account posture check on Coinbase — -`verify_cash_account` exists only on the Alpaca adapter. +At the venue boundary the distinction that matters is the account's posture: on a cash account +every case above is a rejected order rather than a short. Both adapters now carry a +`verify_cash_account` check (#666), and **rail 22** (#691) reads a per-venue operator attestation +of that posture, vetoing new ENTRIES when none stands. Neither the check nor the rail can affirm +a cash account — see "Nothing can affirm that the account cannot go short" below for what that +means and what carries the residual. Beside the rails — not among them, and not numbered — sits one routing-time check with the same prudential character: the **max-spread entry gate** (#350, `keel/execution/executor.py`) @@ -321,10 +324,33 @@ Stated, not hidden — each is a place where keel's encoded behaviour could be w possession, and here silence is not evidence of a cash account either. So the layered defence is sound whenever the venue contradicts itself, and rests on the - operator's own knowledge whenever it does not. That residual is a human attestation this - repository has not yet built — the second half of the #233 pattern, where venue evidence can - refute an attestation but cannot manufacture one. Until it exists, "this account cannot go - short" is something the operator knows and keel does not. + operator's own knowledge whenever it does not. **Since 2026-09-03 that residual is recorded + rather than merely acknowledged (#691).** `keel posture attest --spot-cash` writes a per-venue + claim; **rail 22** vetoes new ENTRIES — and only entries, never an exit — when none stands. + This is the second half of the #233 pattern: venue evidence can refute an attestation and + cannot manufacture one, so an INTX portfolio found at broker-build marks a standing claim + REFUTED, and nothing ever marks one confirmed. + + Three properties of that record are load-bearing, and none is incidental: + + - **There is no `CONFIRMED` state.** Trade scope has one because the venue re-proves it on + every accepted placement. Nothing can ever prove a spot account is cash-only, so a + `CONFIRMED` value would be a state nothing is entitled to write — and an unreachable state is + one a later reader eventually writes anyway. Its absence is the design. + - **The claim EXPIRES**, after 90 days. Nothing re-confirms it, ever, so a due date is the only + thing standing between a lapsed claim and a live entry — the same reasoning as the + subscription record, which is likewise user-asserted with no observation channel. `doctor` + warns 15 days out rather than at the cliff. + - **The claim is about a credential, not just a venue.** A posture attested under one + credential is not a claim about the account another credential reaches (#633), so a rotated + key withdraws permission until the operator re-attests. + + What has NOT changed is the epistemics, and this is the sentence to keep if only one survives: + **keel still cannot verify this, and neither can the venue.** What the engine now does is + record a human's statement, expire it on a clock, let venue evidence contradict it, and refuse + to trade without it. "This account cannot go short" remains something the operator knows and + keel takes on trust — the difference is that the trust is now dated, revocable, and written + down, rather than assumed. - **ZEC and the rest of the deferrals.** The candidate-universe record lists the open questions the attestation step has to answer and "which this agent must not answer". diff --git a/keel/commands/brokers.py b/keel/commands/brokers.py index baa9a489..162db717 100644 --- a/keel/commands/brokers.py +++ b/keel/commands/brokers.py @@ -29,6 +29,7 @@ import json import textwrap +import time from dataclasses import asdict, dataclass from importlib.metadata import PackageNotFoundError from importlib.metadata import version as dist_version @@ -36,6 +37,11 @@ from typing import Any import click +from keel_core.cash_posture import ( + MARGIN_ENABLED, + CashPostureState, + VenueCashPosture, +) from keel.commands._common import DISCLAIMER from keel.venue_readiness import VenueReadinessRow, gather_readiness @@ -325,6 +331,85 @@ def render_readiness_lines(rows: list[VenueReadinessRow]) -> list[str]: return lines +#: The line the posture block ends on, and it is not decoration. Every other readiness-adjacent +#: surface in keel reports something it CHECKED; this one reports something no venue will confirm. +#: A reader who takes these rows for verification has the guarantee backwards -- which is the one +#: misreading `docs/fiqh-basis.md`'s cash-posture section exists to prevent. +CASH_POSTURE_HONESTY_LINE = ( + " No venue exposes a cash-versus-margin field for spot, so these are the OPERATOR's " + "statements, expiring on a clock. Venue evidence can refute one; nothing can confirm one." +) + + +def render_cash_posture_lines( + records: list[VenueCashPosture], *, now_ts: int +) -> list[str]: + """The cash-posture block (#691): what a human has stated about each venue account. + + A THIRD block, after declarations and readiness, and that follows this codebase's own + argument rather than a preference. `keel/venue_readiness.py` renders readiness separately + because merging two different questions "would re-blur exactly the distinction #233 exists to + draw". Posture is a third question -- readiness asks whether this CREDENTIAL may trade, + posture asks whether this ACCOUNT can borrow -- and folding it into either would repeat the + mistake both were shaped to avoid. + + PURE over the records it is handed. Sorted by venue so the same database renders the same + text twice. + """ + lines = ["", "cash posture (rail 22) -- attested by you, refutable by the venue:"] + if not records: + lines.append(" no venue has an attested cash posture -- rail 22 vetoes live ENTRIES") + lines.append(" next: keel posture attest --spot-cash") + lines.append(CASH_POSTURE_HONESTY_LINE) + return lines + for record in sorted(records, key=lambda r: r.venue): + state = record.state.value.upper() + if record.state is CashPostureState.ATTESTED and not record.is_current(now_ts): + state = "EXPIRED" + expires = ( + _utc_day(record.attest_due_ts) if record.attest_due_ts is not None else "never set" + ) + lines.append( + f" {record.venue}: {state} attested={record.attested_posture} expires={expires}" + ) + if record.attested_posture == MARGIN_ENABLED: + lines.append(" margin-enabled: rail 22 vetoes live ENTRIES on this venue") + if record.refuted_ts is not None: + reason = f": {record.refuted_reason}" if record.refuted_reason else "" + lines.append( + f" venue evidence contradicted a claim here on " + f"{_utc_day(record.refuted_ts)}{reason}" + ) + lines.append(CASH_POSTURE_HONESTY_LINE) + return lines + + +def _utc_day(ts: int) -> str: + from datetime import UTC, datetime + + return datetime.fromtimestamp(ts, tz=UTC).strftime("%Y-%m-%d") + + +def _cash_posture_records(ctx: click.Context) -> list[VenueCashPosture]: + """Read-only, best-effort, never creates or migrates a database -- `_readiness_rows`' + discipline, for the same reason: `keel brokers list` must keep working on a fresh checkout + with no deployment at all, and a display command that created a database as a side effect + would be a surprising one.""" + obj = ctx.obj or {} + db_path = obj.get("db_path") + if db_path is None or not Path(db_path).exists(): + return [] + try: + from keel.data.db import connect + from keel.data.repository import Repository + + return Repository(connect(str(db_path))).list_venue_cash_postures() + except Exception: + # A missing table (a database older than v18), a locked file, anything: an empty block + # is the honest display, and never a reason to fail a read-only command. + return [] + + def _readiness_rows(ctx: click.Context) -> list[VenueReadinessRow]: """Gathers this deployment's readiness rows for `keel brokers list` -- `gather_readiness`'s CLI wiring, the same service `/api/venues` wires to a repo it knows is migrated. @@ -394,5 +479,7 @@ def brokers_list(ctx: click.Context, as_json: bool) -> None: click.echo(line) for line in render_readiness_lines(_readiness_rows(ctx)): click.echo(line) + for line in render_cash_posture_lines(_cash_posture_records(ctx), now_ts=int(time.time())): + click.echo(line) click.echo("") click.echo(DISCLAIMER) diff --git a/tests/commands/test_brokers_cash_posture_block.py b/tests/commands/test_brokers_cash_posture_block.py new file mode 100644 index 00000000..5cf18d79 --- /dev/null +++ b/tests/commands/test_brokers_cash_posture_block.py @@ -0,0 +1,114 @@ +"""`keel brokers list` must show the cash posture -- #691's last acceptance box. + +A SEPARATE block, after readiness, and that is this module's own argument rather than a +preference: `keel/venue_readiness.py` states that readiness is "rendered as its own headed block, +AFTER the declarations block, with its own honesty line", because merging two different questions +"would re-blur exactly the distinction #233 exists to draw". Posture is a third question -- +readiness asks whether this CREDENTIAL may trade, posture asks whether this ACCOUNT can borrow -- +and folding it into either of the first two would repeat the mistake both were shaped to avoid. + +`test_the_block_never_claims_a_posture_was_verified` is the one that matters. Every other surface +in keel can say "checked"; this one cannot, because no venue exposes the field. A block that +reads like a verification would invert the exact guarantee `docs/fiqh-basis.md` spends a section +establishing. +""" + +from __future__ import annotations + +from keel_core.cash_posture import ( + ATTESTATION_TTL_SEC, + MARGIN_ENABLED, + SPOT_CASH, + CashPostureState, + VenueCashPosture, +) + +from keel.commands.brokers import render_cash_posture_lines + +NOW = 1_800_000_000 + + +def _record( + venue: str = "coinbase", + state: CashPostureState = CashPostureState.ATTESTED, + posture: str | None = SPOT_CASH, + due_ts: int | None = NOW + ATTESTATION_TTL_SEC, + refuted_ts: int | None = None, +) -> VenueCashPosture: + return VenueCashPosture( + venue=venue, + state=state, + attested_posture=posture, + attested_ts=NOW, + attest_due_ts=due_ts, + refuted_ts=refuted_ts, + refuted_reason="INTX portfolio present" if refuted_ts else None, + credential_fingerprint=None, + ) + + +def _text(records, now_ts: int = NOW) -> str: + return "\n".join(render_cash_posture_lines(records, now_ts=now_ts)) + + +def test_the_block_is_headed_so_it_cannot_be_read_as_part_of_readiness() -> None: + text = _text([_record()]) + assert "cash posture" in text.lower() + + +def test_nothing_attested_says_so_and_names_the_command() -> None: + text = _text([]) + assert "no venue" in text.lower() or "none" in text.lower() + assert "keel posture attest" in text + + +def test_an_attested_venue_shows_its_posture_and_expiry() -> None: + text = _text([_record()]) + assert "coinbase" in text + assert "spot_cash" in text + assert "expires" in text.lower() + + +def test_an_expired_attestation_is_marked_expired() -> None: + text = _text([_record()], now_ts=NOW + ATTESTATION_TTL_SEC + 1) + assert "EXPIRED" in text + + +def test_a_margin_venue_is_shown_as_refusing_entries() -> None: + text = _text([_record(posture=MARGIN_ENABLED)]) + assert "margin" in text.lower() + + +def test_a_refuted_venue_names_the_venue_evidence() -> None: + text = _text([_record(state=CashPostureState.REFUTED, refuted_ts=NOW)]) + assert "INTX portfolio present" in text + + +def test_the_block_never_claims_a_posture_was_verified() -> None: + """THE pin. No venue exposes this field, so nothing here may read as a check that passed. + `docs/fiqh-basis.md` spends a section establishing that the venue can only REFUTE, and a + display saying "verified" would invert it for every reader who never opens the doc.""" + text = _text([_record()]).lower() + for forbidden in ("verified", "confirmed", "checked", "proven"): + assert forbidden not in text, f"the posture block reads as {forbidden!r}" + + +def test_the_block_says_the_claim_is_the_operators_own() -> None: + """The positive half of the same pin: not merely avoiding "verified", but saying whose + statement this is and that nothing can check it.""" + text = _text([_record()]).lower() + assert "attested" in text + assert "no venue" in text or "cannot" in text + + +def test_venues_are_listed_in_a_stable_order() -> None: + """Sorted, so the same database renders the same text twice. + + THREE venues, in an order where reversing is not the same as sorting: with only + `[coinbase, alpaca]` a mutation replacing `sorted` with `reversed` produced the identical + output and the pin stayed green. + """ + text = _text( + [_record(venue="alpaca"), _record(venue="robinhood"), _record(venue="coinbase")] + ) + assert text.index("alpaca") < text.index("coinbase") < text.index("robinhood") diff --git a/tests/test_fiqh_basis.py b/tests/test_fiqh_basis.py index 4e568fd3..ae9b3ba9 100644 --- a/tests/test_fiqh_basis.py +++ b/tests/test_fiqh_basis.py @@ -394,6 +394,12 @@ def test_the_readme_links_the_document(): #: `margin_rate` present-and-NULL and no cash-versus-margin field anywhere in the spot surface. _NO_AFFIRMATIVE = "refute a cash posture and never issue one" +#: The sentence the doc carried while the attestation was unbuilt. It is now FALSE, and its +#: continued presence is the specific regression the pin below watches for -- a section saying +#: keel relies on nothing, where it now relies on an EXPIRING human claim, understates what an +#: operator owes. +_STALE_UNBUILT_CLAIM = "a human attestation this repository has not yet built" + def test_the_long_only_ruling_is_pinned_two_sided_to_the_code_that_enforces_it(): """The doc names two enforcement points by their exact source text; both must still exist. @@ -468,20 +474,35 @@ def test_the_hadith_reference_is_marked_as_outside_the_knowledge_base(): def test_the_unprovable_half_of_the_cash_posture_is_stated_not_hidden(): """The venue can contradict a cash posture. It cannot confirm one, and the doc must say so. - This test has been re-pointed twice, and both moves are the record of a premise changing - rather than a sentence being reworded. + THIS PIN HAS BEEN RE-POINTED THREE TIMES, and the third move is the most instructive, because + the pin did not fail when it should have. Its first form pinned `_sell_base_size`'s docstring as a proxy for "the SELL is not clamped", expecting #667 to break it. #667 clamped at intent construction and left that docstring standing, so the pin held while the paragraph it guarded went false. Its second form pinned the coinbase adapter having NO posture read, expecting #666 to break - it. #666 did — and the test failing is what forced this rewrite, which is exactly what it - was for. - - What is pinned now is the thing that cannot be engineered away: Coinbase exposes no - cash-versus-margin field for spot, so the check REFUTES and never issues. Closing that needs - a human attestation, not another adapter read — and if one is ever built, this fails again. + it. #666 did, and the failure forced a rewrite -- the pin working as intended. + + Its third form promised, in its own docstring, that "closing that needs a human attestation, + not another adapter read -- and if one is ever built, this fails again". #691 built one. + **The pin passed.** Nothing in it observed the attestation, so it repeated its FIRST failure + mode exactly: a promise in a docstring is not a pin, and prose that matches is not prose that + is true. Hence the assertions below reach for the MECHANISM, not for sentences. + + What is pinned now, and why each half cannot be quietly engineered away: + + * The venue check still only refutes. No adapter read can affirm a spot cash posture; if one + is ever added, the first assertion's premise is false and this section must be rewritten. + * The residual is now carried by an expiring human claim, so the doc must say that rather + than that keel relies on nothing. `_STALE_UNBUILT_CLAIM` catches the old sentence. + * `CashPostureState` must carry no affirmative state. The moment someone adds `CONFIRMED`, + this section's central claim is false in code whatever the prose says. + * The claim must EXPIRE. An attestation with no TTL turns "the operator states it and keel + records it" into "keel remembers something a person said once" -- a weaker guarantee than + the doc describes. + * A rail must READ the record. Without one, "an unattested posture vetoes live entries" is + a sentence about nothing. """ doc = _unwrapped(_doc()) assert _NO_AFFIRMATIVE in doc, ( @@ -490,6 +511,15 @@ def test_the_unprovable_half_of_the_cash_posture_is_stated_not_hidden(): "misreading this section exists to prevent" ) assert "#666" in doc, f"{_DOC} must name the issue the residual belongs to" + assert "#691" in doc, ( + f"{_DOC} must name the issue that CLOSED the residual with an attestation -- a reader " + "left at #666 concludes keel relies on nothing here" + ) + assert _STALE_UNBUILT_CLAIM not in doc, ( + f"{_DOC} still says the attestation is unbuilt; #691 built it (rail 22). This is the " + "exact failure this pin repeated once already: prose left standing after its premise " + "changed" + ) coinbase = "packages/keel-broker-coinbase/keel_broker_coinbase/adapter.py" source = _rel(coinbase) @@ -504,3 +534,21 @@ def test_the_unprovable_half_of_the_cash_posture_is_stated_not_hidden(): assert _CASH_POSTURE_CHECK in _rel(alpaca), ( f"{alpaca} must still carry its own posture check -- the doc says both venues have one" ) + + # THE MECHANISM, not the prose. Removing any of these makes a sentence in the doc false. + from keel_core.cash_posture import ATTESTATION_TTL_SEC, CashPostureState + + assert not hasattr(CashPostureState, "CONFIRMED"), ( + "a `CONFIRMED` cash-posture state would mean keel believes something can AFFIRM a spot " + f"cash account. Nothing can, which is what {_DOC} says -- the state machine and the doc " + "have to agree" + ) + assert ATTESTATION_TTL_SEC > 0, ( + "the attestation must EXPIRE. Without a TTL the doc's 'the operator states it and keel " + "records it' degrades to 'keel remembers what someone said once', a weaker guarantee " + "than this section describes" + ) + assert "cash_posture" in _rel("keel/execution/guards.py"), ( + f"rail 22 is gone; {_DOC} says an unattested posture vetoes live ENTRIES, and with no " + "rail reading the record that sentence is about nothing" + ) From 433bc33024af001a107af37d175e5bd7622c3bc1 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Thu, 3 Sep 2026 18:46:35 -0400 Subject: [PATCH 2/2] fix(brokers): a read-only display must not write to the deployment database (#691) Three findings from the review of #713. The first two were both already argued, in a docstring one function away from the code that violated them. 1. `_cash_posture_records` used `keel.data.db.connect` -- the read-WRITE opener. `venue_readiness._read_only_trade_scope` carries a marked paragraph saying "**Not `keel.data.db.connect`, and that is the whole point of this function**", because `connect` runs `PRAGMA journal_mode = WAL` on whatever it opens. That is a WRITE, from a read-only informational command, and it can leave `-wal`/`-shm` sidecars behind. The `Path.exists()` guard stopped file CREATION and did nothing about the write. Now the `mode=ro` URI shape (#610's seam), with the existence check still in front of it because `mode=ro`'s own refusal is an exception and this path wants an answer. `test_reading_the_records_never_writes_to_the_database` opens the file in DELETE journal mode, reads, and asserts the mode is unchanged and no sidecar appeared -- the observable, rather than an assertion about which function was called. 2. It returned `[]` for BOTH "nothing is attested" and "could not read this database", so the block would advise `keel posture attest --spot-cash` about a record that was fine and merely unread. That harm is named verbatim in the same docstring, and it is WORSE here than for trade scope: re-attesting resets `attested_ts` and `attest_due_ts`, and the TTY gate would ask the operator to affirm a cash account they may not have re-checked. A display bug would become a prompt to make an unverified claim. Now `(records, unreadable)`, the shape that function already uses, and the unreadable block deliberately does NOT print the attest command -- withholding that prompt is the entire point of separating the two answers. "No database at all" stays NOT-unreadable: that is a true statement about the machine rather than an admission of ignorance. 3. `_utc_day` duplicated `posture.py::_utc_date` byte for byte. Imports the existing one -- two formatters for one record type drift the day either gains a timezone or a width. Tests written first, red before green: 5 new, all failing for the right reasons. Mutation-verified, 4 mutants, all killed: the read-write opener restored; the unreadable flag collapsed to False; a missing database reported as unreadable; the unreadable block advising an attestation anyway. Refs #691 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NzuKAe2RVrPt9acVAWjRyL --- keel/commands/brokers.py | 75 +++++++++++----- .../test_brokers_cash_posture_block.py | 87 +++++++++++++++++++ 2 files changed, 141 insertions(+), 21 deletions(-) diff --git a/keel/commands/brokers.py b/keel/commands/brokers.py index 162db717..1e7bf321 100644 --- a/keel/commands/brokers.py +++ b/keel/commands/brokers.py @@ -28,6 +28,7 @@ from __future__ import annotations import json +import sqlite3 import textwrap import time from dataclasses import asdict, dataclass @@ -44,6 +45,7 @@ ) from keel.commands._common import DISCLAIMER +from keel.commands.posture import _utc_date from keel.venue_readiness import VenueReadinessRow, gather_readiness #: The venues a SHIPPED deployment selects -- the whole wired/optional classification, in @@ -342,7 +344,7 @@ def render_readiness_lines(rows: list[VenueReadinessRow]) -> list[str]: def render_cash_posture_lines( - records: list[VenueCashPosture], *, now_ts: int + records: list[VenueCashPosture], *, now_ts: int, unreadable: bool = False ) -> list[str]: """The cash-posture block (#691): what a human has stated about each venue account. @@ -357,6 +359,16 @@ def render_cash_posture_lines( text twice. """ lines = ["", "cash posture (rail 22) -- attested by you, refutable by the venue:"] + if unreadable: + # Deliberately does NOT print the attest command. That prompt is the thing this whole + # distinction exists to withhold: the record may be perfectly good and merely unread, + # and re-attesting over it would reset its clock on an unchecked account. + lines.append( + " a database is present but could not be read -- whether a posture is attested is " + "UNKNOWN, which is not the same as nothing being attested" + ) + lines.append(CASH_POSTURE_HONESTY_LINE) + return lines if not records: lines.append(" no venue has an attested cash posture -- rail 22 vetoes live ENTRIES") lines.append(" next: keel posture attest --spot-cash") @@ -367,7 +379,7 @@ def render_cash_posture_lines( if record.state is CashPostureState.ATTESTED and not record.is_current(now_ts): state = "EXPIRED" expires = ( - _utc_day(record.attest_due_ts) if record.attest_due_ts is not None else "never set" + _utc_date(record.attest_due_ts) if record.attest_due_ts is not None else "never set" ) lines.append( f" {record.venue}: {state} attested={record.attested_posture} expires={expires}" @@ -378,36 +390,54 @@ def render_cash_posture_lines( reason = f": {record.refuted_reason}" if record.refuted_reason else "" lines.append( f" venue evidence contradicted a claim here on " - f"{_utc_day(record.refuted_ts)}{reason}" + f"{_utc_date(record.refuted_ts)}{reason}" ) lines.append(CASH_POSTURE_HONESTY_LINE) return lines -def _utc_day(ts: int) -> str: - from datetime import UTC, datetime - - return datetime.fromtimestamp(ts, tz=UTC).strftime("%Y-%m-%d") - - -def _cash_posture_records(ctx: click.Context) -> list[VenueCashPosture]: - """Read-only, best-effort, never creates or migrates a database -- `_readiness_rows`' - discipline, for the same reason: `keel brokers list` must keep working on a fresh checkout - with no deployment at all, and a display command that created a database as a side effect - would be a surprising one.""" +def _cash_posture_records( + ctx: click.Context, +) -> tuple[list[VenueCashPosture], bool]: + """`(records, unreadable)` -- never raises, never migrates, never writes, never creates a file. + + THE TUPLE IS NOT DECORATION, and `venue_readiness._read_only_trade_scope` already paid for + this lesson one function away: returning a bare empty list for both "nothing is attested" and + "there is a database this process could not read" lets the display assert the former about + the latter, and then advise `keel posture attest --spot-cash`. That is worse here than it is + for trade scope -- re-attesting resets `attested_ts` and `attest_due_ts`, and the TTY gate + would ask the operator to affirm a cash account they may not have re-checked. A display bug + would become a prompt to make an unverified claim. + + ⚠️ **Not `keel.data.db.connect`**, for the reason that function's own docstring gives: it is + the read-WRITE opener, it runs `PRAGMA journal_mode = WAL` on whatever it opens, and a + read-only informational command must not modify the deployment database or leave `-wal`/`-shm` + sidecars behind. The `mode=ro` URI shape (#610's seam) is what a display path uses, with an + existence check in front of it because `mode=ro`'s own refusal is an exception and this path + wants an ANSWER. + """ obj = ctx.obj or {} db_path = obj.get("db_path") if db_path is None or not Path(db_path).exists(): - return [] + # NOT "unreadable": there is no deployment, so "nothing is attested" is a true statement + # about this machine rather than an admission of ignorance. + return [], False + conn = None try: - from keel.data.db import connect from keel.data.repository import Repository - return Repository(connect(str(db_path))).list_venue_cash_postures() + conn = sqlite3.connect(f"file:{db_path}?mode=ro", uri=True) + conn.row_factory = sqlite3.Row + return Repository(conn).list_venue_cash_postures(), False except Exception: - # A missing table (a database older than v18), a locked file, anything: an empty block - # is the honest display, and never a reason to fail a read-only command. - return [] + # A file IS there and this process could not read it: a schema older than v18, a + # permissions problem, or a WAL database whose `-shm` sidecar is absent (every copied + # backup has that shape), which `mode=ro` cannot open because SQLite would have to CREATE + # it. Reported as unknown, never as "nothing is attested". + return [], True + finally: + if conn is not None: + conn.close() def _readiness_rows(ctx: click.Context) -> list[VenueReadinessRow]: @@ -479,7 +509,10 @@ def brokers_list(ctx: click.Context, as_json: bool) -> None: click.echo(line) for line in render_readiness_lines(_readiness_rows(ctx)): click.echo(line) - for line in render_cash_posture_lines(_cash_posture_records(ctx), now_ts=int(time.time())): + posture_records, posture_unreadable = _cash_posture_records(ctx) + for line in render_cash_posture_lines( + posture_records, now_ts=int(time.time()), unreadable=posture_unreadable + ): click.echo(line) click.echo("") click.echo(DISCLAIMER) diff --git a/tests/commands/test_brokers_cash_posture_block.py b/tests/commands/test_brokers_cash_posture_block.py index 5cf18d79..82f85490 100644 --- a/tests/commands/test_brokers_cash_posture_block.py +++ b/tests/commands/test_brokers_cash_posture_block.py @@ -112,3 +112,90 @@ def test_venues_are_listed_in_a_stable_order() -> None: [_record(venue="alpaca"), _record(venue="robinhood"), _record(venue="coinbase")] ) assert text.index("alpaca") < text.index("coinbase") < text.index("robinhood") + + +# --- the read path: unreadable is not the same as nothing, and reading must not WRITE ---------- +# +# Both pins come from `venue_readiness._read_only_trade_scope`'s own docstring, which sits one +# function away from the code they guard. The first version of `_cash_posture_records` violated +# both: it used `keel.data.db.connect` (the read-WRITE opener that sets `journal_mode = WAL`) and +# returned `[]` for "no record" and "could not read" alike. + + +def test_reading_the_records_never_writes_to_the_database(tmp_path) -> None: + """A read-only display command must not modify the deployment database. + + `keel.data.db.connect` runs `PRAGMA journal_mode = WAL`, which is a WRITE -- it changes the + file and can leave `-wal`/`-shm` sidecars behind. `_read_only_trade_scope`'s docstring says + "**Not `keel.data.db.connect`, and that is the whole point of this function**". The journal + mode is the observable: open the database in DELETE mode, read, and it must still be DELETE. + """ + import sqlite3 + + from keel.commands.brokers import _cash_posture_records + from keel.data.db import migrate + + db = tmp_path / "keel.db" + conn = sqlite3.connect(str(db)) + conn.row_factory = sqlite3.Row + migrate(conn) + conn.execute("PRAGMA journal_mode = DELETE") + conn.commit() + conn.close() + + records, unreadable = _cash_posture_records(_ctx(db)) + assert records == [] and not unreadable + + probe = sqlite3.connect(str(db)) + mode = probe.execute("PRAGMA journal_mode").fetchone()[0] + probe.close() + assert mode.lower() == "delete", f"the read switched the journal mode to {mode!r}" + assert not (tmp_path / "keel.db-wal").exists(), "the read left a -wal sidecar behind" + + +def test_an_unreadable_database_is_reported_as_unknown_not_as_nothing_attested(tmp_path) -> None: + """`_read_only_trade_scope`'s docstring names the harm exactly: conflating these two "let the + display assert the former about the latter, and then advise `keel scope attest`". It is worse + here -- re-attesting resets `attested_ts` and `attest_due_ts`, and the TTY gate would ask the + operator to affirm a cash account they may not have re-checked. A display bug would become a + prompt to make an unverified claim. + """ + from keel.commands.brokers import _cash_posture_records + + db = tmp_path / "keel.db" + db.write_bytes(b"this is not a sqlite database") + records, unreadable = _cash_posture_records(_ctx(db)) + assert records == [] + assert unreadable is True + + +def test_no_database_at_all_is_not_unreadable(tmp_path) -> None: + """"There is no deployment" is a true statement about this machine, not an admission of + ignorance -- the same distinction `_read_only_trade_scope` draws in its first branch.""" + from keel.commands.brokers import _cash_posture_records + + records, unreadable = _cash_posture_records(_ctx(tmp_path / "absent.db")) + assert records == [] + assert unreadable is False + + +def test_the_block_says_it_could_not_read_rather_than_advising_an_attestation() -> None: + """And critically, it must NOT print the attest command: that is the prompt this whole + distinction exists to withhold.""" + text = "\n".join(render_cash_posture_lines([], now_ts=NOW, unreadable=True)) + assert "could not" in text.lower() or "unreadable" in text.lower() + assert "keel posture attest" not in text + + +def test_the_block_still_advises_attesting_when_there_genuinely_is_no_record() -> None: + text = "\n".join(render_cash_posture_lines([], now_ts=NOW, unreadable=False)) + assert "keel posture attest" in text + + +def _ctx(db_path): + """A minimal stand-in for the click context `_cash_posture_records` reads `db_path` from.""" + + class _Ctx: + obj = {"db_path": str(db_path)} + + return _Ctx()