Skip to content

Commit bd0b173

Browse files
FlyM1ssclaude
andcommitted
docs: merge/branch discipline + fail-closed≠fail-visible rules
PR #107 merged without the fix meant to ship with it. Forensics: not a race — GitHub's tip never moved. The fix commit was authored 17min after the merge and was local-only; the "merge after AF #359" gate was commented 41min after the PR closed. The merge was correct given every published signal. The defect was that the intent was never published. The follow-ups then landed on the already-merged branch and orphaned silently until they were re-landed as #110. Records the rules that would have prevented it, plus the reason nobody noticed for hours: the news adapter's fail-closed fallback makes "endpoint absent" and "every story rejected" byte-identical, and its fixtures were built from the adapter's own field names, so CI stayed green through a live prod breakage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MUHBXCWZYPi2PMExC41D6r
1 parent 8d62693 commit bd0b173

1 file changed

Lines changed: 16 additions & 0 deletions

File tree

CLAUDE.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,22 @@ Two step-driven agent surfaces coexist; they are **not peers**:
101101
- **Frontend** → Vercel (`vercel.json`): static `dashboard/frontend`.
102102
- **Container** (`Dockerfile`): `WORKDIR /app`; `uvicorn dashboard.backend.app:app`.
103103

104+
## Merge & branch discipline
105+
106+
**`main` has no branch protection, no required checks, and no CODEOWNERS.** Nothing gates a merge. Any collaborator can merge any open PR at any moment, and the observed norm is that they do — unreviewed, and over red CI. Merging to `main` also auto-deploys prod (see the Deployment gotcha). Treat every open PR as merge-able *right now* by someone who has not read your plan.
107+
108+
- **Never push follow-up work to a branch whose PR is already merged.** Cut a new branch. GitHub gives **no notification, no reopening, and no warning** when commits land behind a merged PR — they orphan silently, and the only signal is a human noticing the branch is ahead of the PR that consumed it. (This is exactly how PR #107 shipped without the fix that was meant to be part of it; the follow-ups had to be re-landed as #110 off the same ref.) Check before pushing: `gh pr list --head <branch> --state all`.
109+
- **If a PR must not merge yet, publish that where GitHub shows or enforces it****open it as a draft**, or add a `blocked` label, and put the gate as an imperative in the *first line of the body* ("DO NOT MERGE until X ships"). A comment is not a gate, and a body that explains why the change is *safe* to land early ("depends on X, but falls back transparently until then") reads as *please merge me*. A gating instruction posted after the merge is worthless — intent that only exists in a local worktree or an agent session's memory does not exist.
110+
- **Never record in notes/memory that a merge was sequenced deliberately unless a session actually verified and pressed the button.** Check `gh api repos/Open-Finance-Lab/AgenticTrading/pulls/N --jq '.merged_by.login'`. Writing down a gate that nobody applied teaches every later reader that the gate works.
111+
112+
### Fail-closed is not fail-visible
113+
114+
The FinSearch news adapter (`dashboard/backend/integrations/news_sentiment.py`) is the cautionary case. `get_latest_panel_payload`'s `if not feed:` fallback to the Phase-A representative feed makes **"the upstream endpoint isn't deployed"** and **"the endpoint is live and every story is being silently rejected"** produce a byte-identical `status: ok` HTTP 200. The 404 path logs *nothing* (a bare `pass`). A field rename upstream therefore degraded prod for hours with no error, no metric, and a green test suite.
115+
116+
- When adding a fallback, ask **what distinguishes *absent* from *broken***. If nothing does, log ERROR at the wholesale-drift boundary (a per-item warning cannot report a total contract break).
117+
- **Never build an upstream's fixture from your own adapter's field names.** Fixtures written that way test the mapper against itself and drift with the code, so a producer rename stays green forever. Pin the shape from a real recorded response (`dashboard/backend/tests/fixtures/items-wire-fixture.json`).
118+
- **Mocked coverage cannot detect a cross-repo producer rename** — the producer is mocked. Only a canary against the live endpoint can. Don't mistake more mock tests for coverage of this seam.
119+
104120
## Gotchas
105121

106122
- Root `pyproject.toml` (`finagent-orchestration`) is for the **orchestration** subsystem, not the dashboard — edit `requirements.txt` for dashboard deps.

0 commit comments

Comments
 (0)