Skip to content

Commit 0a78caa

Browse files
DrBaherclaude
andcommitted
Merge main into fix/vacuous-approve
Resolves two doc conflicts with #14 (signer policy), which landed first: - CHANGELOG.md: both PRs added a `### Fixed` and `### Added` under [Unreleased]. Merged into one of each, signer-policy entries first since they are already on main. - README.md: both added a row to the Core concepts table. Kept both. nda_review_cli.py auto-merged cleanly -- #14 touches the negotiate red-flag call sites and sign-off, #15 touches the review engine. 200 tests pass and `make smoke` is green on the merged tree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 parents 7050b37 + 07282c6 commit 0a78caa

13 files changed

Lines changed: 1485 additions & 12 deletions

File tree

.github/workflows/ci.yml

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,9 @@ jobs:
166166
--out "$STATE"
167167
./nda_review_cli.py negotiate counter --base "${RUNNER_TEMP}/neg/b" --state "$STATE" --auto
168168
./nda_review_cli.py negotiate accept --base "${RUNNER_TEMP}/neg/a" --state "$STATE" --as a
169+
# Snapshot the converged state: the signer-policy step below needs
170+
# status == converged, and by the end of this flow it is finalized.
171+
cp "$STATE" "${RUNNER_TEMP}/neg/converged.json"
169172
./nda_review_cli.py negotiate sign-off --base "${RUNNER_TEMP}/neg/a" --state "$STATE" --as a --yes
170173
./nda_review_cli.py negotiate sign-off --base "${RUNNER_TEMP}/neg/b" --state "$STATE" --as b --yes
171174
./nda_review_cli.py negotiate finalize \
@@ -181,6 +184,96 @@ jobs:
181184
assert 'a' in state['signoffs'] and 'b' in state['signoffs']
182185
print('finalized state OK')
183186
"
187+
- name: Signer policy gates unattended sign-off
188+
run: |
189+
CONVERGED="${RUNNER_TEMP}/neg/converged.json"
190+
BASE_A="${RUNNER_TEMP}/neg/a"
191+
PERMISSIVE="${RUNNER_TEMP}/neg/permissive.json"
192+
193+
# expect_exit <want> <label> <cmd...>
194+
expect_exit() {
195+
want="$1"; label="$2"; shift 2
196+
rc=0
197+
"$@" >/dev/null 2>&1 || rc=$?
198+
if [ "$rc" != "$want" ]; then
199+
echo "FAIL: $label — expected exit $want, got $rc"
200+
exit 1
201+
fi
202+
echo "ok: $label (exit $rc)"
203+
}
204+
205+
# 1. Deny by default: absent a policy the agent has no signing authority.
206+
test ! -f "$BASE_A/config/signer-policy.json"
207+
expect_exit 4 "no policy -> MISSING_SIGNER_POLICY" \
208+
./nda_review_cli.py signer policy run --base "$BASE_A" --state "$CONVERGED"
209+
210+
# 2. The shipped example escalates this negotiation: round 2 was drafted
211+
# by `auto:middleground`, which is not in the default source whitelist.
212+
expect_exit 3 "example policy -> escalate" \
213+
./nda_review_cli.py signer policy run --base "$BASE_A" --state "$CONVERGED" \
214+
--signer-policy config/signer-policy.json.example
215+
216+
# 3. Whitelisting `auto` (deterministic, no LLM) reaches allow. If this ever
217+
# starts failing, the gate has stopped discriminating.
218+
python3 -c "
219+
import json
220+
json.dump({
221+
'version': '0.1.0',
222+
'name': 'ci-trusts-auto',
223+
'red_flags': 'introduced',
224+
'allowed_amendment_sources': ['initial', 'accept', 'manual', 'auto'],
225+
}, open('${RUNNER_TEMP}/neg/permissive.json', 'w'))
226+
"
227+
expect_exit 0 "permissive policy -> allow" \
228+
./nda_review_cli.py signer policy run --base "$BASE_A" --state "$CONVERGED" \
229+
--signer-policy "$PERMISSIVE"
230+
231+
# 4. `run` is read-only.
232+
before=$(sha256sum "$CONVERGED" | cut -d' ' -f1)
233+
./nda_review_cli.py signer policy run --base "$BASE_A" --state "$CONVERGED" \
234+
--signer-policy "$PERMISSIVE" >/dev/null 2>&1
235+
test "$before" = "$(sha256sum "$CONVERGED" | cut -d' ' -f1)"
236+
echo "ok: signer policy run did not mutate the state file"
237+
238+
# 5. Fail closed: an escalating verdict refuses sign-off even under --yes,
239+
# and leaves the state byte-identical.
240+
cp "$CONVERGED" "${RUNNER_TEMP}/neg/failclosed.json"
241+
before=$(sha256sum "${RUNNER_TEMP}/neg/failclosed.json" | cut -d' ' -f1)
242+
expect_exit 3 "sign-off escalates under --yes (fail closed)" \
243+
./nda_review_cli.py negotiate sign-off --base "$BASE_A" \
244+
--state "${RUNNER_TEMP}/neg/failclosed.json" --as a --yes \
245+
--signer-policy config/signer-policy.json.example
246+
test "$before" = "$(sha256sum "${RUNNER_TEMP}/neg/failclosed.json" | cut -d' ' -f1)"
247+
echo "ok: state untouched on escalate"
248+
249+
# 6. Allow path signs unattended, records the policy digest, and keeps the
250+
# hash chain intact.
251+
cp "$CONVERGED" "${RUNNER_TEMP}/neg/allowed.json"
252+
./nda_review_cli.py negotiate sign-off --base "$BASE_A" \
253+
--state "${RUNNER_TEMP}/neg/allowed.json" --as a --signer-policy "$PERMISSIVE" >/dev/null
254+
python3 -c "
255+
import hashlib, json
256+
state = json.load(open('${RUNNER_TEMP}/neg/allowed.json'))
257+
rec = state['signoffs']['a']
258+
assert rec['method'] == 'signer_policy', rec['method']
259+
digest = hashlib.sha256(open('${RUNNER_TEMP}/neg/permissive.json','rb').read()).hexdigest()
260+
assert rec['signer_policy']['sha256'] == digest, 'recorded digest != policy file bytes'
261+
assert rec['signer_policy']['name'] == 'ci-trusts-auto'
262+
print('signer_policy sign-off record OK')
263+
"
264+
./nda_review_cli.py negotiate validate --state "${RUNNER_TEMP}/neg/allowed.json" \
265+
| python3 -c "
266+
import json, sys
267+
assert json.loads(sys.stdin.read())['hash_chain_verified'] is True
268+
print('hash chain intact after policy sign-off')
269+
"
270+
271+
# 7. The bare flag resolves <base>/config/signer-policy.json.
272+
cp config/signer-policy.json.example "$BASE_A/config/signer-policy.json"
273+
expect_exit 3 "bare --signer-policy uses config/signer-policy.json" \
274+
./nda_review_cli.py negotiate sign-off --base "$BASE_A" \
275+
--state "${RUNNER_TEMP}/neg/failclosed.json" --as a --yes --signer-policy
276+
184277
- name: Run negotiate analyze + validate
185278
run: |
186279
STATE="${RUNNER_TEMP}/neg/state.json"

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ output/*.md
99
# Local organization-specific policy; commit default-policy.json only
1010
config/org-policy.json
1111

12+
# Local signing authority; commit signer-policy.json.example only
13+
config/signer-policy.json
14+
1215
# Local LLM provider config (API keys, base URLs); commit llm.json.example only
1316
config/llm.json
1417
config/quickstart-answers.json

AGENTS.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ Call `--catalog json` at startup to know what's available. Don't hardcode subcom
5050

5151
- **`review --llm`** — second-pass adjudication. The agent votes on each rule finding, adds findings the rules missed, and proposes replacement clause language for high-severity items. Deterministic findings are never overwritten.
5252
- **`negotiate counter --agent --llm`** — drafts amendments aligned with your policy + stance + clause priorities. The hash-chained state file is signed by you, not the agent.
53-
- **`signer policy run`** — a declarative policy spec that gates which clauses the agent can sign off on. (Future scope; today, sign-off is always human.)
53+
- **`signer policy run`** — a declarative policy spec that gates which clauses the agent can sign off on. Read-only: it emits a verdict (exit `0` allow, `3` escalate) and never mutates state. Pass `--signer-policy` to `negotiate sign-off` to act on that verdict. Deny-by-default — no policy file means no signing authority (exit `4`), and every omitted gate resolves to its most restrictive value. See [docs/reference/signer-policy.md](docs/reference/signer-policy.md).
5454

5555
The state file format is hash-chained: any tampering breaks `negotiate validate` and the next load. A human can audit between agent rounds exactly what was proposed, line by line.
5656

@@ -62,6 +62,8 @@ The state file format is hash-chained: any tampering breaks `negotiate validate`
6262
| `STATE_HASH_MISMATCH` on `negotiate counter` | `nda-review-cli negotiate validate --state <path>` | The state file was tampered with or corrupted in transit. Restore from the last known-good copy; don't continue. |
6363
| `MISSING_PLAYBOOK` | `nda-review-cli doctor` | Run `setup --quick --yes` to auto-discover ingest sources and build a playbook. |
6464
| `LLM_DECLINED` | Provider returned a refusal | Try a different model, or fall back to `--auto` (deterministic, no LLM). Don't silently strip `--strict-fidelity`-equivalent guards. |
65+
| `SIGNER_POLICY_ESCALATED` | `signer policy run --state <path>` for the per-clause reasons | A human must sign this one. Don't retry with `--yes` — the gate fails closed under it by design. Don't widen the policy to make one negotiation pass. |
66+
| `MISSING_SIGNER_POLICY` | No `config/signer-policy.json` and no `--signer-policy` | Expected when nobody has granted signing authority. Fall back to human sign-off; don't author a permissive policy to unblock yourself. |
6567
| `STALEMATE_DETECTED` | `negotiate analyze --state <path>` for the stuck clauses | Surface the `block_diagnosis` to a human. Don't auto-resolve; the stalemate exists for a reason. |
6668
| Counterparty profile unknown | `nda-review-cli profile-learn --counterparty <name> --review-json <path>` | One-shot learning from a saved review. |
6769
| `decision: needs_review` | Read `coverage.rules_matched` (0) and `coverage.extraction_status` | Rules ran but matched nothing. Either the file isn't an NDA, or extraction failed. Hand it to a human — never re-run until something returns `approve`. |
@@ -93,6 +95,15 @@ nda-review-cli negotiate counter --state negotiation.json \
9395

9496
Always pair `--agent` with `--dry-run` first in non-interactive contexts; the dry-run output lets you inspect what the LLM proposed before the round is signed and added to the chain.
9597

98+
Once converged, check whether you're allowed to sign it at all before trying:
99+
100+
```bash
101+
nda-review-cli signer policy run --state negotiation.json # exit 0 = allowed, 3 = escalate
102+
nda-review-cli negotiate sign-off --state negotiation.json --as b --signer-policy
103+
```
104+
105+
Note the default `allowed_amendment_sources` excludes `agent`, so a round *you* drafted with `--agent --llm` escalates unless a human explicitly whitelisted that source. That is the intended posture: an agent should not be the one authorising its own prose.
106+
96107
## LLM safety
97108

98109
- **NDA text leaving the box.** `--llm` is opt-in per call. Without `--llm`, no contract text leaves the machine. With `--llm`, the CLI prints the destination (provider + base URL + model) and asks for consent unless `--yes-llm-send` or `NDA_LLM_NO_CONFIRM=1` is set.
@@ -111,6 +122,8 @@ Always pair `--agent` with `--dry-run` first in non-interactive contexts; the dr
111122
| `tutorial` | Interactive primer; runs a sandboxed sample review. |
112123
| `doctor` | Diagnose first-run issues. Add `--check-llm` to verify LLM provider reachability. |
113124
| `doctor --check-llm` | 1-token round-trip to the configured LLM provider. Confirms reachability + auth + model name. |
125+
| `signer policy validate --file <path>` | Schema-checks a signer policy and prints its **effective gates** after restrictive defaults are applied. |
126+
| `signer policy run --state <path>` | Read-only verdict on whether a converged negotiation may be signed unattended. Exit `0` allow, `3` escalate. |
114127

115128
## Library use (Python)
116129

CHANGELOG.md

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,24 @@
44

55
### Fixed
66

7+
- **Red-flag detection was inert on every negotiation path.** `rule_engine.red_flag_hits(clause, text)`
8+
takes the clause *key* first, but three call sites passed the clause *text* first, so the
9+
`RED_FLAG_PATTERNS` lookup always missed and the function always returned `[]`. Consequences,
10+
all silent:
11+
- `negotiate sign-off` never surfaced an active red flag. A final text reading "the obligations
12+
shall survive indefinitely" was presented to the reviewer as clean, and `--yes` signed it.
13+
- `negotiate counter --auto` never fired its red-flag branch, so the **middleground** and
14+
**compromising** stances never countered a red-flagged clause, and always accepted the
15+
counterparty's red-flagged amendments — the `not proposed_red_flags` guard was permanently true.
16+
Only `conservative` (which counters on any textual difference) behaved as documented.
17+
18+
Fixing the argument order also unmasked a latent `TypeError`: the interactive sign-off display
19+
joined `red_flags_active` as strings, but the entries are `{pattern, match}` dicts. That branch
20+
had never executed. It now renders the matched text.
21+
- **`make smoke` never ran its negotiation flow.** The target aborted at `negotiate counter` with
22+
"Could not auto-detect which party you are" because it never set `org_name` on the two
23+
workspaces — the CI job does, so the drift went unnoticed. The Makefile now mirrors CI.
24+
725
**`review` reported `approve, risk_score 0, findings []` on documents that score `block`.**
826
Two independent defects each zeroed out the review, and the fail-open design reported
927
"nothing was evaluated" as "nothing was wrong."
@@ -39,6 +57,36 @@ and the fixture's `approve` case is a genuinely clean NDA.
3957

4058
### Added
4159

60+
- **`signer policy` — declarative signing authority.** A deterministic, read-only gate answering
61+
whether a converged negotiation may be signed off without a human. Previously documented in
62+
`AGENTS.md` as future scope.
63+
- `signer policy validate --file <path>` — schema-check a spec and print its **effective gates**
64+
after defaults are applied.
65+
- `signer policy run --state <path>` — emit a per-clause verdict. Exit `0` allow, `3` escalate.
66+
Never mutates state.
67+
- `negotiate sign-off --signer-policy [PATH]` — act on the verdict. On `allow`, sign-off proceeds
68+
unattended and records `method: signer_policy` with the policy's name, version, and **SHA-256
69+
digest**. On `escalate` it refuses with exit `3` and leaves the state file untouched —
70+
**including under `--yes`**. The flag can only ever reduce what gets signed.
71+
- Deny-by-default throughout: a missing policy is `MISSING_SIGNER_POLICY` (exit `4`), not
72+
permission; every omitted gate resolves to its most restrictive value; and `auto:` / `agent:`
73+
amendment sources are excluded from the default whitelist, so machine-drafted clause text needs
74+
an explicit human opt-in.
75+
- Gates: `require_status`, `max_rounds`, `red_flags`, `allow_fatigue_concessions`,
76+
`allow_non_negotiable_changes`, `allowed_amendment_sources`, `never_autosign_clauses`.
77+
The hash chain is deliberately **not** a knob — `_negotiate_load_typed` verifies it on every
78+
load, so integrity is unconditional and there is no opt-out to misconfigure.
79+
- `red_flags` is three-valued (`any` / `introduced` / `ignore`) rather than a boolean, because
80+
`RED_FLAG_PATTERNS` are topic triggers, not confirmed defects — `courts of` matches every
81+
governing-law clause ever written. Gating on bare presence would escalate every real NDA.
82+
`introduced` escalates only on patterns the negotiation *added* relative to round 1, which is
83+
the right reading when round 1 is your own template. Default is `any`.
84+
- Verdicts report `unevaluated_clauses`: policy clauses whose text `_negotiate_extract_clause_text`
85+
couldn't locate, and whose gates are therefore vacuous. Without it, `allow` would read as full
86+
coverage when it isn't.
87+
- New: `config/signer-policy.json.example`, `docs/reference/signer-policy.md`, and
88+
`tests/test_signer_policy.py` (46 tests).
89+
4290
- **Fail-closed review contract.** A review can no longer report `approve` for a document it
4391
never inspected.
4492
- Every result carries a **`coverage`** block: `rules_evaluated`, `rules_matched`,
@@ -60,6 +108,7 @@ and the fixture's `approve` case is a genuinely clean NDA.
60108
- `tests/test_review_coverage.py` (24 tests) and `tests/test_review_golden.py::ClauseRulesPlaybookTests`
61109
(3 tests). 22 of the 24 fail against the previous source.
62110

111+
63112
## [0.5.4] - 2026-06-03
64113

65114
Security/robustness fixes from a follow-up source audit.

Makefile

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,24 @@ smoke:
3030
@echo "=== negotiate smoke ==="
3131
./nda_review_cli.py quickstart --base /tmp/nda-make-na --no-prompt --yes >/dev/null
3232
./nda_review_cli.py quickstart --base /tmp/nda-make-nb --no-prompt --yes >/dev/null
33+
@# `negotiate` auto-detects which party you are by matching org_name against
34+
@# the negotiation's parties. quickstart seeds a generic name, so without this
35+
@# every counter/accept below aborts. CI does the same in its negotiate-smoke job.
36+
@python3 -c "import json; \
37+
[ (lambda p, n: json.dump({**json.load(open(p)), 'org_name': n}, open(p, 'w')))( \
38+
f'/tmp/nda-make-n{d}/config/org-policy.json', n) \
39+
for d, n in (('a', 'Acme'), ('b', 'Beta')) ]"
3340
./nda_review_cli.py negotiate init --base /tmp/nda-make-na --template mutual \
3441
--party-a-name "Acme" --party-a-address "1 Main" \
3542
--party-b-name "Beta" --party-b-address "2 Side" \
3643
--purpose "smoke" --effective-date "2026-01-01" \
3744
--out /tmp/nda-make-state.json
3845
./nda_review_cli.py negotiate counter --base /tmp/nda-make-nb --state /tmp/nda-make-state.json --auto
3946
./nda_review_cli.py negotiate accept --base /tmp/nda-make-na --state /tmp/nda-make-state.json --as a
47+
@echo "--- signer policy: the shipped example must escalate an auto-drafted round ---"
48+
@! ./nda_review_cli.py signer policy run --base /tmp/nda-make-na \
49+
--state /tmp/nda-make-state.json \
50+
--signer-policy config/signer-policy.json.example >/dev/null 2>&1
4051
./nda_review_cli.py negotiate sign-off --base /tmp/nda-make-na --state /tmp/nda-make-state.json --as a --yes
4152
./nda_review_cli.py negotiate sign-off --base /tmp/nda-make-nb --state /tmp/nda-make-state.json --as b --yes
4253
./nda_review_cli.py negotiate finalize --base /tmp/nda-make-na --state /tmp/nda-make-state.json \

0 commit comments

Comments
 (0)