fix: close the three residual risks from the 2026-09-06 merges

Each of the three was merged on evidence I could not reach myself. This
replaces that trust with a test that runs on every platform, on every CI run.

1. Windows lock (#1261). The no-sentinel fix was unverifiable on POSIX —
   msvcrt is None, so the branch never executes and a regression would be
   invisible to Linux and macOS CI. tests/test_windows_lock_no_sentinel.py
   swaps the module-level fcntl/msvcrt handles for stubs and drives the
   Windows branch anywhere, asserting behaviour (an empty file stays empty,
   the first ledger append succeeds and verify_chain passes) rather than
   'was locking() called'. Restoring the sentinel in all three lock sites
   fails 3 of the 4 tests, including the first-append regression itself.

2. Dividend cash column (#1356). A dividend row has no quantity and no price,
   so 'amount or qty * price' collapses to 0.0 when the export books the
   payout under a cash-movement heading instead of the fill column. That is a
   silent zero — the shape #1207 item 15 is about. Dividend rows now also try
   发生金额 / 资金发生额 / 红利金额 / 实发金额 / 税后金额 and Futu's net-amount
   columns, across all four parsers; anything still zero is counted into a new
   dividend_rows_missing_cash profile field so the caller can SEE that the
   journal was read but the money was not.

3. Docker action majors (#1359). The v4/v6/v7 bump had never executed in this
   repository: docker-build.yml is workflow_dispatch only, total_runs=0. Every
   input the workflow passes was checked against each new major's action.yml
   at the pinned SHA (13 inputs, all present; the two setup actions take none).
   A validate-build job now runs the SAME pins with push: false on any change
   to the Dockerfile, this workflow, or pyproject, and the publish job is
   gated to workflow_dispatch so the new triggers cannot publish an image.
   That gate is one 'if:' line, so it is pinned too — removing it, flipping
   validate-build to push: true, or unpinning an action each fail a test.

The file comment claiming these majors were 'a deliberate, separately testable
bump, not a drive-by change here' contradicted the file after #1359 and is
replaced by what is actually true now.
This commit is contained in:
Haozhe Wu committed 2026-09-06 20:08:31 +08:00
1 parent 2308f25738
commit ff6695912c
6 files changed
+451 -8

No files matched your search

+66 -2
View File
@@ -1,8 +1,15 @@
name: Build and Push Docker Image
# Actions are pinned to immutable commit SHAs, matching every other workflow in
# this repository. Newer majors exist for the docker/* actions (v4/v6/v7); those
# are a deliberate, separately testable bump, not a drive-by change here.
# this repository. The docker/* actions were moved to v4/v6/v7 in #1359; every
# input this workflow passes was checked against each new major's action.yml at
# the pinned SHA before the bump landed.
#
# The publish job runs on workflow_dispatch only, so it is exercised roughly
# once per release. That is why `validate-build` below exists: a bad pin or a
# broken Dockerfile would otherwise be discovered by whoever is mid-release,
# not by CI. It runs the SAME four actions with `push: false`, on every change
# to the Dockerfile, this workflow, or the packaging metadata.
on:
workflow_dispatch:
@@ -11,6 +18,19 @@ on:
description: 'Image tag (e.g., v1.0.0, latest)'
required: true
default: 'latest'
pull_request:
paths:
- 'Dockerfile'
- '.dockerignore'
- '.github/workflows/docker-build.yml'
- 'pyproject.toml'
push:
branches: [main]
paths:
- 'Dockerfile'
- '.dockerignore'
- '.github/workflows/docker-build.yml'
- 'pyproject.toml'
env:
GHCR_REGISTRY: ghcr.io
@@ -19,7 +39,51 @@ env:
DOCKERHUB_IMAGE_NAME: ${{ secrets.DOCKERHUB_USERNAME }}/${{ github.event.repository.name }}
jobs:
# Exercises the pinned docker/* actions and the Dockerfile without pushing
# anything. Single platform on purpose: this is a "does it still build and do
# the actions still accept these inputs" check, not a release build, and
# emulating arm64 here would triple the runtime for no extra signal.
validate-build:
if: github.event_name != 'workflow_dispatch'
runs-on: ubuntu-24.04
permissions:
contents: read
steps:
- name: Checkout repository
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4.3.0
- name: Set up QEMU
uses: docker/setup-qemu-action@1f40c72289eff860ee54a304f1438e3cff362e0a # v4.3.0
- name: Extract metadata
id: meta-validate
uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0
with:
images: ${{ env.GHCR_REGISTRY }}/${{ env.GHCR_IMAGE_NAME }}
tags: |
type=raw,value=ci-validate
- name: Build without pushing
uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0
with:
context: .
file: Dockerfile
platforms: linux/amd64
push: false
tags: ${{ steps.meta-validate.outputs.tags }}
labels: ${{ steps.meta-validate.outputs.labels }}
# Deliberately no GitHub Actions cache here. `type=gha` export needs
# a wider token than this job's `contents: read`, and a cache
# misconfiguration failing the run would make the check a source of
# false alarms rather than a guard. This job is path-filtered and
# rare; a cold build is the cheaper trade.
build-and-push:
if: github.event_name == 'workflow_dispatch'
runs-on: ubuntu-24.04
permissions:
contents: read
+60 -5
View File
@@ -209,6 +209,48 @@ def _normalize_side(raw: Any) -> str:
raise ValueError(f"Unsupported trade side: {raw!r}")
# A cash-dividend row carries no quantity and no price, so the usual
# ``amount or qty * price`` fallback collapses to 0.0 when the export names the
# cash column differently from a fill's. Brokers do exactly that: 同花顺 and
# 东财 book the payout under a cash-movement heading (发生金额 / 资金发生额 /
# 红利金额 / 实发金额) rather than under 成交金额, and Futu uses a net-amount
# column. Reading only the fill column would book the dividend as 0 SILENTLY —
# the "successful run recorded as zero" shape this parser exists to avoid — so
# a dividend row tries the cash headings too, and anything still zero is
# surfaced by ``_compute_profile`` rather than averaged in as a real zero.
_DIVIDEND_AMOUNT_KEYS: tuple[str, ...] = (
"成交金额",
"发生金额",
"资金发生额",
"红利金额",
"实发金额",
"税后金额",
"Amount",
"Net Amount",
"NetAmount",
)
def _dividend_amount(row: Any, primary: float) -> float:
"""Return a dividend row's cash, trying the cash-movement headings.
Args:
row: One export row (mapping-like, from ``DataFrame.iterrows``).
primary: Amount already derived from the fill columns.
Returns:
The first non-zero candidate, else ``0.0`` (surfaced downstream, never
silently treated as a real zero payout).
"""
if primary:
return primary
for key in _DIVIDEND_AMOUNT_KEYS:
value = _to_float(row.get(key))
if value:
return value
return 0.0
def _is_skippable_side(raw: Any) -> bool:
"""True for known non-trade corporate-action rows; callers drop the row."""
if raw is None:
@@ -286,12 +328,15 @@ def parse_tonghuashun(df: pd.DataFrame) -> list[TradeRecord]:
qty = _to_float(row.get("成交数量"))
price = _to_float(row.get("成交价格"))
amount = _to_float(row.get("成交金额")) or qty * price
side = _normalize_side(row.get("操作"))
if side == "dividend":
amount = _dividend_amount(row, amount)
fee = _to_float(row.get("手续费")) + _to_float(row.get("印花税")) + _to_float(row.get("过户费"))
records.append(TradeRecord(
datetime=_ths_datetime(row.get("成交时间", "")),
symbol=_qualify_a_share(raw_code),
name=str(row.get("证券名称", "")).strip(),
side=_normalize_side(row.get("操作")),
side=side,
quantity=qty,
price=price,
amount=amount,
@@ -364,12 +409,15 @@ def parse_eastmoney(df: pd.DataFrame) -> list[TradeRecord]:
qty = _to_float(row.get("成交数量"))
price = _to_float(row.get("成交均价"))
amount = _to_float(row.get("成交金额")) or qty * price
side = _normalize_side(row.get("买卖标志"))
if side == "dividend":
amount = _dividend_amount(row, amount)
fee = _to_float(row.get("佣金")) + _to_float(row.get("印花税"))
records.append(TradeRecord(
datetime=dt,
symbol=_qualify_a_share(raw_code),
name=str(row.get("股票名称", "")).strip(),
side=_normalize_side(row.get("买卖标志")),
side=side,
quantity=qty,
price=price,
amount=amount,
@@ -471,12 +519,15 @@ def parse_futu(df: pd.DataFrame) -> list[TradeRecord]:
qty = _to_float(row.get("Quantity"))
price = _to_float(row.get("Price"))
amount = _to_float(row.get("Amount")) or qty * price
futu_side = _normalize_side(side_raw)
if futu_side == "dividend":
amount = _dividend_amount(row, amount)
fee = _to_float(row.get("Commission")) + _to_float(row.get("Platform Fee"))
records.append(TradeRecord(
datetime=dt,
symbol=symbol,
name=str(row.get("Name", "")).strip(),
side=_normalize_side(side_raw),
side=futu_side,
quantity=qty,
price=price,
amount=amount,
@@ -538,15 +589,19 @@ def parse_generic(df: pd.DataFrame) -> list[TradeRecord]:
price = _to_float(row.get(price_col)) if price_col else 0.0
amount = _to_float(row.get(amount_col)) if amount_col else qty * price
fee = _to_float(row.get(fee_col)) if fee_col else 0.0
generic_side = _normalize_side(row.get(side_col))
generic_amount = amount or qty * price
if generic_side == "dividend":
generic_amount = _dividend_amount(row, generic_amount)
market = _infer_market_from_symbol(symbol)
records.append(TradeRecord(
datetime=dt,
symbol=symbol.upper(),
name=str(row.get(name_col, "")).strip() if name_col else "",
side=_normalize_side(row.get(side_col)),
side=generic_side,
quantity=qty,
price=price,
amount=amount or qty * price,
amount=generic_amount,
fee=fee,
market=market,
))
+10 -1
View File
@@ -233,7 +233,8 @@ def _compute_profile(df: pd.DataFrame) -> dict[str, Any]:
Returns:
Dict with avg_holding_days, trade_frequency_per_week, win_rate,
profit_loss_ratio, total_pnl, total_dividends, max_drawdown,
profit_loss_ratio, total_pnl, total_dividends,
dividend_rows_missing_cash, max_drawdown,
top_symbols, market_distribution, hourly_distribution,
roundtrips_sample.
"""
@@ -246,6 +247,13 @@ def _compute_profile(df: pd.DataFrame) -> dict[str, Any]:
trades_df = df[df["side"].isin(("buy", "sell"))]
dividend_amounts = df.loc[df["side"] == "dividend", "amount"]
total_dividends = round(float(dividend_amounts.sum()), 2)
# A dividend row that parsed with no cash is almost never a real zero
# payout — it means the export named its cash column something the parser
# did not recognise, and the payout would otherwise be booked as 0 with
# nothing to show for it. Surface the count so the caller can see that the
# journal was read but the money was not, instead of trusting a total that
# silently omits it.
dividend_rows_missing_cash = int((dividend_amounts == 0).sum())
total_trades = len(trades_df)
span_days = max(1, (df["datetime"].max() - df["datetime"].min()).days)
@@ -300,6 +308,7 @@ def _compute_profile(df: pd.DataFrame) -> dict[str, Any]:
"profit_loss_ratio": pnl_ratio,
"total_pnl": total_pnl,
"total_dividends": total_dividends,
"dividend_rows_missing_cash": dividend_rows_missing_cash,
"max_drawdown": max_drawdown,
"top_symbols": top_symbols,
"market_distribution": market_dist,
@@ -0,0 +1,116 @@
"""The Docker publish job must stay manual; the validation job must never push.
`docker-build.yml` gained `pull_request` / `push` triggers so the pinned
docker/* actions and the Dockerfile are exercised by CI rather than discovered
broken by whoever is mid-release. That is only safe while the publishing job
stays gated to `workflow_dispatch`: without the gate, those new triggers would
publish an image to GHCR and Docker Hub on every commit that touches the
Dockerfile.
The guard is one `if:` expression, which is exactly the kind of line a later
edit drops silently — nothing else in the repository would notice. These tests
are offline and read the workflow as data.
"""
from __future__ import annotations
from pathlib import Path
import yaml
_WORKFLOW = Path(__file__).resolve().parents[2] / ".github" / "workflows" / "docker-build.yml"
def _workflow() -> dict:
parsed = yaml.safe_load(_WORKFLOW.read_text(encoding="utf-8"))
assert isinstance(parsed, dict), "workflow did not parse as a mapping"
return parsed
def _triggers(parsed: dict) -> dict:
# PyYAML parses a bare `on:` key as the boolean True.
return parsed[True] if True in parsed else parsed["on"]
def _push_flags(job: dict) -> list:
return [
step.get("with", {}).get("push")
for step in job["steps"]
if "build-push-action" in str(step.get("uses", ""))
]
def test_publishing_job_is_workflow_dispatch_only() -> None:
"""A commit touching the Dockerfile must not be able to publish an image."""
jobs = _workflow()["jobs"]
publishing = {
name: job for name, job in jobs.items() if True in _push_flags(job)
}
assert publishing, "no publishing job found — did the push flag change?"
for name, job in publishing.items():
assert job.get("if") == "github.event_name == 'workflow_dispatch'", (
f"job {name!r} pushes images but is not gated to workflow_dispatch; "
"with the pull_request/push triggers present this publishes on "
"every Dockerfile change"
)
def test_validation_job_never_pushes() -> None:
"""The job that runs on ordinary commits must build only."""
jobs = _workflow()["jobs"]
assert "validate-build" in jobs, "the no-push validation job was removed"
flags = _push_flags(jobs["validate-build"])
assert flags, "validate-build no longer runs build-push-action"
assert all(flag is False for flag in flags), (
f"validate-build must pass push: false, got {flags}"
)
def test_validation_job_uses_the_same_action_pins_as_the_publish_job() -> None:
"""A validation that ran different versions would prove nothing.
The point of this job is to exercise the pins the release build uses, so a
drift between the two makes the check decorative.
"""
jobs = _workflow()["jobs"]
def pins(job: dict) -> set[str]:
return {
str(step["uses"])
for step in job["steps"]
if str(step.get("uses", "")).startswith("docker/")
}
validate = pins(jobs["validate-build"])
publish = pins(jobs["build-and-push"])
shared = {p.split("@")[0] for p in validate} & {p.split("@")[0] for p in publish}
assert shared, "the two jobs share no docker/* actions"
for action in shared:
v = {p for p in validate if p.startswith(action + "@")}
p_ = {p for p in publish if p.startswith(action + "@")}
assert v == p_, f"{action} is pinned differently: validate={v} publish={p_}"
def test_every_docker_action_is_pinned_to_a_sha() -> None:
"""A moving tag would silently change what the release build runs."""
parsed = _workflow()
for name, job in parsed["jobs"].items():
for step in job["steps"]:
uses = str(step.get("uses", ""))
if not uses.startswith("docker/"):
continue
ref = uses.split("@", 1)[1]
assert len(ref) == 40 and all(
c in "0123456789abcdef" for c in ref
), f"{name}: {uses} is not pinned to a full commit SHA"
def test_triggers_include_the_paths_that_can_break_the_image() -> None:
"""Without these paths the validation job never runs on the real changes."""
triggers = _triggers(_workflow())
for event in ("pull_request", "push"):
assert event in triggers, f"{event} trigger missing"
paths = set(triggers[event]["paths"])
assert {"Dockerfile", ".github/workflows/docker-build.yml"} <= paths, (
f"{event} paths do not cover the Dockerfile and this workflow: {paths}"
)
+68
View File
@@ -824,3 +824,71 @@ def test_parse_generic_currency_price_not_zero() -> None:
assert rec[0].price == 150.25
assert rec[0].fee == 1.0
assert rec[0].amount == 1502.5
# ── M3d: a dividend's cash may not live in the fill column ────────────────
def test_dividend_cash_read_from_a_cash_movement_column(tmp_path: Path) -> None:
"""A 红利入账 row books its payout even when 成交金额 is not where it sits.
A dividend row has no quantity and no price, so the usual
``成交金额 or qty * price`` fallback collapses to 0.0. Brokers book the
payout under a cash-movement heading instead, and reading only the fill
column would record the dividend as 0 silently — the exact
"run succeeded, number is zero" shape #1207 item 15 is about.
"""
for cash_column in ("发生金额", "资金发生额", "红利金额", "实发金额"):
csv = tmp_path / f"ths_{cash_column}.csv"
csv.write_text(
"成交时间,证券代码,证券名称,操作,成交数量,成交价格,成交金额,"
f"{cash_column},手续费,印花税,过户费\n"
"2026-01-02 09:35:00,600519,贵州茅台,买入,100,1700,170000,0,5,0,0.1\n"
"2026-01-08 10:00:00,600519,贵州茅台,卖出,100,1800,180000,0,5,180,0.1\n"
# the fill column is empty for a payout; the money is in the other one
"2026-01-10 09:00:00,600519,贵州茅台,红利入账,0,0,0,500,0,0,0\n",
encoding="utf-8",
)
_fmt, records = parse_file(csv)
payouts = [r for r in records if r.side == "dividend"]
assert len(payouts) == 1, cash_column
assert payouts[0].amount == pytest.approx(500.0), (
f"{cash_column}: dividend cash silently booked as {payouts[0].amount}"
)
def test_unreadable_dividend_cash_is_surfaced_not_silently_zero() -> None:
"""A zero-cash dividend row must be counted, not averaged in as a real 0.
If a broker names the cash column something none of the parsers know, the
payout still cannot be read — but the caller has to be able to SEE that,
rather than trust a total that quietly omits the money.
"""
profile = _compute_profile(
_df(
[
_rec("2026-01-01 10:00:00", "A.SH", "buy", 100, 10),
_rec("2026-01-05 10:00:00", "A.SH", "sell", 100, 12),
_dividend_rec("2026-01-10 09:00:00", "A.SH", 0.0),
_dividend_rec("2026-01-11 09:00:00", "A.SH", 500.0),
]
)
)
assert profile["dividend_rows_missing_cash"] == 1
assert profile["total_dividends"] == 500.0
assert profile["total_pnl"] == 700.0
def test_clean_journal_reports_no_missing_dividend_cash() -> None:
"""The counter must not fire on a journal that parsed correctly."""
profile = _compute_profile(
_df(
[
_rec("2026-01-01 10:00:00", "A.SH", "buy", 100, 10),
_rec("2026-01-05 10:00:00", "A.SH", "sell", 100, 12),
_dividend_rec("2026-01-10 09:00:00", "A.SH", 500.0),
]
)
)
assert profile["dividend_rows_missing_cash"] == 0
@@ -0,0 +1,131 @@
"""The Windows lock path must not write a sentinel byte into an empty file.
`msvcrt.locking` locks a byte RANGE, and on Windows a range beyond EOF is a
valid lock target — so an empty file can be locked at offset 0 without writing
anything. The previous implementation seeked to EOF and wrote ``b"\\0"`` first
to give itself a byte to lock. For the compliance ledger that sentinel is a
malformed line 0, so `_walk_chain` raised `LedgerCorruptionError` on the very
first append and the ledger was unusable on Windows.
These tests exist because that path is unreachable on POSIX: `msvcrt` is None
here, so the branch never runs and a regression would be invisible to every
non-Windows contributor and to the Linux and macOS CI jobs. Rather than trust
a reviewer's Windows run, the module-level `fcntl` / `msvcrt` handles are
swapped for stubs so the Windows branch executes on every platform.
The assertions are deliberately behavioural, not "was locking() called":
a test that only checks the call would stay green if the sentinel came back.
"""
from __future__ import annotations
from pathlib import Path
import pytest
import src.governance.ledger as ledger_mod
import src.live.daily_count as daily_count_mod
import src.providers.openai_codex as codex_mod
from src.governance.ledger import append_record, verify_chain
class _StubMsvcrt:
"""Records lock calls; never touches a real Windows lock."""
LK_LOCK = 1
LK_NBLCK = 2
LK_UNLCK = 0
def __init__(self) -> None:
self.calls: list[tuple[int, int]] = []
def locking(self, fd: int, mode: int, nbytes: int) -> None:
self.calls.append((mode, nbytes))
@pytest.fixture()
def windows_locks(monkeypatch: pytest.MonkeyPatch) -> _StubMsvcrt:
"""Force every lock helper down its Windows branch on any platform."""
stub = _StubMsvcrt()
for module in (ledger_mod, daily_count_mod, codex_mod):
monkeypatch.setattr(module, "fcntl", None, raising=False)
monkeypatch.setattr(module, "msvcrt", stub, raising=False)
return stub
def test_first_ledger_append_succeeds_on_the_windows_path(
tmp_path: Path, windows_locks: _StubMsvcrt
) -> None:
"""The regression itself: a sentinel byte makes line 0 malformed.
With the sentinel, this raised LedgerCorruptionError on the first append
and the compliance ledger could never be started on Windows.
"""
path = tmp_path / "audit.jsonl"
first = append_record(path, {"event": "first"}, fsync=False)
second = append_record(path, {"event": "second"}, fsync=False)
assert first["seq"] == 1
assert first["prev_record_hash"] == ledger_mod.GENESIS_PREV_HASH
assert second["prev_record_hash"] == first["record_hash"]
result = verify_chain(path)
assert result.ok is True, result.first_break
assert result.record_count == 2
assert windows_locks.calls, "the Windows lock branch did not execute"
def test_locking_an_empty_file_writes_nothing(
tmp_path: Path, windows_locks: _StubMsvcrt
) -> None:
"""Locking must leave a zero-byte file zero-byte.
This is the property the ledger test depends on, asserted directly so a
regression names the cause rather than only its downstream symptom.
"""
path = tmp_path / "empty.bin"
path.touch()
assert path.stat().st_size == 0
with path.open("r+b") as handle:
ledger_mod._lock_exclusive(handle)
ledger_mod._unlock(handle)
assert path.stat().st_size == 0, "the lock wrote a sentinel byte"
def test_daily_count_and_codex_locks_also_write_nothing(
tmp_path: Path, windows_locks: _StubMsvcrt
) -> None:
"""The same fix landed in three places; pin all three.
``daily_count`` gates the mandate's trades-per-day cap and
``openai_codex`` guards a cached token file — a stray byte in either is a
parse failure on the next read, not a cosmetic difference.
"""
for name, locker in (
("daily.json", daily_count_mod._try_lock),
("codex_token.json", codex_mod._lock_token_file),
):
path = tmp_path / name
path.touch()
with path.open("r+b") as handle:
locker(handle)
assert path.stat().st_size == 0, f"{name}: the lock wrote a sentinel byte"
def test_locks_target_offset_zero(
tmp_path: Path, windows_locks: _StubMsvcrt
) -> None:
"""A one-byte range at offset 0 is what makes the beyond-EOF lock valid."""
path = tmp_path / "offset.bin"
path.touch()
with path.open("r+b") as handle:
handle.seek(64) # a stale position must not become the lock offset
ledger_mod._lock_exclusive(handle)
assert handle.tell() == 0, "lock did not seek to offset 0 first"
ledger_mod._unlock(handle)
assert [nbytes for _, nbytes in windows_locks.calls] == [1, 1]