From 30aeb33edc722fa32c72ded5db3cf8728618d3a6 Mon Sep 17 00:00:00 2001 From: Michael Lieberman Date: Wed, 21 Oct 2026 00:23:03 -0400 Subject: [PATCH 01/10] docs(spec): data and judgment tools return the procedure to follow get_pending_data and get_judgment_requests carry a 'procedure': the ordered steps for the result, with calls that name only the repository the caller named. A confirm_project_data result on a server with get_pending_data ends with the next get_pending_data call. The text has one home in the framework, so skills and clients defer to it. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Michael Lieberman --- docs/architecture/framework-design.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/architecture/framework-design.md b/docs/architecture/framework-design.md index 80f938854..ccd7e8e22 100644 --- a/docs/architecture/framework-design.md +++ b/docs/architecture/framework-design.md @@ -1407,6 +1407,8 @@ get_judgment_requests( A coding agent obtains what it judges through `get_judgment_requests`, registered beside `submit_judgment` and requiring the same named repository identity. It audits `local_path` the same way and returns one request per control left `PENDING` (`llm_judgment`), optionally limited to `control_ids`: the control's `prompt`, `analysis_hints`, `target_key` when set, `evidence_digest`, and the keys of its files in a shared `files` map. Each `files` entry holds `path`, `digest`, and `content`, where `content` is exactly the content `submit_judgment` checks the control's citations against (including its caps); a file read by several controls appears once, under a distinct key only if its content differs. `gathered_evidence` and the rest of the control's evidence are not returned. A control with a stored candidate for its current digest (`PENDING`, `confirmation`) is left out unless `include_judged` is true. Requests are ordered by control id and paged: a page holds whole requests and its serialized result stays under 40,000 bytes (UTF-8), below the size at which a coding agent's client writes a tool result to a file instead of showing it, and `remaining` lists the controls not yet returned, to be passed as `control_ids` on the next call. A single request larger than the page limit is returned alone, with `oversized = true`. The tool records nothing, and `submit_judgment` never trusts its output. +Each `get_judgment_requests` page also carries `procedure`: the ordered steps for judging the page (judge each request from its prompt, hints, and listed files only; call `submit_judgment` once per request with passages copied verbatim; a pass is a PASS candidate and a fail a suggestive finding, never the audit result; fetch the next page with `remaining`). The steps name the repository and checkout the caller passed and hold placeholders for everything else. The procedure text lives in one place in the framework (`darnit.server.procedures`), so a skill or client defers to it instead of restating it. + The compact `summary` output of an audit carries, on each `PENDING` row, `pending.kind` and, for a candidate, `pending.candidate` (its model, reasoning, and number of cited excerpts, never the excerpts), so a client can tell a control awaiting a judgment from one awaiting confirmation without the full evidence. ``` @@ -1757,7 +1759,7 @@ Each context key has one canonical name and one vocabulary, taken from the frame The observable payloads of `get_pending_data` and `confirm_project_data` are defined in `specs/042-candidate-integrity/contracts/context-confirmation-tools.md`: -- `get_pending_data` is read-only. Each question carries its candidate as data (`candidate: {value, origin, digest, label}`, where `digest` is the value digest (7.5) and `label` marks it unconfirmed); the candidate's value appears nowhere else in the response. `command_template` holds placeholders only (``, ``, `owner=..., repo=...`), never a candidate value or a configuration example. `answer_mapping.value_map["Yes"]` for a candidate is `{"accept_candidates": {"": ""}}`, a placeholder the agent fills from the question's `candidate.digest` only after the person answered yes. Enum questions list every allowed value in `allowed_values`; when there are more than 4, the selector (`ask_user`) is omitted and the prompt lists them all. `examples` appear only as `format_hint`. Every question in the batch is returned under `questions`, with or without a selector. The response lists `stored_unconfirmed` values with their locations. +- `get_pending_data` is read-only. Each question carries its candidate as data (`candidate: {value, origin, digest, label}`, where `digest` is the value digest (7.5) and `label` marks it unconfirmed); the candidate's value appears nowhere else in the response. `command_template` holds placeholders only (``, ``, `owner=..., repo=...`), never a candidate value or a configuration example. `answer_mapping.value_map["Yes"]` for a candidate is `{"accept_candidates": {"": ""}}`, a placeholder the agent fills from the question's `candidate.digest` only after the person answered yes. Enum questions list every allowed value in `allowed_values`; when there are more than 4, the selector (`ask_user`) is omitted and the prompt lists them all. `examples` appear only as `format_hint`. Every question in the batch is returned under `questions`, with or without a selector. The response lists `stored_unconfirmed` values with their locations. The response also carries `procedure`: the ordered steps for collecting the data (show each candidate with its origin, labelled unconfirmed; ask every question, passing `ask_user_batch` verbatim; per-question skip; one `confirm_project_data` call per batch, with `accept_candidates` filled only after a yes; call `get_pending_data` again until `complete`; review `stored_unconfirmed`; summarize). Its calls name only the `owner` and `repo` the caller passed, never ones detected from the checkout's remote (placeholders otherwise), and hold no candidate or detected value. A `confirm_project_data` result that handled context values on a server with `get_pending_data` ends with the next `get_pending_data` call, named the same way. - `confirm_project_data` takes a parameter for every context key that the server's bound framework defines, and no other framework's keys (generated from the definitions; an enum parameter accepts only its `values`), plus `accept_candidates` (`{key: candidate digest}`: detection runs again and the current candidate, or a value concluded in this run, is confirmed only if its digest still matches, recording its value and origin as the basis; a mismatch, a key with no candidate, or a key given both an answer and a digest is refused and nothing is written for it), `confirm_stored` and `reject_stored` (review of stored values: a rejected `.project/darnit.yaml` value is deleted; a rejected `.project/project.yaml` value is reported with its file and field and the file is left unchanged), `expires_at` (`{key: date}`), and `owner`, `repo`, `host`, which are required and decide the record location (7.5). `confirm_stored` records a confirmation of the value currently stored for each key (basis: that value, origin `stored_unconfirmed` or `expired_confirmation`). Like a confirmation, a rejection writes to `.project/darnit.yaml` only when the operator trusts the named repository; otherwise it is refused and names the file and field for the person to edit. The result names, per key, `confirmed (in-repository)` with the file written or `confirmed (operator-side)`, `rejected`, `edit required: :`, or `refused: `. - **Which server exposes it**: each server's `confirm_project_data` covers only the keys of the framework the server is bound to. The OpenSSF Baseline server defines its own (which also carries `confirm_not_applicable` and `confirm_pass_candidate`). Every other framework server that defines context keys and whose TOML does not define a tool named `confirm_project_data` gets a framework-neutral builtin `confirm_project_data` (a framework with no context keys gets none), registered by the factory the same way as `submit_judgment` and `confirm_pass_candidate` (5.4) and bound to that framework's context definitions. It records context values only: the per-key parameters, `accept_candidates`, `confirm_stored`, `reject_stored`, `expires_at`, `owner`, `repo`, `host`, and `local_path`. Claim confirmations and PASS candidates stay on their existing tools. - Remediation prompts (the preflight and `format_context_prompt`) show a candidate as labelled, unconfirmed data with its origin and digest, name the key's hint source files that exist without parsing values out of them, and hold placeholders only in their commands. From 2102d0098e246e559b9c8be3d6f92c20fa5d34d5 Mon Sep 17 00:00:00 2001 From: Michael Lieberman Date: Wed, 21 Oct 2026 00:23:03 -0400 Subject: [PATCH 02/10] feat(mcp): return the data and judgment procedures from the tools darnit-comply told the agent to "follow the /darnit-data skill" and "follow the /darnit-audit skill", but nothing loads another skill's text, so the comply run improvised data collection from one line and asked questions differently from a darnit-data run. darnit.server.procedures holds the steps once. get_pending_data returns them as 'procedure' (show candidates labelled unconfirmed, ask every question, per-question skip, one confirm_project_data call with accept_candidates filled only after a yes, the next call, the review of stored values, the summary), get_judgment_requests returns the judging steps on every page, and the baseline confirm_project_data result ends with the next get_pending_data call. Calls name only the owner and repo the caller passed, never ones read from the checkout's remote, and hold no candidate value. The skills defer to the procedures instead of restating them, and no skill tells the agent to follow another one (a test checks). The comply and remediate skills pass owner and repo to remediate_audit_findings, which needs them to find operator-side confirmations (#612). Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Michael Lieberman --- CHANGELOG.md | 11 +++ .../src/darnit_baseline/openssf-baseline.toml | 17 +--- .../src/darnit_baseline/tools.py | 40 +++----- .../darnit/src/darnit/server/procedures.py | 79 +++++++++++++++ .../src/darnit/server/tools/judgments.py | 12 ++- .../src/darnit/server/tools/project_data.py | 14 ++- .../src/darnit/skills/darnit-audit/SKILL.md | 4 +- .../src/darnit/skills/darnit-comply/SKILL.md | 28 +++--- .../src/darnit/skills/darnit-data/SKILL.md | 45 +++------ .../darnit/skills/darnit-remediate/SKILL.md | 9 +- tests/darnit/server/test_judgment_requests.py | 14 +++ tests/darnit/server/test_procedures.py | 57 +++++++++++ .../test_get_pending_context.py | 99 +++++++++++++++++++ 13 files changed, 332 insertions(+), 97 deletions(-) create mode 100644 packages/darnit/src/darnit/server/procedures.py create mode 100644 tests/darnit/server/test_procedures.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 80a1be814..fc9223d15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -442,6 +442,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 hard-coded `0.1.0`, and `baseline.version` / `baseline.specification` derive from the framework TOML metadata (`OSPS v2026.02.19`) instead of `2025.10.10` (Refs #494). +- `get_pending_data` and `get_judgment_requests` return a `procedure`: the + steps a coding agent follows with the result (how to show candidates and + ask the questions, the exact `confirm_project_data` or `submit_judgment` + call, and the next call), naming only the repository the caller named. + A `confirm_project_data` result on the OpenSSF Baseline server ends with + the next `get_pending_data` call. The `darnit-data`, `darnit-audit`, + `darnit-comply`, and `darnit-remediate` skills defer to these procedures + instead of restating them or telling the agent to follow another skill, + which a client may not load, so `darnit-comply` collected data + differently from `darnit-data`. The comply and remediate skills now pass + `owner` and `repo` to `remediate_audit_findings`. - **BREAKING:** `register_handlers()` is the one handler registration hook for plugins (#451). `darnit-gittuf` and `darnit-reproducibility` rename `register_sieve_handlers()` to `register_handlers()`, and their `register()` diff --git a/packages/darnit-baseline/src/darnit_baseline/openssf-baseline.toml b/packages/darnit-baseline/src/darnit_baseline/openssf-baseline.toml index 06731486d..9d30a7add 100644 --- a/packages/darnit-baseline/src/darnit_baseline/openssf-baseline.toml +++ b/packages/darnit-baseline/src/darnit_baseline/openssf-baseline.toml @@ -4640,18 +4640,11 @@ Each question carries its candidate, if any, as data: `candidate: {value, origin labelled UNCONFIRMED. Command templates and answer mappings hold placeholders only; `allowed_values` lists every allowed value of an enum; `format_hint` is a format, never an answer. -MANDATORY WORKFLOW - your next action after calling this tool MUST be calling AskUserQuestion: -1. Call this tool. It returns "questions" and "ask_user_batch" (AskUserQuestion-compatible objects). -2. Show the person each question's candidate value and origin, labelled unconfirmed. Then call - AskUserQuestion with questions=, VERBATIM. Ask the questions that have no - ask_user entry after that, listing every allowed value. -3. After the person answers, use "answer_mapping" to call confirm_project_data(). A "Yes" on a - candidate becomes accept_candidates with that question's candidate.digest, filled in only now. -4. Call this tool again for the next batch. Repeat until status is "complete". - -`stored_unconfirmed` lists values stored in .project/ without a confirmation (key, value, -location). Show them to the person; confirm or reject the ones the person names in one -confirm_project_data call with `confirm_stored` / `reject_stored`. +The result's `procedure` lists the steps to follow, in order: show candidates, ask the +questions (`ask_user_batch` is AskUserQuestion-compatible; pass it verbatim), record the answers +with confirm_project_data, call this tool again until status is "complete", review +`stored_unconfirmed` values, and summarize. Your next action after calling this tool MUST be +asking the person the questions. Do NOT pre-fill or suggest answers. Never type a detected value as an answer. """ diff --git a/packages/darnit-baseline/src/darnit_baseline/tools.py b/packages/darnit-baseline/src/darnit_baseline/tools.py index 9ce3e81e9..a21135d8d 100644 --- a/packages/darnit-baseline/src/darnit_baseline/tools.py +++ b/packages/darnit-baseline/src/darnit_baseline/tools.py @@ -711,6 +711,7 @@ def confirm_project_data( Returns: What was recorded or refused, per key """ + from darnit.server.procedures import data_next_step from darnit.server.tools.project_data import confirm_project_data_impl return confirm_project_data_impl( @@ -725,6 +726,7 @@ def confirm_project_data( host=host, framework_name=_FRAMEWORK_NAME, confirm_pass_candidate=confirm_pass_candidate, + next_step=data_next_step(owner, repo, str(Path(local_path).resolve())), **values, ) @@ -751,25 +753,8 @@ def _context_key_order() -> list[str]: return [] -_LLM_DIRECTIVE_PREFIX = """MANDATORY: Your next action MUST be calling the AskUserQuestion tool. - -For every question in "questions" that has a "candidate", first show the person its -candidate.value and candidate.origin, labelled UNCONFIRMED. Then pass "ask_user_batch" -below verbatim as the "questions" parameter to AskUserQuestion. Do NOT paraphrase, do NOT -pre-select answers, and do NOT add options. A question in "questions" with no entry in -"ask_user_batch" (free text, or more allowed values than the selector holds) is asked -after that, listing every value in its "allowed_values"; never offer its "format_hint" -as an answer. - -After the person answers, call confirm_project_data() once for the batch, with owner and -repo (and host when not github.com) naming the repository. Use "answer_mapping": -- "Yes" / "No" on a yes/no question -> pass true / false (not strings) -- A selected option label -> pass the label as a string -- "Yes" on a candidate question -> accept_candidates, mapping the key to that question's - candidate.digest; fill in the digest only now, after the person answered Yes -- "No" on a candidate question, or "Other" -> pass the value the person typed -Never type a detected value as an answer. Then call get_pending_data() again for the -next batch (if any remain). +_LLM_DIRECTIVE_PREFIX = """MANDATORY: Your next action MUST be asking the person the questions below. +Follow "procedure" in this result, step by step. --- """ @@ -791,12 +776,9 @@ def get_pending_data( its candidate, if any, as unconfirmed data (value, origin, digest); command templates and answer mappings hold placeholders only. - Workflow: - 1. Call this tool. It returns "questions" and "ask_user_batch". - 2. Show each question's candidate to the person, labelled unconfirmed, then - call AskUserQuestion(questions=). - 3. After the person answers, use "answer_mapping" to call confirm_project_data(). - 4. Call get_pending_data() again for the next batch. Repeat until status is "complete". + The result's ``procedure`` lists the steps to follow: how to show + candidates and ask the questions, the confirm_project_data call, the next + get_pending_data call, the review of stored values, and the summary. Parameters: - `local_path`: Path to repository (default: ".") @@ -807,17 +789,21 @@ def get_pending_data( - `limit`: Max questions to return per batch (default: 4). Use 0 for all. Returns: - JSON with the questions, ask_user_batch (for AskUserQuestion), + JSON with the procedure, the questions, ask_user_batch (for AskUserQuestion), answer_mapping for confirm_project_data, a progress indicator, and stored_unconfirmed: values stored in .project/ without a confirmation, with their locations, for review with confirm_stored / reject_stored. """ from darnit.config.context_storage import get_pending_context as _get_pending + from darnit.server.procedures import data_collection_procedure from darnit.server.tools.project_data import stored_unconfirmed from darnit.trust.decision import target_from_owner_repo repo_path = Path(local_path).resolve() target = target_from_owner_repo(owner, repo) + # Only the repository the caller named goes into the procedure's calls, + # never one detected from the checkout's remote. + procedure = data_collection_procedure(owner, repo, str(repo_path)) # Auto-detect owner/repo from git if owner is None or repo is None: @@ -842,6 +828,7 @@ def get_pending_data( complete: dict = { "status": "complete", "message": "All context has been confirmed. No additional input needed.", + "procedure": procedure, "questions": [], } if stored: @@ -891,6 +878,7 @@ def get_pending_data( response: dict = { "status": "pending", + "procedure": procedure, "progress": { "answered": answered, "total": total, diff --git a/packages/darnit/src/darnit/server/procedures.py b/packages/darnit/src/darnit/server/procedures.py new file mode 100644 index 000000000..5600c9a88 --- /dev/null +++ b/packages/darnit/src/darnit/server/procedures.py @@ -0,0 +1,79 @@ +"""The steps a coding agent follows after a data or judgment tool returns. + +One source for the procedure text that ``get_pending_data``, +``confirm_project_data``, and ``get_judgment_requests`` return, so every +skill and client follows the same steps (framework-design 5.4, 7.10). +Calls name only what the caller passed (the repository and checkout); +an unnamed repository is a placeholder, never a value read from the +checkout, and no step holds a candidate or detected value. +""" + +from __future__ import annotations + +import json + +_OWNER = "" +_REPO = "" + + +def _args(owner: str | None, repo: str | None, host: str | None, local_path: str) -> str: + parts = [ + f"owner={json.dumps(owner) if owner else _OWNER}", + f"repo={json.dumps(repo) if repo else _REPO}", + ] + if host and host != "github.com": + parts.append(f"host={json.dumps(host)}") + parts.append(f"local_path={json.dumps(local_path)}") + return ", ".join(parts) + + +def data_next_step(owner: str | None, repo: str | None, local_path: str) -> str: + """The call that follows a ``confirm_project_data`` call (``get_pending_data`` takes no host).""" + return ( + f"Next: call get_pending_data({_args(owner, repo, None, local_path)}) for the remaining questions " + "and follow its procedure; when it returns status complete, re-run the audit." + ) + + +def data_collection_procedure(owner: str | None, repo: str | None, local_path: str) -> list[str]: + """Ordered steps for collecting project data from the person (``get_pending_data`` takes no host).""" + args = _args(owner, repo, None, local_path) + return [ + "1. For each question with a candidate, show the person candidate.value and candidate.origin, " + "labelled UNCONFIRMED. A candidate is never a fact or a default answer.", + "2. Ask every question. Pass ask_user_batch verbatim to your question tool (AskUserQuestion in " + "Claude Code); ask a question with no ask_user entry in plain text, listing every allowed_values " + "entry. format_hint is a format, never an answer. Do not pre-fill or suggest answers.", + "3. If the person says skip or does not know, leave that key out and move on.", + f"4. Record the answers in one call: confirm_project_data({args}, =). " + "Yes/No questions take true/false. For a Yes on a candidate pass " + 'accept_candidates={"": ""}, filled in only after the ' + "person said yes. Pass only what the person said, never a detected value.", + f"5. Call get_pending_data({args}) again and repeat from step 1 until status is complete.", + "6. If stored_unconfirmed is not empty, show every entry (key, value, location) labelled " + "unconfirmed, ask which to keep and which to reject, and make one confirm_project_data call with " + "confirm_stored and reject_stored holding only the keys the person named. For an edit required " + "result, tell the person which file and field to edit.", + "7. Tell the person what was confirmed, rejected, left to edit, and skipped, and which controls " + "the confirmed keys affect (affects_controls).", + "Confirm a not-applicable claim or a PASS candidate only when the operator explicitly names it.", + ] + + +def judgment_procedure(owner: str, repo: str, host: str, local_path: str) -> list[str]: + """Ordered steps for judging the controls on a ``get_judgment_requests`` page.""" + args = _args(owner, repo, host, local_path) + return [ + "1. For each request, judge the control from its prompt and analysis_hints, using only the " + "files it lists (keys into files).", + f'2. Call submit_judgment(control_id=, verdict="pass" or "fail", ' + "reasoning=, cited_evidence=[], model=, model_version=, {args}) once per request. " + "A paraphrased citation is rejected.", + "3. A pass verdict becomes a PASS candidate that stays non-compliant until the operator " + "confirms it; a fail verdict is a suggestive finding. Never report your verdict as the audit " + "result.", + f"4. If remaining is not empty, call get_judgment_requests({args}, control_ids=) " + "and repeat; stop when remaining is empty.", + "Confirm a PASS candidate only when the operator explicitly names it.", + ] diff --git a/packages/darnit/src/darnit/server/tools/judgments.py b/packages/darnit/src/darnit/server/tools/judgments.py index 74c920076..2bafc05af 100644 --- a/packages/darnit/src/darnit/server/tools/judgments.py +++ b/packages/darnit/src/darnit/server/tools/judgments.py @@ -150,6 +150,7 @@ def get_judgment_requests_impl( from darnit.config.operator.loader import OperatorConfigError, resolve_operator_config from darnit.core.utils import validate_local_path + from darnit.server.procedures import judgment_procedure from darnit.tools.audit import judgment_audit from darnit.trust.decision import decide_trust, target_from_owner_repo from darnit.trust.judgments import judged_content @@ -202,7 +203,13 @@ def dump(payload: dict[str, Any]) -> str: key=lambda r: r["id"], ) - page: dict[str, Any] = {"status": "ok", "files": {}, "requests": [], "remaining": []} + page: dict[str, Any] = { + "status": "ok", + "procedure": judgment_procedure(owner, repo, host, str(resolved_path)), + "files": {}, + "requests": [], + "remaining": [], + } oversized = False for index, result in enumerate(pending): files = dict(page["files"]) @@ -250,7 +257,8 @@ async def get_judgment_requests( ) -> str: """Get the judgment requests for controls awaiting a judgment (PENDING, pending.kind = "llm_judgment"). - Returns JSON: ``requests`` (one per control: ``control_id``, ``prompt``, + Returns JSON: ``procedure`` (the steps to follow with this page), + ``requests`` (one per control: ``control_id``, ``prompt``, ``analysis_hints``, ``evidence_digest``, and ``files``, keys into the shared ``files`` map), ``files`` (``path``, ``digest``, and ``content``, exactly the content ``submit_judgment`` checks citations against), and diff --git a/packages/darnit/src/darnit/server/tools/project_data.py b/packages/darnit/src/darnit/server/tools/project_data.py index 529e8a090..9950a12f4 100644 --- a/packages/darnit/src/darnit/server/tools/project_data.py +++ b/packages/darnit/src/darnit/server/tools/project_data.py @@ -164,6 +164,7 @@ def confirm_project_data_impl( host: str | None = None, framework_name: str | None = None, confirm_pass_candidate: list[str] | None = None, + next_step: str | None = None, **values: Any, ) -> str: """Record a person's confirmation of project data values. @@ -196,6 +197,8 @@ def confirm_project_data_impl( confirm_pass_candidate: Control IDs whose PASS candidate (a positive model judgment) the operator confirms for the current evidence. Recorded operator-side, never in the repository. + next_step: The call that follows, added to a result that handled + context values (the framework's ``get_pending_data``). **values: The person's answer per context key. Returns: @@ -250,7 +253,7 @@ def confirm_project_data_impl( ) for key in dict.fromkeys([*requested, *expiries]) ] - return _report(claims_result, results) + return _report(claims_result, results, next_step) try: operator_config = resolve_operator_config(resolved_path) @@ -292,14 +295,17 @@ def wanted(kind: str) -> dict[str, Any]: ) for key in wanted("reject_stored"): results += reject_stored_value(resolved_path, key, **common) - return _report(claims_result, results) + return _report(claims_result, results, next_step) -def _report(claims_result: str, results: list[Any]) -> str: +def _report(claims_result: str, results: list[Any], next_step: str | None = None) -> str: lines = ["Project data:"] for result in results: lines.extend(_result_lines(result)) - if any(result.outcome in ("confirmed", "deleted") for result in results): + if next_step: + lines.append("") + lines.append(next_step) + elif any(result.outcome in ("confirmed", "deleted") for result in results): lines.append("") lines.append("Re-run the audit to see the updated status.") claims_section = f"{claims_result}\n\n" if claims_result else "" diff --git a/packages/darnit/src/darnit/skills/darnit-audit/SKILL.md b/packages/darnit/src/darnit/skills/darnit-audit/SKILL.md index 01fe33ecd..3001dac11 100644 --- a/packages/darnit/src/darnit/skills/darnit-audit/SKILL.md +++ b/packages/darnit/src/darnit/skills/darnit-audit/SKILL.md @@ -4,7 +4,7 @@ description: Run a compliance audit on the current repository using darnit. Use compatibility: Requires darnit MCP server running (darnit serve) metadata: author: kusari-oss - version: "2.0" + version: "2.1" --- # Compliance Audit @@ -29,7 +29,7 @@ Common audit tools: - If the user mentions a profile (e.g., "just level 1", "access control only", "onboard"), pass it as the `profile` parameter. - If the user mentions a level, pass it as the `level` parameter. -2. Inspect the results for `PENDING` controls with `pending.kind = "llm_judgment"`. These are controls whose deterministic steps could not decide and that need a judgment of their content. Call `get_judgment_requests` with `owner`, `repo`, and `local_path` naming the audited repository. It returns `requests` (one per control: `control_id`, `prompt`, `analysis_hints`, and `files`, keys into the shared `files` map) and `remaining`; while `remaining` is not empty, call it again with `control_ids` set to `remaining`. For each request, read the prompt and the `content` of each of its files, then call `submit_judgment` with `control_id`, `verdict` (`"pass"` or `"fail"`), your `reasoning`, `cited_evidence` (the passages you relied on, copied verbatim from those files' `content`; a `"pass"` needs at least one), `model` and `model_version` (the model you are), and `owner`, `repo`, `local_path` naming the audited repository. Your judgment is not the audit result: +2. If any control is `PENDING` with `pending.kind = "llm_judgment"`, call `get_judgment_requests` with `owner`, `repo`, and `local_path` naming the audited repository, and follow the `procedure` in its result: it says how to judge each request, how to call `submit_judgment`, and how to fetch the next page. Your judgment is not the audit result: - A `"pass"` with verified citations becomes a PASS candidate (`PENDING`, `pending.kind = "confirmation"`), still non-compliant until the operator confirms it. - A `"fail"` is reported as a model finding (suggestive FAIL). - A rejection means a cited passage was not found verbatim; the control stays unverified. Do not rephrase the passage to get it accepted unless it is genuinely in the file. diff --git a/packages/darnit/src/darnit/skills/darnit-comply/SKILL.md b/packages/darnit/src/darnit/skills/darnit-comply/SKILL.md index 0356901eb..928758437 100644 --- a/packages/darnit/src/darnit/skills/darnit-comply/SKILL.md +++ b/packages/darnit/src/darnit/skills/darnit-comply/SKILL.md @@ -4,7 +4,7 @@ description: Run the full compliance pipeline — audit, collect data, remediate compatibility: Requires darnit MCP server running (darnit serve) and gh CLI for PR creation metadata: author: kusari-oss - version: "2.1" + version: "2.2" --- # Full Compliance Pipeline @@ -23,28 +23,30 @@ Darnit registers tools per implementation module. Look for available tools match ### 1. Initial audit -Call the appropriate `audit_*` tool with `output_format: "summary"` and any profile the user mentioned. The "summary" format returns compact JSON; PENDING rows carry `pending.kind`. Do not use `output_format: "json"`: it is too large for a tool result. Present a brief summary: total controls, pass/fail/warn counts, compliance percentage. If any row has `pending.kind = "llm_judgment"`, follow the `/darnit-audit` skill: call `get_judgment_requests` with `owner`, `repo`, and `local_path` (and again with `control_ids` set to `remaining` until it is empty), then for each request call `submit_judgment` with your verdict, reasoning, and passages copied verbatim from the `content` of the request's files. Never state your own verdict as the audit result: a `"pass"` becomes a PASS candidate that stays non-compliant until the operator confirms it. +Call the appropriate `audit_*` tool with `output_format: "summary"` and any profile the user mentioned. The "summary" format returns compact JSON; PENDING rows carry `pending.kind`. Do not use `output_format: "json"`: it is too large for a tool result. Present a brief summary: total controls, pass/fail/warn counts, compliance percentage. -### 2. Collect data (if needed) +### 2. Judgments -If WARN controls exist due to missing data: -- Tell the user you'll collect data to improve accuracy -- Follow the `/darnit-data` skill workflow: call `get_pending_data`, present questions, call `confirm_project_data` for answers -- Continue until `status: "complete"` or the user says "skip remaining" +If any row has `pending.kind = "llm_judgment"`, call `get_judgment_requests` with `owner`, `repo`, and `local_path` naming the audited repository, and follow the `procedure` in its result until `remaining` is empty. Never state your own verdict as the audit result: a `"pass"` becomes a PASS candidate that stays non-compliant until the operator confirms it. -### 3. Remediation plan +### 3. Collect data + +Tell the user you'll collect project data, then call `get_pending_data` with `owner`, `repo`, and `local_path` naming the audited repository, and follow the `procedure` in its result until `status` is `"complete"` or the user says to skip the rest. + +### 4. Remediation plan If there are FAIL controls with auto-fixes: -- Call `remediate_audit_findings` with `dry_run: true` +- Call `remediate_audit_findings` with `dry_run: true`, and `owner` and `repo` naming the audited repository - Read the fenced JSON block after the report: its `plan` lists each item's `file_changes`, `change_sets`, `requires_individual_approval`, and `digest` - Present every file change; every platform change set with each field as `before -> after`, its `impact_notes`, and its digest; and every item that needs individual approval. Show a high-impact change (repository visibility, organization settings) on its own with its impact - Ask the person which changes to apply, asking separately about each platform change set and each item that needs individual approval -If no failures or no auto-fixes: report the status and list manual steps. Skip to step 6. +If no failures or no auto-fixes: report the status and list manual steps. Skip to step 7. -### 4. Apply fixes (if confirmed) +### 5. Apply fixes (if confirmed) Call `remediate_audit_findings` with: +- `owner` and `repo` naming the audited repository - `dry_run: false` - `approve: [...]`: only the digests the person approved, copied from the preview - `branch_name: "fix/compliance"` (or `"fix/compliance-{profile}"`) @@ -55,7 +57,7 @@ Call `remediate_audit_findings` with: This single call creates the branch, applies all remediations, and commits. Do NOT make separate calls to `create_remediation_branch` or `commit_remediation_changes`. -### 5. Enhance generated files (value-add) +### 6. Enhance generated files (value-add) Read the generated template files and improve them with project-specific content: - Fill in real project details (maintainer names, security contact, CI/CD specifics) @@ -64,7 +66,7 @@ Read the generated template files and improve them with project-specific content If files were enhanced, make a new commit with the improvements. -### 6. Create PR and final report +### 7. Create PR and final report Ask if the user wants a PR. If yes, call `create_remediation_pr` and display the URL. diff --git a/packages/darnit/src/darnit/skills/darnit-data/SKILL.md b/packages/darnit/src/darnit/skills/darnit-data/SKILL.md index fca07e2f4..9158efba4 100644 --- a/packages/darnit/src/darnit/skills/darnit-data/SKILL.md +++ b/packages/darnit/src/darnit/skills/darnit-data/SKILL.md @@ -4,7 +4,7 @@ description: Collect missing project data to improve audit accuracy. Use when th compatibility: Requires darnit MCP server running (darnit serve) metadata: author: kusari-oss - version: "1.3" + version: "2.0" --- # Project Data Collection @@ -13,48 +13,25 @@ Guide the user through providing missing project data that darnit needs for accu ## Workflow -1. Call `get_pending_data` MCP tool with `local_path` set to the repository root and `level` set to `3`. - - If the user mentioned a profile, pass it as the `profile` parameter to scope questions to that profile's controls. - - This call writes nothing. +1. Call `get_pending_data` with `local_path` set to the repository root and `owner` and `repo` naming the repository. If the user mentioned a profile, pass it as `profile`. This call writes nothing. -2. The tool returns a batch of `questions` (and `ask_user_batch` for the ones a selector can hold). For each question: - - If it has a `candidate`: show the user `candidate.value` and `candidate.origin`, labelled unconfirmed, and ask whether to accept it. Never present it as a fact or as the default. - - Otherwise: ask the question. For an enum, offer every value in `allowed_values`. `format_hint` shows the expected format; never offer it as an answer. - - If the user says "skip" or "I don't know": move on. Don't block. +2. Follow the `procedure` in its result, step by step. It says how to show candidates, how to ask the questions, how to call `confirm_project_data`, when to call `get_pending_data` again, how to review values stored without a confirmation, and what to summarize at the end. Each `confirm_project_data` result names the next call. -3. After the user has answered, call `confirm_project_data` MCP tool, with `owner` and `repo` (and `host` when not github.com) naming the repository: - - The user accepted a candidate: pass `accept_candidates={"": }`. Fill in the digest only now, after the user answered yes; never before, and never on your own. The tool re-runs detection and refuses if the candidate changed; then show the new candidate and ask again. - - The user gave a value: pass it as the key's parameter (`=`). Never type a detected value as an answer, and never pass anything the user did not say. - -4. Call `get_pending_data` again. If `status` is `"complete"`, move to step 5. Otherwise repeat from step 2. - -5. Review stored values. If the response lists `stored_unconfirmed` (values in `.project/` with no confirmation, each with its `key`, `value`, and `location`), show the whole list to the user, labelled unconfirmed, and ask which to keep and which to reject. Then make one `confirm_project_data` call with `confirm_stored=[]` and `reject_stored=[]` (plus `owner`/`repo`). Only keys the user named; leave the rest as they are. - - A rejected value in `.project/darnit.yaml` is deleted (when the operator trusts the repository). - - A rejected value in `.project/project.yaml` comes back as `edit required: :`; the file is not changed. Tell the user which field to edit; until they do, the value stays an unconfirmed candidate. - -6. Summarize what was collected: - - Values confirmed (accepted candidates, values the user gave, and stored values the user kept) - - Values rejected, and any `edit required` fields the user still has to change - - Skipped questions - - Which controls are now unblocked - - Suggest running `/darnit-audit` to see improved results +3. When `status` is `"complete"` and the procedure's review and summary are done, suggest re-running the audit to see the updated results. ## Gotchas - Never fill in values on the user's behalf. A candidate is not a value until the user confirms it, however confident its origin. -- Command templates and `answer_mapping` hold placeholders only (``, ``). Replace a placeholder only with what the user actually answered. +- Do not edit `.project/` files by hand to record answers; `confirm_project_data` is the only way to record them. `.project/project.yaml` is never written by this workflow. +- Confirmed values are recorded in `.project/darnit.yaml` when the operator trusts the repository; otherwise they are recorded operator-side and nothing is written to the repository. - Answers submitted to an ActionPlan `collect_context` step (`submit_action_result`) are used for that run only and are never saved. To record a value, use `confirm_project_data`. -- Confirmed values are recorded in `.project/darnit.yaml` (the value under `context:` and who confirmed it, when, and on what basis under `confirmations:`) when the operator trusts the repository; otherwise they are recorded operator-side and nothing is written to the repository. `.project/project.yaml` is never written by this workflow. -- A value in `.project/` without a confirmation record is a candidate too (origin `stored_unconfirmed`, with its file and field). Review it with the user (step 5), or correct it by passing the user's answer; a hand edit to a confirmed value makes it a candidate again. -- `expires_at={"": ""}` records an expiry with a confirmation, only when the user asks for one. With no expiry a confirmation stays valid until the value changes or the framework's validity period for the key passes. -- Each framework's server has its own `confirm_project_data`, covering only that framework's keys (for example `csl_*` keys on the Community Specification server). Use the tool of the server whose framework asked the question. +- `expires_at={"": ""}` records an expiry with a confirmation, only when the user asks for one. +- Each framework's server has its own `confirm_project_data`, covering only that framework's keys. Use the tool of the server whose framework asked the question. - Register darnit's MCP server at user scope (`darnit install` does this by default), not in a configuration file committed to the repository; a repository-scoped `uv run darnit serve` runs the repository's own copy of darnit. -- Project data that makes a control not applicable (for example "no releases") is a not-applicable claim. It counts only when the operator trusts the repository and no evidence contradicts it; otherwise the audit reports it as pending and the control counts as non-compliant. -- Pending claims can be confirmed with `confirm_project_data` (`confirm_not_applicable` plus `owner`/`repo`); the confirmation is stored on the operator's machine, not in the repository. Only do this when the operator explicitly tells you to confirm that specific claim. Never confirm a claim on your own, and never treat a user's answer to a data question as a confirmation of a claim. -- Questions come in the framework's definition order. Present them in order. -- Do not edit `.project/` files by hand to record answers; `confirm_project_data` is the only way to record them. If `get_pending_data` is not available, the implementation module may not support data collection; say so and stop. +- Project data that makes a control not applicable (for example "no releases") is a not-applicable claim. It counts only when the operator trusts the repository and no evidence contradicts it. Confirm a pending claim (`confirm_not_applicable`) only when the operator explicitly names that claim; a user's answer to a data question is not a confirmation of a claim. ## Error handling -If all data is already collected, report that and suggest running `/darnit-audit`. +If `get_pending_data` is not available, the implementation module may not support data collection; say so and stop. +If all data is already collected, report that and suggest re-running the audit. If `confirm_project_data` refuses a key, report the reason to the user; nothing was written for that key. diff --git a/packages/darnit/src/darnit/skills/darnit-remediate/SKILL.md b/packages/darnit/src/darnit/skills/darnit-remediate/SKILL.md index 1c427b333..3c2a9c4d8 100644 --- a/packages/darnit/src/darnit/skills/darnit-remediate/SKILL.md +++ b/packages/darnit/src/darnit/skills/darnit-remediate/SKILL.md @@ -4,7 +4,7 @@ description: Apply automated fixes for failing compliance controls. Shows a plan compatibility: Requires darnit MCP server running (darnit serve) metadata: author: kusari-oss - version: "2.1" + version: "2.2" --- # Compliance Remediation @@ -15,7 +15,7 @@ Show a dry-run plan of fixes for failing controls, get confirmation, then apply ### 1. Show dry-run plan -Call `remediate_audit_findings` with `dry_run: true` and any profile mentioned. +Call `remediate_audit_findings` with `dry_run: true`, `owner` and `repo` naming the audited repository, and any profile mentioned. The tool internally runs an audit (or uses cached results) — do NOT run a separate audit call. The response is a Markdown report followed by a fenced JSON block (the run record). Read the `plan` in the JSON block: each item has `file_changes`, `change_sets` (platform settings), `commands`, `previewable`, `requires_individual_approval`, and a `digest`; each change set has its own `digest`. @@ -32,6 +32,7 @@ Ask the person which changes to apply. Ask about each platform change set and ea ### 2. Apply fixes (if confirmed) Call `remediate_audit_findings` with: +- `owner` and `repo` naming the audited repository - `dry_run: false` - `approve: [...]`: only the digests (change-set digests and plan-item digests) the person approved, copied from the preview's JSON block - `branch_name: "fix/compliance"` (or `"fix/compliance-{profile}"` if a profile was specified) @@ -97,10 +98,10 @@ Show: branch name, controls fixed, files changed, PR URL (if created), and remai - `enable_branch_protection` follows the same rules: it previews by default, never lowers existing settings, and applies only with `dry_run: false` and `approve` set to the change-set digest the person approved. - Register darnit's MCP server at user scope (`darnit install` does this by default); a repository-scoped `uv run darnit serve` runs the repository's own copy of darnit. - Only controls whose not-applicable claim is `honored` are skipped. Controls with a `pending` or `contradicted` claim are remediated like any other failing control. Do not confirm a pending claim to avoid a remediation; confirm one only when the operator explicitly tells you to. -- If there are unresolved data questions, the remediation tool will block and tell you. Suggest running `/darnit-data` first. +- If there are unresolved data questions, the remediation tool will block and tell you. Call `get_pending_data` with `owner`, `repo`, and `local_path` and follow the `procedure` in its result, then preview again. - The §3a verification-prompt-block files (today: `THREAT_MODEL.md` / `docs/threatmodel/SUMMARY.md`) are the ONE exception to §3b's "trust the template" stance. Always ask the user before doing a large triage pass on these — don't silently rewrite dozens of findings, and don't silently skip them either. The default "skip enhancement" rule from §3b is wrong for these files. ## Error handling -If the tool returns a data warning, suggest running `/darnit-data` first. +If the tool returns a data warning, call `get_pending_data` and follow the `procedure` in its result, then preview again. If branch creation fails, report the error — the tool aborts before applying any changes. diff --git a/tests/darnit/server/test_judgment_requests.py b/tests/darnit/server/test_judgment_requests.py index 4cbc74f18..2dab0df85 100644 --- a/tests/darnit/server/test_judgment_requests.py +++ b/tests/darnit/server/test_judgment_requests.py @@ -228,3 +228,17 @@ def test_get_judgment_requests_returns_its_text_without_a_structured_copy(tmp_pa tools = {tool.name: tool for tool in asyncio.new_event_loop().run_until_complete(server.list_tools())} assert tools["get_judgment_requests"].outputSchema is None + + +@pytest.mark.integration +def test_every_page_carries_the_judgment_procedure(tmp_path: Path) -> None: + checkout = _big_repo(tmp_path) + + pages = _all_pages(checkout) + + assert len(pages) > 1 + for page in pages: + text = "\n".join(page["procedure"]) + assert "submit_judgment(" in text + assert f'owner="{OWNER}", repo="{REPO}", local_path="{checkout}"' in text + assert "get_judgment_requests(" in text diff --git a/tests/darnit/server/test_procedures.py b/tests/darnit/server/test_procedures.py new file mode 100644 index 000000000..86f41f7df --- /dev/null +++ b/tests/darnit/server/test_procedures.py @@ -0,0 +1,57 @@ +"""The procedures data and judgment tools return to a coding agent (framework-design 5.4, 7.10).""" + +from __future__ import annotations + +import json +import re +from pathlib import Path + +import pytest + +from darnit.server.procedures import data_collection_procedure, data_next_step, judgment_procedure + +SKILLS = Path(__file__).resolve().parents[3] / "packages" / "darnit" / "src" / "darnit" / "skills" + + +@pytest.mark.unit +class TestProcedures: + def test_data_calls_name_the_repository_the_caller_named(self) -> None: + text = "\n".join(data_collection_procedure("acme", "widget", "/work/widget")) + + assert 'confirm_project_data(owner="acme", repo="widget", local_path="/work/widget"' in text + assert 'get_pending_data(owner="acme", repo="widget", local_path="/work/widget")' in text + + def test_an_unnamed_repository_is_a_placeholder(self) -> None: + text = "\n".join(data_collection_procedure(None, None, "/work/widget")) + + assert "owner=" in text + assert "repo=" in text + + def test_get_pending_data_calls_carry_no_host(self) -> None: + assert "host=" not in data_next_step("acme", "widget", "/w") + assert "host=" not in "\n".join(data_collection_procedure("acme", "widget", "/w")) + + def test_judgment_calls_carry_a_non_default_host(self) -> None: + text = "\n".join(judgment_procedure("acme", "widget", "git.example.org", "/w")) + + assert 'host="git.example.org"' in text + assert "host=" not in "\n".join(judgment_procedure("acme", "widget", "github.com", "/w")) + + def test_procedures_are_short_ascii_steps(self) -> None: + for steps in (data_collection_procedure("a", "b", "/w"), judgment_procedure("a", "b", "github.com", "/w")): + text = json.dumps(steps) + assert len(text) < 2_000 + assert text.isascii() + + +@pytest.mark.unit +def test_no_skill_tells_the_agent_to_follow_another_skill() -> None: + """A skill defers to the procedure in a tool result, never to another skill's text (which a client may not load).""" + pattern = re.compile(r"follow\s+(the\s+)?`?/?darnit[:\w-]*`?\s+skill", re.IGNORECASE) + offenders = [ + f"{path.parent.name}: {match.group(0)}" + for path in sorted(SKILLS.glob("*/SKILL.md")) + for match in pattern.finditer(path.read_text(encoding="utf-8")) + ] + + assert offenders == [] diff --git a/tests/darnit_baseline/test_get_pending_context.py b/tests/darnit_baseline/test_get_pending_context.py index 473bb0796..53a05a4d7 100644 --- a/tests/darnit_baseline/test_get_pending_context.py +++ b/tests/darnit_baseline/test_get_pending_context.py @@ -6,8 +6,11 @@ """ import json +import subprocess from unittest.mock import patch +import pytest + from darnit.config.context_schema import ( ContextDefinition, ContextPromptRequest, @@ -602,3 +605,99 @@ def test_context_key_order_is_every_framework_key(self) -> None: assert "platform" in order assert "license_type" in order, "feature 043 T055c: a LICENSE is created only for a confirmed license" assert len(order) == len(set(order)) == 10 + + +class TestGetPendingDataProcedure: + """The result carries the steps to follow (framework-design 7.10).""" + + def _candidate_request(self) -> ContextPromptRequest: + return ContextPromptRequest( + key="ci_provider", + definition=ContextDefinition( + type=ContextType.STRING, prompt="What CI provider?", auto_detect=True, affects=["CTRL-01"] + ), + control_ids=["CTRL-01"], + priority=1, + candidate=_detected("ci_provider", "detected-ci-value"), + ) + + @patch("darnit_baseline.tools.Path") + @patch("darnit.config.context_storage.get_pending_context") + def test_procedure_names_the_repository_the_caller_named(self, mock_get, mock_path) -> None: + mock_path.return_value.resolve.return_value = "/tmp/repo" + mock_get.return_value = _make_pending(2) + + data = _parse_json_from_result(get_pending_data(local_path="/tmp/repo", owner="acme", repo="widget")) + + text = "\n".join(data["procedure"]) + assert 'confirm_project_data(owner="acme", repo="widget", local_path="/tmp/repo"' in text + assert 'get_pending_data(owner="acme", repo="widget", local_path="/tmp/repo")' in text + + @patch("darnit.core.utils.detect_owner_repo", return_value=("remote-owner", "remote-repo")) + @patch("darnit_baseline.tools.Path") + @patch("darnit.config.context_storage.get_pending_context") + def test_procedure_never_names_a_repository_read_from_the_remote(self, mock_get, mock_path, _detect) -> None: + mock_path.return_value.resolve.return_value = "/tmp/repo" + mock_get.return_value = _make_pending(2) + + data = _parse_json_from_result(get_pending_data(local_path="/tmp/repo")) + + text = "\n".join(data["procedure"]) + assert "remote-owner" not in text + assert "remote-repo" not in text + assert "owner=" in text + + @patch("darnit_baseline.tools.Path") + @patch("darnit.config.context_storage.get_pending_context") + def test_procedure_holds_no_candidate_value(self, mock_get, mock_path) -> None: + mock_path.return_value.resolve.return_value = "/tmp/repo" + mock_get.return_value = [self._candidate_request()] + + data = _parse_json_from_result(get_pending_data(local_path="/tmp/repo", owner="acme", repo="widget")) + + assert "detected-ci-value" not in json.dumps(data["procedure"]) + assert "accept_candidates" in "\n".join(data["procedure"]) + + @patch("darnit_baseline.tools.Path") + @patch("darnit.config.context_storage.get_pending_context") + def test_complete_result_carries_the_procedure(self, mock_get, mock_path) -> None: + mock_path.return_value.resolve.return_value = "/tmp/repo" + mock_get.return_value = [] + + data = json.loads(get_pending_data(local_path="/tmp/repo", owner="acme", repo="widget")) + + assert data["status"] == "complete" + assert any("stored_unconfirmed" in step for step in data["procedure"]) + + @patch("darnit_baseline.tools.Path") + @patch("darnit.config.context_storage.get_pending_context") + def test_directive_defers_to_the_procedure(self, mock_get, mock_path) -> None: + mock_path.return_value.resolve.return_value = "/tmp/repo" + mock_get.return_value = _make_pending(1) + + result = get_pending_data(local_path="/tmp/repo") + + assert result.startswith(_LLM_DIRECTIVE_PREFIX) + assert '"procedure"' in _LLM_DIRECTIVE_PREFIX + + +class TestConfirmProjectDataNextStep: + @pytest.fixture(autouse=True) + def _git_repo(self, tmp_path) -> None: + subprocess.run(["git", "init", "-q", str(tmp_path)], check=True) + + def test_result_names_the_next_get_pending_data_call(self, tmp_path) -> None: + from darnit_baseline.tools import confirm_project_data + + result = confirm_project_data(local_path=str(tmp_path), maintainers="@someone") + + assert "refused" in result + assert "Next: call get_pending_data(owner=" in result + assert f'local_path="{tmp_path.resolve()}"' in result + + def test_neutral_tool_names_no_get_pending_data_call(self, tmp_path) -> None: + from darnit.server.tools.project_data import confirm_project_data_impl + + result = confirm_project_data_impl(str(tmp_path), maintainers="@someone", framework_name="openssf-baseline") + + assert "get_pending_data" not in result From 12b7a46d57b18dfb110792080cdcacebd8fe8349 Mon Sep 17 00:00:00 2001 From: Michael Lieberman Date: Wed, 21 Oct 2026 00:24:38 -0400 Subject: [PATCH 03/10] feat(mcp): list the controls get_judgment_requests skipped A page now names, in 'skipped', the controls left out because they already have a stored PASS candidate, and the judging procedure says not to request them again. In the #644 e2e runs the agent called the tool again with those ids and got empty pages back. Refs #644. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Michael Lieberman --- docs/architecture/framework-design.md | 2 +- packages/darnit/src/darnit/server/procedures.py | 1 + .../darnit/src/darnit/server/tools/judgments.py | 16 +++++++++++++++- tests/darnit/server/test_judgment_requests.py | 3 +++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/docs/architecture/framework-design.md b/docs/architecture/framework-design.md index ccd7e8e22..845083dc5 100644 --- a/docs/architecture/framework-design.md +++ b/docs/architecture/framework-design.md @@ -1405,7 +1405,7 @@ get_judgment_requests( ) -> {status: "ok", files, requests, remaining, oversized?} | {status: "rejected" | "error", rejection | error} ``` -A coding agent obtains what it judges through `get_judgment_requests`, registered beside `submit_judgment` and requiring the same named repository identity. It audits `local_path` the same way and returns one request per control left `PENDING` (`llm_judgment`), optionally limited to `control_ids`: the control's `prompt`, `analysis_hints`, `target_key` when set, `evidence_digest`, and the keys of its files in a shared `files` map. Each `files` entry holds `path`, `digest`, and `content`, where `content` is exactly the content `submit_judgment` checks the control's citations against (including its caps); a file read by several controls appears once, under a distinct key only if its content differs. `gathered_evidence` and the rest of the control's evidence are not returned. A control with a stored candidate for its current digest (`PENDING`, `confirmation`) is left out unless `include_judged` is true. Requests are ordered by control id and paged: a page holds whole requests and its serialized result stays under 40,000 bytes (UTF-8), below the size at which a coding agent's client writes a tool result to a file instead of showing it, and `remaining` lists the controls not yet returned, to be passed as `control_ids` on the next call. A single request larger than the page limit is returned alone, with `oversized = true`. The tool records nothing, and `submit_judgment` never trusts its output. +A coding agent obtains what it judges through `get_judgment_requests`, registered beside `submit_judgment` and requiring the same named repository identity. It audits `local_path` the same way and returns one request per control left `PENDING` (`llm_judgment`), optionally limited to `control_ids`: the control's `prompt`, `analysis_hints`, `target_key` when set, `evidence_digest`, and the keys of its files in a shared `files` map. Each `files` entry holds `path`, `digest`, and `content`, where `content` is exactly the content `submit_judgment` checks the control's citations against (including its caps); a file read by several controls appears once, under a distinct key only if its content differs. `gathered_evidence` and the rest of the control's evidence are not returned. A control with a stored candidate for its current digest (`PENDING`, `confirmation`) is left out unless `include_judged` is true, and is listed in `skipped` so the agent does not request it again. Requests are ordered by control id and paged: a page holds whole requests and its serialized result stays under 40,000 bytes (UTF-8), below the size at which a coding agent's client writes a tool result to a file instead of showing it, and `remaining` lists the controls not yet returned, to be passed as `control_ids` on the next call. A single request larger than the page limit is returned alone, with `oversized = true`. The tool records nothing, and `submit_judgment` never trusts its output. Each `get_judgment_requests` page also carries `procedure`: the ordered steps for judging the page (judge each request from its prompt, hints, and listed files only; call `submit_judgment` once per request with passages copied verbatim; a pass is a PASS candidate and a fail a suggestive finding, never the audit result; fetch the next page with `remaining`). The steps name the repository and checkout the caller passed and hold placeholders for everything else. The procedure text lives in one place in the framework (`darnit.server.procedures`), so a skill or client defers to it instead of restating it. diff --git a/packages/darnit/src/darnit/server/procedures.py b/packages/darnit/src/darnit/server/procedures.py index 5600c9a88..0c79723ca 100644 --- a/packages/darnit/src/darnit/server/procedures.py +++ b/packages/darnit/src/darnit/server/procedures.py @@ -75,5 +75,6 @@ def judgment_procedure(owner: str, repo: str, host: str, local_path: str) -> lis "result.", f"4. If remaining is not empty, call get_judgment_requests({args}, control_ids=) " "and repeat; stop when remaining is empty.", + "5. Controls in skipped already have a stored PASS candidate: do not request or judge them again.", "Confirm a PASS candidate only when the operator explicitly names it.", ] diff --git a/packages/darnit/src/darnit/server/tools/judgments.py b/packages/darnit/src/darnit/server/tools/judgments.py index 2bafc05af..108196373 100644 --- a/packages/darnit/src/darnit/server/tools/judgments.py +++ b/packages/darnit/src/darnit/server/tools/judgments.py @@ -202,6 +202,18 @@ def dump(payload: dict[str, Any]) -> str: ), key=lambda r: r["id"], ) + skipped = ( + [] + if include_judged + else sorted( + r["id"] + for r in results + if r.get("status") == "PENDING" + and (r.get("pending") or {}).get("kind") == "confirmation" + and (r.get("evidence") or {}).get("llm_consultation") + and (wanted is None or r["id"] in wanted) + ) + ) page: dict[str, Any] = { "status": "ok", @@ -209,6 +221,7 @@ def dump(payload: dict[str, Any]) -> str: "files": {}, "requests": [], "remaining": [], + "skipped": skipped, } oversized = False for index, result in enumerate(pending): @@ -262,7 +275,8 @@ async def get_judgment_requests( ``analysis_hints``, ``evidence_digest``, and ``files``, keys into the shared ``files`` map), ``files`` (``path``, ``digest``, and ``content``, exactly the content ``submit_judgment`` checks citations against), and - ``remaining`` (controls not on this page). Pages stay under 40,000 + ``remaining`` (controls not on this page), and ``skipped`` (controls + left out because they have a stored PASS candidate). Pages stay under 40,000 bytes; call again with ``control_ids`` set to ``remaining`` until it is empty. Read only: records nothing. diff --git a/tests/darnit/server/test_judgment_requests.py b/tests/darnit/server/test_judgment_requests.py index 2dab0df85..7146a501b 100644 --- a/tests/darnit/server/test_judgment_requests.py +++ b/tests/darnit/server/test_judgment_requests.py @@ -80,6 +80,7 @@ def test_request_carries_prompt_hints_and_the_judged_files(self, repo: Path) -> assert response["status"] == "ok" assert response["remaining"] == [] + assert response["skipped"] == [] [request] = response["requests"] assert request["control_id"] == CONTROL assert request["kind"] == "llm_judgment" @@ -138,6 +139,8 @@ def test_a_stored_candidate_is_left_out_unless_asked_for(self, repo: Path) -> No ) assert _requests(repo)["requests"] == [] + assert _requests(repo)["skipped"] == [CONTROL] + assert _requests(repo, include_judged=True)["skipped"] == [] [request] = _requests(repo, include_judged=True)["requests"] assert request["control_id"] == CONTROL assert request["kind"] == "confirmation" From 4c358e2a9d5377067f91b8d6ed906898546223c5 Mon Sep 17 00:00:00 2001 From: Michael Lieberman Date: Thu, 5 Nov 2026 10:02:05 -0500 Subject: [PATCH 04/10] docs(spec): the data procedure names host, maps answers, keeps control_ids Section 7.10 and the 042 contract: get_pending_data takes no host, so the confirm_project_data call in its procedure names host as a placeholder to fill for a repository not on github.com. Answers are mapped by answer_mapping, where a No on a candidate, or Other, is the value the person typed. The next get_pending_data call keeps the caller's control_ids. The contract now lists procedure. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Michael Lieberman --- docs/architecture/framework-design.md | 2 +- .../contracts/context-confirmation-tools.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/architecture/framework-design.md b/docs/architecture/framework-design.md index 845083dc5..7968106d0 100644 --- a/docs/architecture/framework-design.md +++ b/docs/architecture/framework-design.md @@ -1759,7 +1759,7 @@ Each context key has one canonical name and one vocabulary, taken from the frame The observable payloads of `get_pending_data` and `confirm_project_data` are defined in `specs/042-candidate-integrity/contracts/context-confirmation-tools.md`: -- `get_pending_data` is read-only. Each question carries its candidate as data (`candidate: {value, origin, digest, label}`, where `digest` is the value digest (7.5) and `label` marks it unconfirmed); the candidate's value appears nowhere else in the response. `command_template` holds placeholders only (``, ``, `owner=..., repo=...`), never a candidate value or a configuration example. `answer_mapping.value_map["Yes"]` for a candidate is `{"accept_candidates": {"": ""}}`, a placeholder the agent fills from the question's `candidate.digest` only after the person answered yes. Enum questions list every allowed value in `allowed_values`; when there are more than 4, the selector (`ask_user`) is omitted and the prompt lists them all. `examples` appear only as `format_hint`. Every question in the batch is returned under `questions`, with or without a selector. The response lists `stored_unconfirmed` values with their locations. The response also carries `procedure`: the ordered steps for collecting the data (show each candidate with its origin, labelled unconfirmed; ask every question, passing `ask_user_batch` verbatim; per-question skip; one `confirm_project_data` call per batch, with `accept_candidates` filled only after a yes; call `get_pending_data` again until `complete`; review `stored_unconfirmed`; summarize). Its calls name only the `owner` and `repo` the caller passed, never ones detected from the checkout's remote (placeholders otherwise), and hold no candidate or detected value. A `confirm_project_data` result that handled context values on a server with `get_pending_data` ends with the next `get_pending_data` call, named the same way. +- `get_pending_data` is read-only. Each question carries its candidate as data (`candidate: {value, origin, digest, label}`, where `digest` is the value digest (7.5) and `label` marks it unconfirmed); the candidate's value appears nowhere else in the response. `command_template` holds placeholders only (``, ``, `owner=..., repo=...`), never a candidate value or a configuration example. `answer_mapping.value_map["Yes"]` for a candidate is `{"accept_candidates": {"": ""}}`, a placeholder the agent fills from the question's `candidate.digest` only after the person answered yes. Enum questions list every allowed value in `allowed_values`; when there are more than 4, the selector (`ask_user`) is omitted and the prompt lists them all. `examples` appear only as `format_hint`. Every question in the batch is returned under `questions`, with or without a selector. The response lists `stored_unconfirmed` values with their locations. The response also carries `procedure`: the ordered steps for collecting the data (show each candidate with its origin, labelled unconfirmed; ask every question, passing `ask_user_batch` verbatim; per-question skip; one `confirm_project_data` call per batch, mapping each answer by `answer_mapping` (Yes/No on a yes/no question is a boolean; No on a candidate, or Other, is the value the person typed), with `accept_candidates` filled only after a yes; call `get_pending_data` again, with the caller's `control_ids`, until `complete`; review `stored_unconfirmed`; summarize). Its calls name only the `owner` and `repo` the caller passed, never ones detected from the checkout's remote (placeholders otherwise), and hold no candidate or detected value. `get_pending_data` takes no `host`, so its `confirm_project_data` call names `host` as a placeholder to fill when the repository is not on github.com (7.10). A `confirm_project_data` result that handled context values on a server with `get_pending_data` ends with the next `get_pending_data` call, named the same way. - `confirm_project_data` takes a parameter for every context key that the server's bound framework defines, and no other framework's keys (generated from the definitions; an enum parameter accepts only its `values`), plus `accept_candidates` (`{key: candidate digest}`: detection runs again and the current candidate, or a value concluded in this run, is confirmed only if its digest still matches, recording its value and origin as the basis; a mismatch, a key with no candidate, or a key given both an answer and a digest is refused and nothing is written for it), `confirm_stored` and `reject_stored` (review of stored values: a rejected `.project/darnit.yaml` value is deleted; a rejected `.project/project.yaml` value is reported with its file and field and the file is left unchanged), `expires_at` (`{key: date}`), and `owner`, `repo`, `host`, which are required and decide the record location (7.5). `confirm_stored` records a confirmation of the value currently stored for each key (basis: that value, origin `stored_unconfirmed` or `expired_confirmation`). Like a confirmation, a rejection writes to `.project/darnit.yaml` only when the operator trusts the named repository; otherwise it is refused and names the file and field for the person to edit. The result names, per key, `confirmed (in-repository)` with the file written or `confirmed (operator-side)`, `rejected`, `edit required: :`, or `refused: `. - **Which server exposes it**: each server's `confirm_project_data` covers only the keys of the framework the server is bound to. The OpenSSF Baseline server defines its own (which also carries `confirm_not_applicable` and `confirm_pass_candidate`). Every other framework server that defines context keys and whose TOML does not define a tool named `confirm_project_data` gets a framework-neutral builtin `confirm_project_data` (a framework with no context keys gets none), registered by the factory the same way as `submit_judgment` and `confirm_pass_candidate` (5.4) and bound to that framework's context definitions. It records context values only: the per-key parameters, `accept_candidates`, `confirm_stored`, `reject_stored`, `expires_at`, `owner`, `repo`, `host`, and `local_path`. Claim confirmations and PASS candidates stay on their existing tools. - Remediation prompts (the preflight and `format_context_prompt`) show a candidate as labelled, unconfirmed data with its origin and digest, name the key's hint source files that exist without parsing values out of them, and hold placeholders only in their commands. diff --git a/specs/042-candidate-integrity/contracts/context-confirmation-tools.md b/specs/042-candidate-integrity/contracts/context-confirmation-tools.md index f87fe816e..a7b6d44a2 100644 --- a/specs/042-candidate-integrity/contracts/context-confirmation-tools.md +++ b/specs/042-candidate-integrity/contracts/context-confirmation-tools.md @@ -25,6 +25,7 @@ Writes nothing. Each question: - `command_template` never contains a candidate value or a configuration example. - Enum questions list every allowed value in `allowed_values`; the selector options are omitted when there are more than 4. +- The response carries `procedure`, the ordered steps for collecting the data (framework-design 7.10, `darnit.server.procedures`). Its calls name only the `owner`, `repo`, and `control_ids` the caller passed; the `confirm_project_data` call names `host` as a placeholder, since `get_pending_data` takes no `host`. It holds no candidate or detected value. - `answer_mapping.value_map["Yes"]` is `{"accept_candidates": {"": ""}}`: a placeholder the agent fills from the question's `candidate.digest` only after the person has actually answered yes. The mapping never contains a ready-to-send digest, and it applies only to the person's own answer; accepting a candidate by digest is a confirmation and follows the same rule as any other. Response also includes: From 242a0bc7b7769b4e87040c4ef478c128f0f76168 Mon Sep 17 00:00:00 2001 From: Michael Lieberman Date: Thu, 5 Nov 2026 10:02:05 -0500 Subject: [PATCH 05/10] fix(mcp): the data procedure names host and maps a No on a candidate Review of #671. The procedure that replaced the old directive lost two of its instructions: - Step 4 built the confirm_project_data call without host, so on a repository not hosted on github.com an agent following it verbatim recorded confirmations under github.com. get_pending_data takes no host, so step 4 now names it as a placeholder. - Step 4 said only that Yes/No questions take true/false. A candidate question is also Yes/No, so a No could be sent as false. Step 4 now maps answers by answer_mapping: a No on a candidate, or Other, is the value the person typed. Step 5 and the next step after confirm_project_data keep the caller's control_ids, so a scoped collection stays scoped. The ASCII test checked json.dumps output, which is always ASCII; it now checks the text itself. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Michael Lieberman --- CHANGELOG.md | 9 +++- .../src/darnit_baseline/tools.py | 2 +- .../darnit/src/darnit/server/procedures.py | 44 ++++++++++++++----- tests/darnit/server/test_procedures.py | 35 +++++++++++++-- .../test_get_pending_context.py | 2 +- 5 files changed, 76 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc9223d15..8ed1236be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -452,7 +452,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 instead of restating them or telling the agent to follow another skill, which a client may not load, so `darnit-comply` collected data differently from `darnit-data`. The comply and remediate skills now pass - `owner` and `repo` to `remediate_audit_findings`. + `owner` and `repo` to `remediate_audit_findings`, and `darnit-comply` + always calls `get_pending_data` (it used to only when WARN controls + were waiting on data); with nothing pending it returns `complete`. + The data procedure's `confirm_project_data` call names `host` as a + placeholder for repositories not on github.com, maps answers by + `answer_mapping` (a No on a candidate is the value the person typed, + never `false`), and keeps the caller's `control_ids` on the next + `get_pending_data` call. - **BREAKING:** `register_handlers()` is the one handler registration hook for plugins (#451). `darnit-gittuf` and `darnit-reproducibility` rename `register_sieve_handlers()` to `register_handlers()`, and their `register()` diff --git a/packages/darnit-baseline/src/darnit_baseline/tools.py b/packages/darnit-baseline/src/darnit_baseline/tools.py index a21135d8d..0590fc1ed 100644 --- a/packages/darnit-baseline/src/darnit_baseline/tools.py +++ b/packages/darnit-baseline/src/darnit_baseline/tools.py @@ -803,7 +803,7 @@ def get_pending_data( target = target_from_owner_repo(owner, repo) # Only the repository the caller named goes into the procedure's calls, # never one detected from the checkout's remote. - procedure = data_collection_procedure(owner, repo, str(repo_path)) + procedure = data_collection_procedure(owner, repo, str(repo_path), control_ids) # Auto-detect owner/repo from git if owner is None or repo is None: diff --git a/packages/darnit/src/darnit/server/procedures.py b/packages/darnit/src/darnit/server/procedures.py index 0c79723ca..01438643f 100644 --- a/packages/darnit/src/darnit/server/procedures.py +++ b/packages/darnit/src/darnit/server/procedures.py @@ -14,30 +14,52 @@ _OWNER = "" _REPO = "" +# get_pending_data takes no host, so the confirm_project_data call it +# describes carries a placeholder, never a host read from the checkout. +_HOST = "" -def _args(owner: str | None, repo: str | None, host: str | None, local_path: str) -> str: +def _args( + owner: str | None, + repo: str | None, + host: str | None, + local_path: str, + *, + host_placeholder: bool = False, + control_ids: list[str] | None = None, +) -> str: parts = [ f"owner={json.dumps(owner) if owner else _OWNER}", f"repo={json.dumps(repo) if repo else _REPO}", ] if host and host != "github.com": parts.append(f"host={json.dumps(host)}") + elif host_placeholder: + parts.append(f"host={_HOST}") parts.append(f"local_path={json.dumps(local_path)}") + if control_ids: + parts.append(f"control_ids={json.dumps(control_ids)}") return ", ".join(parts) def data_next_step(owner: str | None, repo: str | None, local_path: str) -> str: """The call that follows a ``confirm_project_data`` call (``get_pending_data`` takes no host).""" return ( - f"Next: call get_pending_data({_args(owner, repo, None, local_path)}) for the remaining questions " - "and follow its procedure; when it returns status complete, re-run the audit." + f"Next: call get_pending_data({_args(owner, repo, None, local_path)}), with the same control_ids " + "if you passed any, for the remaining questions and follow its procedure; when it returns status " + "complete, re-run the audit." ) -def data_collection_procedure(owner: str | None, repo: str | None, local_path: str) -> list[str]: +def data_collection_procedure( + owner: str | None, + repo: str | None, + local_path: str, + control_ids: list[str] | None = None, +) -> list[str]: """Ordered steps for collecting project data from the person (``get_pending_data`` takes no host).""" - args = _args(owner, repo, None, local_path) + confirm_args = _args(owner, repo, None, local_path, host_placeholder=True) + next_args = _args(owner, repo, None, local_path, control_ids=control_ids) return [ "1. For each question with a candidate, show the person candidate.value and candidate.origin, " "labelled UNCONFIRMED. A candidate is never a fact or a default answer.", @@ -45,11 +67,13 @@ def data_collection_procedure(owner: str | None, repo: str | None, local_path: s "Claude Code); ask a question with no ask_user entry in plain text, listing every allowed_values " "entry. format_hint is a format, never an answer. Do not pre-fill or suggest answers.", "3. If the person says skip or does not know, leave that key out and move on.", - f"4. Record the answers in one call: confirm_project_data({args}, =). " - "Yes/No questions take true/false. For a Yes on a candidate pass " - 'accept_candidates={"": ""}, filled in only after the ' - "person said yes. Pass only what the person said, never a detected value.", - f"5. Call get_pending_data({args}) again and repeat from step 1 until status is complete.", + f"4. Record the answers in one call: confirm_project_data({confirm_args}, =), using answer_mapping: Yes/No on a yes/no question is true/false; a chosen option is its " + 'label as a string; Yes on a candidate is accept_candidates={"": ""}, filled in only after the person said yes; No on a candidate, or Other, is ' + "the value the person typed (ask for it if they did not type one). Pass only what the person " + "said, never a detected value.", + f"5. Call get_pending_data({next_args}) again and repeat from step 1 until status is complete.", "6. If stored_unconfirmed is not empty, show every entry (key, value, location) labelled " "unconfirmed, ask which to keep and which to reject, and make one confirm_project_data call with " "confirm_stored and reject_stored holding only the keys the person named. For an edit required " diff --git a/tests/darnit/server/test_procedures.py b/tests/darnit/server/test_procedures.py index 86f41f7df..e84bae0ea 100644 --- a/tests/darnit/server/test_procedures.py +++ b/tests/darnit/server/test_procedures.py @@ -18,7 +18,10 @@ class TestProcedures: def test_data_calls_name_the_repository_the_caller_named(self) -> None: text = "\n".join(data_collection_procedure("acme", "widget", "/work/widget")) - assert 'confirm_project_data(owner="acme", repo="widget", local_path="/work/widget"' in text + assert ( + 'confirm_project_data(owner="acme", repo="widget", host=, local_path="/work/widget"' + in text + ) assert 'get_pending_data(owner="acme", repo="widget", local_path="/work/widget")' in text def test_an_unnamed_repository_is_a_placeholder(self) -> None: @@ -29,7 +32,33 @@ def test_an_unnamed_repository_is_a_placeholder(self) -> None: def test_get_pending_data_calls_carry_no_host(self) -> None: assert "host=" not in data_next_step("acme", "widget", "/w") - assert "host=" not in "\n".join(data_collection_procedure("acme", "widget", "/w")) + steps = data_collection_procedure("acme", "widget", "/w") + pending_calls = re.findall(r"get_pending_data\([^)]*\)", "\n".join(steps)) + assert pending_calls + assert all("host=" not in call for call in pending_calls) + + def test_confirm_call_names_the_host_as_a_placeholder(self) -> None: + """get_pending_data takes no host, so step 4 names it as a placeholder, never a detected value (7.10).""" + step4 = data_collection_procedure("acme", "widget", "/w")[3] + + assert "confirm_project_data(" in step4 + assert "host=" in step4 + + def test_answers_follow_answer_mapping(self) -> None: + """A No on a candidate is the typed value, never false; Yes/No maps to booleans only on yes/no questions.""" + step4 = data_collection_procedure("acme", "widget", "/w")[3] + + assert "answer_mapping" in step4 + assert "Yes/No on a yes/no question is true/false" in step4 + assert "No on a candidate, or Other, is the value the person typed" in step4 + + def test_a_scoped_collection_stays_scoped(self) -> None: + steps = data_collection_procedure("acme", "widget", "/w", ["OSPS-GV-01.01"]) + + assert ( + 'get_pending_data(owner="acme", repo="widget", local_path="/w", control_ids=["OSPS-GV-01.01"])' in steps[4] + ) + assert "same control_ids" in data_next_step("acme", "widget", "/w") def test_judgment_calls_carry_a_non_default_host(self) -> None: text = "\n".join(judgment_procedure("acme", "widget", "git.example.org", "/w")) @@ -39,7 +68,7 @@ def test_judgment_calls_carry_a_non_default_host(self) -> None: def test_procedures_are_short_ascii_steps(self) -> None: for steps in (data_collection_procedure("a", "b", "/w"), judgment_procedure("a", "b", "github.com", "/w")): - text = json.dumps(steps) + text = json.dumps(steps, ensure_ascii=False) assert len(text) < 2_000 assert text.isascii() diff --git a/tests/darnit_baseline/test_get_pending_context.py b/tests/darnit_baseline/test_get_pending_context.py index 53a05a4d7..91b29390e 100644 --- a/tests/darnit_baseline/test_get_pending_context.py +++ b/tests/darnit_baseline/test_get_pending_context.py @@ -630,7 +630,7 @@ def test_procedure_names_the_repository_the_caller_named(self, mock_get, mock_pa data = _parse_json_from_result(get_pending_data(local_path="/tmp/repo", owner="acme", repo="widget")) text = "\n".join(data["procedure"]) - assert 'confirm_project_data(owner="acme", repo="widget", local_path="/tmp/repo"' in text + assert 'confirm_project_data(owner="acme", repo="widget", host=, local_path="/tmp/repo"' in text assert 'get_pending_data(owner="acme", repo="widget", local_path="/tmp/repo")' in text @patch("darnit.core.utils.detect_owner_repo", return_value=("remote-owner", "remote-repo")) From 92d0d9fc9db5ed247c1756eb57dddc6d64fa7541 Mon Sep 17 00:00:00 2001 From: Michael Lieberman Date: Wed, 21 Oct 2026 11:47:30 -0400 Subject: [PATCH 06/10] fix(data): ask every project-data question through the question tool Since #552, _build_ask_user_params returned no selector for free-text questions or for choice questions with more than 4 values, so a batch was split between Claude Code's question tool and plain text. Every question now has a selector: a long choice list shows its first 4 values and names all of them in the question (each typeable under Other, FR-015), and a free-text question offers "Skip for now" and "Answer in chat", never a value. Word-boundary headers and the yes/no hint in the question came from #657 and are kept as merged. The spec (7.x) and the data procedure say the same. Refs #650, #655. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Michael Lieberman --- docs/architecture/framework-design.md | 2 +- .../src/darnit_baseline/tools.py | 67 ++++++++++++------- .../darnit/src/darnit/server/procedures.py | 10 +-- .../test_get_pending_context.py | 38 +++++++---- tests/darnit_baseline/test_pending_payload.py | 25 ++++--- 5 files changed, 88 insertions(+), 54 deletions(-) diff --git a/docs/architecture/framework-design.md b/docs/architecture/framework-design.md index 7968106d0..f2e0fd71d 100644 --- a/docs/architecture/framework-design.md +++ b/docs/architecture/framework-design.md @@ -1759,7 +1759,7 @@ Each context key has one canonical name and one vocabulary, taken from the frame The observable payloads of `get_pending_data` and `confirm_project_data` are defined in `specs/042-candidate-integrity/contracts/context-confirmation-tools.md`: -- `get_pending_data` is read-only. Each question carries its candidate as data (`candidate: {value, origin, digest, label}`, where `digest` is the value digest (7.5) and `label` marks it unconfirmed); the candidate's value appears nowhere else in the response. `command_template` holds placeholders only (``, ``, `owner=..., repo=...`), never a candidate value or a configuration example. `answer_mapping.value_map["Yes"]` for a candidate is `{"accept_candidates": {"": ""}}`, a placeholder the agent fills from the question's `candidate.digest` only after the person answered yes. Enum questions list every allowed value in `allowed_values`; when there are more than 4, the selector (`ask_user`) is omitted and the prompt lists them all. `examples` appear only as `format_hint`. Every question in the batch is returned under `questions`, with or without a selector. The response lists `stored_unconfirmed` values with their locations. The response also carries `procedure`: the ordered steps for collecting the data (show each candidate with its origin, labelled unconfirmed; ask every question, passing `ask_user_batch` verbatim; per-question skip; one `confirm_project_data` call per batch, mapping each answer by `answer_mapping` (Yes/No on a yes/no question is a boolean; No on a candidate, or Other, is the value the person typed), with `accept_candidates` filled only after a yes; call `get_pending_data` again, with the caller's `control_ids`, until `complete`; review `stored_unconfirmed`; summarize). Its calls name only the `owner` and `repo` the caller passed, never ones detected from the checkout's remote (placeholders otherwise), and hold no candidate or detected value. `get_pending_data` takes no `host`, so its `confirm_project_data` call names `host` as a placeholder to fill when the repository is not on github.com (7.10). A `confirm_project_data` result that handled context values on a server with `get_pending_data` ends with the next `get_pending_data` call, named the same way. +- `get_pending_data` is read-only. Each question carries its candidate as data (`candidate: {value, origin, digest, label}`, where `digest` is the value digest (7.5) and `label` marks it unconfirmed); the candidate's value appears nowhere else in the response. `command_template` holds placeholders only (``, ``, `owner=..., repo=...`), never a candidate value or a configuration example. `answer_mapping.value_map["Yes"]` for a candidate is `{"accept_candidates": {"": ""}}`, a placeholder the agent fills from the question's `candidate.digest` only after the person answered yes. Every question carries a selector (`ask_user`), so one batch is asked in one round of the client's question tool, which offers at most 4 options and adds a free-text "Other". Enum questions list every allowed value in `allowed_values`; when there are more than 4, the selector offers the first 4 and its question names them all, and any of them can be typed under Other (every allowed value stays reachable). A free-text question's selector offers "Skip for now" and "Answer in chat" and never a value; the answer is typed under Other. A yes/no question's options carry neutral descriptions, and its hint is part of the question. A selector header is at most 12 characters, cut at a word boundary. `examples` appear only as `format_hint`, never as an option. Every question in the batch is returned under `questions`. The response lists `stored_unconfirmed` values with their locations. The response also carries `procedure`: the ordered steps for collecting the data (show each candidate with its origin, labelled unconfirmed; ask every question, passing `ask_user_batch` verbatim in one round; per-question skip; one `confirm_project_data` call per batch, mapping each answer by `answer_mapping` (Yes/No on a yes/no question is a boolean; No on a candidate, or Other, is the value the person typed), with `accept_candidates` filled only after a yes; call `get_pending_data` again, with the caller's `control_ids`, until `complete`; review `stored_unconfirmed`; summarize). Its calls name only the `owner` and `repo` the caller passed, never ones detected from the checkout's remote (placeholders otherwise), and hold no candidate or detected value. `get_pending_data` takes no `host`, so its `confirm_project_data` call names `host` as a placeholder to fill when the repository is not on github.com (7.10). A `confirm_project_data` result that handled context values on a server with `get_pending_data` ends with the next `get_pending_data` call, named the same way. - `confirm_project_data` takes a parameter for every context key that the server's bound framework defines, and no other framework's keys (generated from the definitions; an enum parameter accepts only its `values`), plus `accept_candidates` (`{key: candidate digest}`: detection runs again and the current candidate, or a value concluded in this run, is confirmed only if its digest still matches, recording its value and origin as the basis; a mismatch, a key with no candidate, or a key given both an answer and a digest is refused and nothing is written for it), `confirm_stored` and `reject_stored` (review of stored values: a rejected `.project/darnit.yaml` value is deleted; a rejected `.project/project.yaml` value is reported with its file and field and the file is left unchanged), `expires_at` (`{key: date}`), and `owner`, `repo`, `host`, which are required and decide the record location (7.5). `confirm_stored` records a confirmation of the value currently stored for each key (basis: that value, origin `stored_unconfirmed` or `expired_confirmation`). Like a confirmation, a rejection writes to `.project/darnit.yaml` only when the operator trusts the named repository; otherwise it is refused and names the file and field for the person to edit. The result names, per key, `confirmed (in-repository)` with the file written or `confirmed (operator-side)`, `rejected`, `edit required: :`, or `refused: `. - **Which server exposes it**: each server's `confirm_project_data` covers only the keys of the framework the server is bound to. The OpenSSF Baseline server defines its own (which also carries `confirm_not_applicable` and `confirm_pass_candidate`). Every other framework server that defines context keys and whose TOML does not define a tool named `confirm_project_data` gets a framework-neutral builtin `confirm_project_data` (a framework with no context keys gets none), registered by the factory the same way as `submit_judgment` and `confirm_pass_candidate` (5.4) and bound to that framework's context definitions. It records context values only: the per-key parameters, `accept_candidates`, `confirm_stored`, `reject_stored`, `expires_at`, `owner`, `repo`, `host`, and `local_path`. Claim confirmations and PASS candidates stay on their existing tools. - Remediation prompts (the preflight and `format_context_prompt`) show a candidate as labelled, unconfirmed data with its origin and digest, name the key's hint source files that exist without parsing values out of them, and hold placeholders only in their commands. diff --git a/packages/darnit-baseline/src/darnit_baseline/tools.py b/packages/darnit-baseline/src/darnit_baseline/tools.py index 0590fc1ed..6b81090c0 100644 --- a/packages/darnit-baseline/src/darnit_baseline/tools.py +++ b/packages/darnit-baseline/src/darnit_baseline/tools.py @@ -856,10 +856,7 @@ def get_pending_data( batch_questions = [] answer_mapping = [] for q in questions: - ask_user = q.get("ask_user") - if ask_user is None: - continue - batch_questions.append(ask_user) + batch_questions.append(q["ask_user"]) mapping: dict = { "question_index": len(batch_questions) - 1, "context_key": q["key"], @@ -871,6 +868,8 @@ def get_pending_data( } elif q["input_type"] == "select" and q.get("options") == ["true", "false"]: mapping["value_map"] = {"Yes": True, "No": False} + elif q["input_type"] == "free_text": + mapping["value_map"] = {SKIP_OPTION: "SKIP", CHAT_OPTION: "ASK_USER_FOR_VALUE"} answer_mapping.append(mapping) # Determine answered count from total minus pending @@ -993,23 +992,26 @@ def _build_context_question(req) -> dict: "repository owner, git config, or any other source. The format hint is not an answer." ) - ask_user = _build_ask_user_params(req.key, question, definition) - if ask_user is not None: - question["ask_user"] = ask_user + question["ask_user"] = _build_ask_user_params(req.key, question, definition) return question -def _build_ask_user_params(key: str, question_dict: dict, definition) -> dict | None: - """AskUserQuestion parameters (question/header/options/multiSelect), or None. +SKIP_OPTION = "Skip for now" +CHAT_OPTION = "Answer in chat" - None for free text and for an enum with more values than the selector - holds, so no allowed value is dropped (FR-015). A candidate's value is - not repeated here; the person sees it from the question's ``candidate``. + +def _build_ask_user_params(key: str, question_dict: dict, definition) -> dict: + """AskUserQuestion parameters (question/header/options/multiSelect) for every question. + + The question tool shows at most 4 options and always adds a free-text + "Other". A choice key with more values names them all in the question, so + each stays reachable under Other (FR-015). A free-text key offers skip and + chat options, never a value. A candidate's value is not repeated here; the + person sees it from the question's ``candidate``. """ input_type = question_dict.get("input_type") question_text = question_dict["prompt"] - header = _ask_user_header(key) if input_type == "confirm": @@ -1026,26 +1028,39 @@ def _build_ask_user_params(key: str, question_dict: dict, definition) -> dict | "multiSelect": False, } - if input_type != "select": - return None - - if definition.type == "boolean": + if input_type == "select" and definition.type == "boolean": if hint := question_dict.get("hint"): question_text = f"{question_text} {hint}" - options = [ - {"label": "Yes", "description": "Yes, this applies"}, - {"label": "No", "description": "No, this does not apply"}, - ] - else: + return { + "question": question_text, + "header": header, + "options": [ + {"label": "Yes", "description": "Yes, this applies"}, + {"label": "No", "description": "No, this does not apply"}, + ], + "multiSelect": False, + } + + if input_type == "select": values = question_dict["options"] if len(values) > _SELECTOR_OPTION_LIMIT: - return None - options = [{"label": str(v), "description": f"Select '{v}'"} for v in values] + question_text = f"{question_text} Choose Other to type a value not listed below." + return { + "question": question_text, + "header": header, + "options": [{"label": str(v), "description": f"Select '{v}'"} for v in values[:_SELECTOR_OPTION_LIMIT]], + "multiSelect": False, + } + hint = question_dict.get("hint") + details = f" {hint}." if hint and not hint.endswith(".") else (f" {hint}" if hint else "") return { - "question": question_text, + "question": f"{question_text}{details} Type your answer under Other.", "header": header, - "options": options, + "options": [ + {"label": SKIP_OPTION, "description": "Leave it unanswered; the controls it affects stay non-compliant"}, + {"label": CHAT_OPTION, "description": "Type a longer answer in the conversation instead"}, + ], "multiSelect": False, } diff --git a/packages/darnit/src/darnit/server/procedures.py b/packages/darnit/src/darnit/server/procedures.py index 01438643f..e637b4a99 100644 --- a/packages/darnit/src/darnit/server/procedures.py +++ b/packages/darnit/src/darnit/server/procedures.py @@ -63,10 +63,12 @@ def data_collection_procedure( return [ "1. For each question with a candidate, show the person candidate.value and candidate.origin, " "labelled UNCONFIRMED. A candidate is never a fact or a default answer.", - "2. Ask every question. Pass ask_user_batch verbatim to your question tool (AskUserQuestion in " - "Claude Code); ask a question with no ask_user entry in plain text, listing every allowed_values " - "entry. format_hint is a format, never an answer. Do not pre-fill or suggest answers.", - "3. If the person says skip or does not know, leave that key out and move on.", + "2. Ask every question at once: pass ask_user_batch verbatim to your question tool (AskUserQuestion " + "in Claude Code). A typed answer under Other is the person's answer. If a client has no question " + "tool, ask in plain text, listing every allowed_values entry. format_hint is a format, never an " + "answer. Do not pre-fill or suggest answers.", + '3. "Skip for now", or a person who says skip or does not know, leaves that key out. For ' + '"Answer in chat", ask that one question in plain text.', f"4. Record the answers in one call: confirm_project_data({confirm_args}, =), using answer_mapping: Yes/No on a yes/no question is true/false; a chosen option is its " 'label as a string; Yes on a candidate is accept_candidates={"": " None: assert ask["options"][1]["description"] == "No, this does not apply" assert ask["multiSelect"] is False - def test_enum_question_with_more_than_four_values_has_no_selector(self) -> None: - """Replaces "max 4 options shown": every allowed value must be reachable (feature 042, FR-015).""" + def test_enum_question_with_more_than_four_values_names_every_value(self) -> None: + """The selector shows 4 values and its question names them all, each typeable under Other (FR-015).""" values = ["github", "gitlab", "jenkins", "circleci", "azure", "travis"] defn = ContextDefinition( type=ContextType.ENUM, @@ -271,10 +271,12 @@ def test_enum_question_with_more_than_four_values_has_no_selector(self) -> None: ) question = _build_context_question(req) - assert "ask_user" not in question + ask = question["ask_user"] + assert [o["label"] for o in ask["options"]] == values[:4] + assert "Other" in ask["question"] assert question["allowed_values"] == values for value in values: - assert value in question["prompt"] + assert value in ask["question"] def test_confirm_question_has_ask_user_accept_reject(self) -> None: """A detected candidate is asked as Yes/No; the options name its origin, not its value (feature 042, FR-013).""" @@ -316,11 +318,14 @@ def test_free_text_examples_are_a_format_hint_not_options(self) -> None: ) question = _build_context_question(req) - assert "ask_user" not in question + ask = question["ask_user"] + assert [o["label"] for o in ask["options"]] == ["Skip for now", "Answer in chat"] + assert "security@example.com" not in json.dumps(ask) + assert "See SECURITY.md" not in json.dumps(ask) assert question["format_hint"] == "security@example.com or See SECURITY.md" - def test_free_text_without_examples_has_no_ask_user(self) -> None: - """Free text question without examples has no ask_user field.""" + def test_free_text_offers_skip_and_chat_and_is_typed_under_other(self) -> None: + """A free-text question is in the batch; its answer is typed under Other, never chosen.""" defn = ContextDefinition( type=ContextType.STRING, prompt="What is the project name?", @@ -334,7 +339,9 @@ def test_free_text_without_examples_has_no_ask_user(self) -> None: ) question = _build_context_question(req) - assert "ask_user" not in question + ask = question["ask_user"] + assert ask["question"] == "What is the project name? Type your answer under Other." + assert [o["label"] for o in ask["options"]] == ["Skip for now", "Answer in chat"] def test_header_truncated_at_word_boundary(self) -> None: """Header keeps complete words instead of cutting through one.""" @@ -515,10 +522,9 @@ def test_confirm_answer_mapping_accepts_by_digest_placeholder(self, mock_get, mo @patch("darnit_baseline.tools.Path") @patch("darnit.config.context_storage.get_pending_context") - def test_no_batch_when_no_ask_user(self, mock_get, mock_path) -> None: - """No ask_user_batch field when questions lack ask_user params.""" + def test_free_text_question_is_in_the_batch(self, mock_get, mock_path) -> None: + """A free-text question is asked in the same round as the others.""" mock_path.return_value.resolve.return_value = "/tmp/repo" - # Free text without examples → no ask_user defn = ContextDefinition( type=ContextType.STRING, prompt="Project name?", @@ -534,8 +540,14 @@ def test_no_batch_when_no_ask_user(self, mock_get, mock_path) -> None: result = get_pending_data(local_path="/tmp/repo") data = _parse_json_from_result(result) - assert "ask_user_batch" not in data - assert "answer_mapping" not in data + assert len(data["ask_user_batch"]) == 1 + assert data["answer_mapping"] == [ + { + "question_index": 0, + "context_key": "project_name", + "value_map": {"Skip for now": "SKIP", "Answer in chat": "ASK_USER_FOR_VALUE"}, + } + ] class TestTomlDefinitionOrder: diff --git a/tests/darnit_baseline/test_pending_payload.py b/tests/darnit_baseline/test_pending_payload.py index 5e014f013..01ea4a265 100644 --- a/tests/darnit_baseline/test_pending_payload.py +++ b/tests/darnit_baseline/test_pending_payload.py @@ -155,12 +155,13 @@ def test_allowed_values_lists_the_whole_vocabulary(self, key: str) -> None: assert question["allowed_values"] == _definitions()[key].values @pytest.mark.parametrize("key", ["governance_model", "ci_provider"]) - def test_selector_omitted_when_more_than_four_values(self, key: str) -> None: + def test_selector_names_every_value_when_more_than_four(self, key: str) -> None: question = _build_context_question(_request(key)) + values = _definitions()[key].values - assert "ask_user" not in question - for value in _definitions()[key].values: - assert value in question["prompt"] + assert [o["label"] for o in question["ask_user"]["options"]] == values[:4] + for value in values: + assert value in question["ask_user"]["question"] def test_selector_lists_every_value_when_four_or_fewer(self) -> None: question = _build_context_question(_request("platform")) @@ -168,11 +169,13 @@ def test_selector_lists_every_value_when_four_or_fewer(self) -> None: labels = [option["label"] for option in question["ask_user"]["options"]] assert labels == _definitions()["platform"].values - def test_questions_without_selector_are_still_returned(self) -> None: - payload = _payload([_request("governance_model"), _request("has_releases")]) + def test_every_question_is_asked_in_one_batch(self) -> None: + payload = _payload([_request("governance_model"), _request("security_contact"), _request("has_releases")]) - assert [q["key"] for q in payload["questions"]] == ["governance_model", "has_releases"] - assert [m["context_key"] for m in payload["answer_mapping"]] == ["has_releases"] + keys = [q["key"] for q in payload["questions"]] + assert sorted(keys) == ["governance_model", "has_releases", "security_contact"] + assert [m["context_key"] for m in payload["answer_mapping"]] == keys + assert len(payload["ask_user_batch"]) == 3 def test_non_enum_has_no_allowed_values(self) -> None: assert _build_context_question(_request("maintainers"))["allowed_values"] is None @@ -186,8 +189,8 @@ def test_examples_are_a_format_hint_and_not_options(self, key: str) -> None: question = _build_context_question(_request(key)) assert question["format_hint"] == " or ".join(examples) - assert "ask_user" not in question for example in examples: + assert example not in json.dumps(question["ask_user"]["options"]) assert example not in question["command_template"] def test_examples_are_not_offered_in_the_payload(self) -> None: @@ -198,7 +201,9 @@ def test_examples_are_not_offered_in_the_payload(self) -> None: templates = json.dumps([q["command_template"] for q in payload["questions"]]) for request in pending: for example in request.definition.examples: - assert f'"{example}"' not in offered + # An enum's examples are allowed values, so they may be offered as values. + if request.definition.type != "enum": + assert f'"{example}"' not in offered assert example not in templates From c858af911e681a8d1a1f15db6d00e1f11dd386af Mon Sep 17 00:00:00 2001 From: Michael Lieberman Date: Wed, 21 Oct 2026 11:47:30 -0400 Subject: [PATCH 07/10] docs(skills): confirm the repository once and name it on every call The audit and comply skills called the audit without owner and repo, so confirmed project data and stored PASS candidates, which apply only to a named repository, were never applied: after answering the data questions a re-audit showed the same results, and submit_judgment (which requires the name) saw a different set of PENDING controls than the audit. Every skill now asks the person to confirm the repository, suggesting the origin remote, and names it on every call; comply re-runs the audit after data is confirmed. A test checks every skill. Refs #646, #655. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Michael Lieberman --- CHANGELOG.md | 19 +++++++++++++++++++ .../src/darnit/skills/darnit-audit/SKILL.md | 8 ++++++-- .../src/darnit/skills/darnit-comply/SKILL.md | 10 ++++++++-- .../src/darnit/skills/darnit-data/SKILL.md | 6 +++++- .../darnit/skills/darnit-remediate/SKILL.md | 6 +++++- tests/darnit/server/test_procedures.py | 12 ++++++++++++ 6 files changed, 55 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ed1236be..7f8e90fca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -442,6 +442,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 hard-coded `0.1.0`, and `baseline.version` / `baseline.specification` derive from the framework TOML metadata (`OSPS v2026.02.19`) instead of `2025.10.10` (Refs #494). +- `get_pending_data` gives every question a selector, so a batch of up to 4 + questions is asked in one round of the client's question tool (#650, #655). + After 0.1.1, free-text questions and choice questions with more than 4 + values had none, so Claude Code switched between the question tool and + plain text within a batch. (In 0.1.1 a free-text question with two or more + examples offered the examples as answers, and a choice question showed + only its first 4 values.) A choice question now offers its first 4 values + and names every allowed value in the question, any of which can be typed + under "Other"; a free-text question offers "Skip for now" and "Answer in + chat" and never a value. Selector headers are cut at a word boundary + ("Compiled", not "Compiled Ass"), and a yes/no question's hint is in the + question instead of next to "Yes". +- The skills ask the person to confirm which repository is being audited + and name it on every darnit call, the audit included. An audit called + without `owner` and `repo` does not apply the person's confirmed answers or + stored PASS candidates, so after answering the data questions a re-audit + showed the same results. `darnit-comply` re-runs the audit after data is + confirmed and shows what changed. + - `get_pending_data` and `get_judgment_requests` return a `procedure`: the steps a coding agent follows with the result (how to show candidates and ask the questions, the exact `confirm_project_data` or `submit_judgment` diff --git a/packages/darnit/src/darnit/skills/darnit-audit/SKILL.md b/packages/darnit/src/darnit/skills/darnit-audit/SKILL.md index 3001dac11..785ec8330 100644 --- a/packages/darnit/src/darnit/skills/darnit-audit/SKILL.md +++ b/packages/darnit/src/darnit/skills/darnit-audit/SKILL.md @@ -4,7 +4,7 @@ description: Run a compliance audit on the current repository using darnit. Use compatibility: Requires darnit MCP server running (darnit serve) metadata: author: kusari-oss - version: "2.1" + version: "2.2" --- # Compliance Audit @@ -22,9 +22,13 @@ Darnit's MCP server registers audit tools per implementation module. Look for av Common audit tools: - `audit_openssf_baseline` — OpenSSF Baseline (OSPS v2026.02.19) +## The repository + +Name the audited repository on every darnit call, the audit included: `owner` and `repo`, plus `host` when it is not github.com. darnit applies the person's confirmed answers, stored PASS candidates, and trust only to a named repository, and never takes the name from the checkout's remotes. Before the first call, ask the person to confirm which repository this is, offering the checkout's `origin` remote as the suggestion, and use their answer for every call. + ## Workflow -1. Identify the audit tool to use (see above). Call it with `local_path` set to the repository root and `output_format` set to `"markdown"`. +1. Identify the audit tool to use (see above). Call it with `owner` and `repo` naming the repository, `local_path` set to the repository root, and `output_format` set to `"markdown"`. - Use `output_format: "summary"` if you only need pass/fail counts (compact JSON; PENDING rows carry `pending.kind`). Do not use `output_format: "json"` to read judgment requests: it holds every control's full evidence and is too large for a tool result. Use `get_judgment_requests` (step 2). - If the user mentions a profile (e.g., "just level 1", "access control only", "onboard"), pass it as the `profile` parameter. - If the user mentions a level, pass it as the `level` parameter. diff --git a/packages/darnit/src/darnit/skills/darnit-comply/SKILL.md b/packages/darnit/src/darnit/skills/darnit-comply/SKILL.md index 928758437..911e7585a 100644 --- a/packages/darnit/src/darnit/skills/darnit-comply/SKILL.md +++ b/packages/darnit/src/darnit/skills/darnit-comply/SKILL.md @@ -4,7 +4,7 @@ description: Run the full compliance pipeline — audit, collect data, remediate compatibility: Requires darnit MCP server running (darnit serve) and gh CLI for PR creation metadata: author: kusari-oss - version: "2.2" + version: "2.3" --- # Full Compliance Pipeline @@ -19,11 +19,15 @@ Register darnit's MCP server at user scope (`darnit install` does this by defaul Darnit registers tools per implementation module. Look for available tools matching `audit_*` for the audit step, and `remediate_audit_findings` for remediation. If the user specifies a framework, use those tools. If only one set is available, use it. If multiple exist, ask. +## The repository + +Name the audited repository on every darnit call, the audit included: `owner` and `repo`, plus `host` when it is not github.com. darnit applies the person's confirmed answers, stored PASS candidates, and trust only to a named repository, and never takes the name from the checkout's remotes. Before the first call, ask the person to confirm which repository this is, offering the checkout's `origin` remote as the suggestion, and use their answer for every call. + ## Workflow ### 1. Initial audit -Call the appropriate `audit_*` tool with `output_format: "summary"` and any profile the user mentioned. The "summary" format returns compact JSON; PENDING rows carry `pending.kind`. Do not use `output_format: "json"`: it is too large for a tool result. Present a brief summary: total controls, pass/fail/warn counts, compliance percentage. +Call the appropriate `audit_*` tool with `owner` and `repo` naming the audited repository, `output_format: "summary"`, and any profile the user mentioned. The "summary" format returns compact JSON; PENDING rows carry `pending.kind`. Do not use `output_format: "json"`: it is too large for a tool result. Present a brief summary: total controls, pass/fail/warn counts, compliance percentage. ### 2. Judgments @@ -35,6 +39,8 @@ Tell the user you'll collect project data, then call `get_pending_data` with `ow ### 4. Remediation plan +If data was confirmed in step 3, call the audit again (same arguments) and show what changed: confirmed answers can make controls pass or not applicable. + If there are FAIL controls with auto-fixes: - Call `remediate_audit_findings` with `dry_run: true`, and `owner` and `repo` naming the audited repository - Read the fenced JSON block after the report: its `plan` lists each item's `file_changes`, `change_sets`, `requires_individual_approval`, and `digest` diff --git a/packages/darnit/src/darnit/skills/darnit-data/SKILL.md b/packages/darnit/src/darnit/skills/darnit-data/SKILL.md index 9158efba4..b2786768f 100644 --- a/packages/darnit/src/darnit/skills/darnit-data/SKILL.md +++ b/packages/darnit/src/darnit/skills/darnit-data/SKILL.md @@ -4,13 +4,17 @@ description: Collect missing project data to improve audit accuracy. Use when th compatibility: Requires darnit MCP server running (darnit serve) metadata: author: kusari-oss - version: "2.0" + version: "2.1" --- # Project Data Collection Guide the user through providing missing project data that darnit needs for accurate audits. A value darnit detected is only a candidate: it is shown to the user as unconfirmed data with where it came from, and it counts only after the user confirms it. +## The repository + +Name the audited repository on every darnit call, the audit included: `owner` and `repo`, plus `host` when it is not github.com. darnit applies the person's confirmed answers, stored PASS candidates, and trust only to a named repository, and never takes the name from the checkout's remotes. Before the first call, ask the person to confirm which repository this is, offering the checkout's `origin` remote as the suggestion, and use their answer for every call. + ## Workflow 1. Call `get_pending_data` with `local_path` set to the repository root and `owner` and `repo` naming the repository. If the user mentioned a profile, pass it as `profile`. This call writes nothing. diff --git a/packages/darnit/src/darnit/skills/darnit-remediate/SKILL.md b/packages/darnit/src/darnit/skills/darnit-remediate/SKILL.md index 3c2a9c4d8..cf86d8d92 100644 --- a/packages/darnit/src/darnit/skills/darnit-remediate/SKILL.md +++ b/packages/darnit/src/darnit/skills/darnit-remediate/SKILL.md @@ -4,13 +4,17 @@ description: Apply automated fixes for failing compliance controls. Shows a plan compatibility: Requires darnit MCP server running (darnit serve) metadata: author: kusari-oss - version: "2.2" + version: "2.3" --- # Compliance Remediation Show a dry-run plan of fixes for failing controls, get confirmation, then apply changes on a new branch. After applying, enhance generated template files with project-specific content. +## The repository + +Name the audited repository on every darnit call, the audit included: `owner` and `repo`, plus `host` when it is not github.com. darnit applies the person's confirmed answers, stored PASS candidates, and trust only to a named repository, and never takes the name from the checkout's remotes. Before the first call, ask the person to confirm which repository this is, offering the checkout's `origin` remote as the suggestion, and use their answer for every call. + ## Workflow ### 1. Show dry-run plan diff --git a/tests/darnit/server/test_procedures.py b/tests/darnit/server/test_procedures.py index e84bae0ea..79f10fac2 100644 --- a/tests/darnit/server/test_procedures.py +++ b/tests/darnit/server/test_procedures.py @@ -84,3 +84,15 @@ def test_no_skill_tells_the_agent_to_follow_another_skill() -> None: ] assert offenders == [] + + +@pytest.mark.parametrize("path", sorted(SKILLS.glob("*/SKILL.md")), ids=lambda p: p.parent.name) +def test_every_skill_names_the_repository_on_every_call(path: Path) -> None: + """Confirmed answers, PASS candidates, and trust apply only to a named repository (#655).""" + text = path.read_text(encoding="utf-8") + + assert "## The repository" in text + assert "never takes the name from the checkout's remotes" in text + for line in text.splitlines(): + if re.search(r"Call (it|the appropriate `audit_\*` tool) with", line): + assert "`owner` and `repo`" in line, line From 57329b9ef6f8d274c128b46387d13ae07b15f670 Mon Sep 17 00:00:00 2001 From: Michael Lieberman Date: Wed, 21 Oct 2026 11:47:58 -0400 Subject: [PATCH 08/10] test: the agent flow asks every question through the question tool Refs #655. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Michael Lieberman --- tests/darnit_baseline/test_agent_flow.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/darnit_baseline/test_agent_flow.py b/tests/darnit_baseline/test_agent_flow.py index d4149c9ec..f2422eca9 100644 --- a/tests/darnit_baseline/test_agent_flow.py +++ b/tests/darnit_baseline/test_agent_flow.py @@ -139,6 +139,11 @@ def test_the_questions_end_after_every_answer_is_confirmed(self, flow: dict) -> assert rounds[-1]["status"] == "complete", rounds[-1] assert len(rounds) <= MAX_ROUNDS + def test_every_question_goes_through_the_question_tool(self, flow: dict) -> None: + for pending in flow["rounds"][:-1]: + assert len(pending["ask_user_batch"]) == len(pending["questions"]) + assert len(pending["ask_user_batch"]) <= 4 + def test_answers_for_an_untrusted_repository_stay_out_of_the_checkout(self, flow: dict, tmp_path: Path) -> None: assert not (tmp_path / "repo" / ".project").exists() From c85693f2ac63cdbd4c4885dcc57eca1808a8711f Mon Sep 17 00:00:00 2001 From: Michael Lieberman Date: Thu, 5 Nov 2026 10:06:36 -0500 Subject: [PATCH 09/10] docs(spec): a selector label or answer_mapping marker is not an answer Section 7.10 and the 042 contract: confirm_project_data refuses "Skip for now", "Answer in chat", "SKIP" and "ASK_USER_FOR_VALUE" as a value, alone or inside a list, since an agent may forward the label its question tool returned. The contract no longer says a selector is omitted above 4 values: every question has one, a long choice list offers its first 4 and names them all, and a free-text question offers the two labels above, never a value. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Michael Lieberman --- docs/architecture/framework-design.md | 2 +- .../contracts/context-confirmation-tools.md | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/architecture/framework-design.md b/docs/architecture/framework-design.md index f2e0fd71d..ad56d9581 100644 --- a/docs/architecture/framework-design.md +++ b/docs/architecture/framework-design.md @@ -1760,7 +1760,7 @@ Each context key has one canonical name and one vocabulary, taken from the frame The observable payloads of `get_pending_data` and `confirm_project_data` are defined in `specs/042-candidate-integrity/contracts/context-confirmation-tools.md`: - `get_pending_data` is read-only. Each question carries its candidate as data (`candidate: {value, origin, digest, label}`, where `digest` is the value digest (7.5) and `label` marks it unconfirmed); the candidate's value appears nowhere else in the response. `command_template` holds placeholders only (``, ``, `owner=..., repo=...`), never a candidate value or a configuration example. `answer_mapping.value_map["Yes"]` for a candidate is `{"accept_candidates": {"": ""}}`, a placeholder the agent fills from the question's `candidate.digest` only after the person answered yes. Every question carries a selector (`ask_user`), so one batch is asked in one round of the client's question tool, which offers at most 4 options and adds a free-text "Other". Enum questions list every allowed value in `allowed_values`; when there are more than 4, the selector offers the first 4 and its question names them all, and any of them can be typed under Other (every allowed value stays reachable). A free-text question's selector offers "Skip for now" and "Answer in chat" and never a value; the answer is typed under Other. A yes/no question's options carry neutral descriptions, and its hint is part of the question. A selector header is at most 12 characters, cut at a word boundary. `examples` appear only as `format_hint`, never as an option. Every question in the batch is returned under `questions`. The response lists `stored_unconfirmed` values with their locations. The response also carries `procedure`: the ordered steps for collecting the data (show each candidate with its origin, labelled unconfirmed; ask every question, passing `ask_user_batch` verbatim in one round; per-question skip; one `confirm_project_data` call per batch, mapping each answer by `answer_mapping` (Yes/No on a yes/no question is a boolean; No on a candidate, or Other, is the value the person typed), with `accept_candidates` filled only after a yes; call `get_pending_data` again, with the caller's `control_ids`, until `complete`; review `stored_unconfirmed`; summarize). Its calls name only the `owner` and `repo` the caller passed, never ones detected from the checkout's remote (placeholders otherwise), and hold no candidate or detected value. `get_pending_data` takes no `host`, so its `confirm_project_data` call names `host` as a placeholder to fill when the repository is not on github.com (7.10). A `confirm_project_data` result that handled context values on a server with `get_pending_data` ends with the next `get_pending_data` call, named the same way. -- `confirm_project_data` takes a parameter for every context key that the server's bound framework defines, and no other framework's keys (generated from the definitions; an enum parameter accepts only its `values`), plus `accept_candidates` (`{key: candidate digest}`: detection runs again and the current candidate, or a value concluded in this run, is confirmed only if its digest still matches, recording its value and origin as the basis; a mismatch, a key with no candidate, or a key given both an answer and a digest is refused and nothing is written for it), `confirm_stored` and `reject_stored` (review of stored values: a rejected `.project/darnit.yaml` value is deleted; a rejected `.project/project.yaml` value is reported with its file and field and the file is left unchanged), `expires_at` (`{key: date}`), and `owner`, `repo`, `host`, which are required and decide the record location (7.5). `confirm_stored` records a confirmation of the value currently stored for each key (basis: that value, origin `stored_unconfirmed` or `expired_confirmation`). Like a confirmation, a rejection writes to `.project/darnit.yaml` only when the operator trusts the named repository; otherwise it is refused and names the file and field for the person to edit. The result names, per key, `confirmed (in-repository)` with the file written or `confirmed (operator-side)`, `rejected`, `edit required: :`, or `refused: `. +- `confirm_project_data` takes a parameter for every context key that the server's bound framework defines, and no other framework's keys (generated from the definitions; an enum parameter accepts only its `values`), plus `accept_candidates` (`{key: candidate digest}`: detection runs again and the current candidate, or a value concluded in this run, is confirmed only if its digest still matches, recording its value and origin as the basis; a mismatch, a key with no candidate, or a key given both an answer and a digest is refused and nothing is written for it), `confirm_stored` and `reject_stored` (review of stored values: a rejected `.project/darnit.yaml` value is deleted; a rejected `.project/project.yaml` value is reported with its file and field and the file is left unchanged), `expires_at` (`{key: date}`), and `owner`, `repo`, `host`, which are required and decide the record location (7.5). `confirm_stored` records a confirmation of the value currently stored for each key (basis: that value, origin `stored_unconfirmed` or `expired_confirmation`). Like a confirmation, a rejection writes to `.project/darnit.yaml` only when the operator trusts the named repository; otherwise it is refused and names the file and field for the person to edit. A value that is one of the free-text selector's labels ("Skip for now", "Answer in chat") or an `answer_mapping` marker (`"SKIP"`, `"ASK_USER_FOR_VALUE"`), alone or inside a list, is not an answer and is refused, since an agent may forward the label its question tool returned. The result names, per key, `confirmed (in-repository)` with the file written or `confirmed (operator-side)`, `rejected`, `edit required: :`, or `refused: `. - **Which server exposes it**: each server's `confirm_project_data` covers only the keys of the framework the server is bound to. The OpenSSF Baseline server defines its own (which also carries `confirm_not_applicable` and `confirm_pass_candidate`). Every other framework server that defines context keys and whose TOML does not define a tool named `confirm_project_data` gets a framework-neutral builtin `confirm_project_data` (a framework with no context keys gets none), registered by the factory the same way as `submit_judgment` and `confirm_pass_candidate` (5.4) and bound to that framework's context definitions. It records context values only: the per-key parameters, `accept_candidates`, `confirm_stored`, `reject_stored`, `expires_at`, `owner`, `repo`, `host`, and `local_path`. Claim confirmations and PASS candidates stay on their existing tools. - Remediation prompts (the preflight and `format_context_prompt`) show a candidate as labelled, unconfirmed data with its origin and digest, name the key's hint source files that exist without parsing values out of them, and hold placeholders only in their commands. diff --git a/specs/042-candidate-integrity/contracts/context-confirmation-tools.md b/specs/042-candidate-integrity/contracts/context-confirmation-tools.md index a7b6d44a2..4e882c75d 100644 --- a/specs/042-candidate-integrity/contracts/context-confirmation-tools.md +++ b/specs/042-candidate-integrity/contracts/context-confirmation-tools.md @@ -24,7 +24,8 @@ Writes nothing. Each question: ``` - `command_template` never contains a candidate value or a configuration example. -- Enum questions list every allowed value in `allowed_values`; the selector options are omitted when there are more than 4. +- Every question carries a selector (`ask_user`); the client's question tool shows at most 4 options and adds a free-text "Other". Enum questions list every allowed value in `allowed_values`; with more than 4, the selector offers the first 4 and its question names them all, and any of them can be typed under Other. +- A free-text question's selector offers "Skip for now" and "Answer in chat", never a value; the answer is typed under Other. Its `answer_mapping.value_map` maps them to the markers `"SKIP"` and `"ASK_USER_FOR_VALUE"`, and a candidate question's maps "No" to `"ASK_USER_FOR_VALUE"`. These four strings are not answers: `confirm_project_data` refuses any of them as a value, alone or inside a list (`darnit.server.procedures.NOT_AN_ANSWER`). - The response carries `procedure`, the ordered steps for collecting the data (framework-design 7.10, `darnit.server.procedures`). Its calls name only the `owner`, `repo`, and `control_ids` the caller passed; the `confirm_project_data` call names `host` as a placeholder, since `get_pending_data` takes no `host`. It holds no candidate or detected value. - `answer_mapping.value_map["Yes"]` is `{"accept_candidates": {"": ""}}`: a placeholder the agent fills from the question's `candidate.digest` only after the person has actually answered yes. The mapping never contains a ready-to-send digest, and it applies only to the person's own answer; accepting a candidate by digest is a confirmation and follows the same rule as any other. From c77cb47b1c9751fcdec8c774cf4f4ea37d65aa9b Mon Sep 17 00:00:00 2001 From: Michael Lieberman Date: Thu, 5 Nov 2026 10:06:36 -0500 Subject: [PATCH 10/10] fix(data): refuse a selector label or marker as a confirmed value Review of #672. Every free-text question now offers "Skip for now" and "Answer in chat", and answer_mapping maps them to "SKIP" and "ASK_USER_FOR_VALUE". record_value_confirmation refused only None and out-of-vocabulary enum values, so an agent that forwarded the label AskUserQuestion returned recorded it as a person-confirmed value, for example maintainers = ["Skip for now"]. Only the procedure text guarded against that. record_value_confirmation now refuses those four strings, alone or in a list, before and after coercion, with a reason that says to leave the key out or ask for the value. They are defined once, in darnit.server.procedures, and the selector, answer_mapping and the procedure use them. The agent-flow test forwards a "Skip for now" label and requires the refusal. darnit-comply said both to re-run the audit after confirming data and "Do NOT run a separate audit"; the gotcha now says only not to audit just to feed remediate_audit_findings. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Michael Lieberman --- CHANGELOG.md | 5 ++- .../src/darnit_baseline/tools.py | 18 +++++++---- .../src/darnit/config/context_writes.py | 32 +++++++++++++++++++ .../darnit/src/darnit/server/procedures.py | 13 ++++++-- .../src/darnit/skills/darnit-comply/SKILL.md | 2 +- tests/darnit/config/test_context_writes.py | 30 +++++++++++++++++ tests/darnit/server/test_procedures.py | 11 +++++++ tests/darnit_baseline/test_agent_flow.py | 23 +++++++++++++ 8 files changed, 123 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f8e90fca..fd9306652 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -451,7 +451,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 only its first 4 values.) A choice question now offers its first 4 values and names every allowed value in the question, any of which can be typed under "Other"; a free-text question offers "Skip for now" and "Answer in - chat" and never a value. Selector headers are cut at a word boundary + chat" and never a value. `confirm_project_data` refuses those two labels, + and the `answer_mapping` markers `SKIP` and `ASK_USER_FOR_VALUE`, as a + value, so an agent that forwards the label its question tool returned + records nothing for that key. Selector headers are cut at a word boundary ("Compiled", not "Compiled Ass"), and a yes/no question's hint is in the question instead of next to "Yes". - The skills ask the person to confirm which repository is being audited diff --git a/packages/darnit-baseline/src/darnit_baseline/tools.py b/packages/darnit-baseline/src/darnit_baseline/tools.py index 6b81090c0..b84ed2f86 100644 --- a/packages/darnit-baseline/src/darnit_baseline/tools.py +++ b/packages/darnit-baseline/src/darnit_baseline/tools.py @@ -795,7 +795,13 @@ def get_pending_data( with their locations, for review with confirm_stored / reject_stored. """ from darnit.config.context_storage import get_pending_context as _get_pending - from darnit.server.procedures import data_collection_procedure + from darnit.server.procedures import ( + ASK_FOR_VALUE_MARKER, + CHAT_OPTION, + SKIP_MARKER, + SKIP_OPTION, + data_collection_procedure, + ) from darnit.server.tools.project_data import stored_unconfirmed from darnit.trust.decision import target_from_owner_repo @@ -864,12 +870,12 @@ def get_pending_data( if q["candidate"] is not None: mapping["value_map"] = { "Yes": {"accept_candidates": {q["key"]: ""}}, - "No": "ASK_USER_FOR_VALUE", + "No": ASK_FOR_VALUE_MARKER, } elif q["input_type"] == "select" and q.get("options") == ["true", "false"]: mapping["value_map"] = {"Yes": True, "No": False} elif q["input_type"] == "free_text": - mapping["value_map"] = {SKIP_OPTION: "SKIP", CHAT_OPTION: "ASK_USER_FOR_VALUE"} + mapping["value_map"] = {SKIP_OPTION: SKIP_MARKER, CHAT_OPTION: ASK_FOR_VALUE_MARKER} answer_mapping.append(mapping) # Determine answered count from total minus pending @@ -997,10 +1003,6 @@ def _build_context_question(req) -> dict: return question -SKIP_OPTION = "Skip for now" -CHAT_OPTION = "Answer in chat" - - def _build_ask_user_params(key: str, question_dict: dict, definition) -> dict: """AskUserQuestion parameters (question/header/options/multiSelect) for every question. @@ -1010,6 +1012,8 @@ def _build_ask_user_params(key: str, question_dict: dict, definition) -> dict: chat options, never a value. A candidate's value is not repeated here; the person sees it from the question's ``candidate``. """ + from darnit.server.procedures import CHAT_OPTION, SKIP_OPTION + input_type = question_dict.get("input_type") question_text = question_dict["prompt"] header = _ask_user_header(key) diff --git a/packages/darnit/src/darnit/config/context_writes.py b/packages/darnit/src/darnit/config/context_writes.py index 32fab4057..3ac85f0ae 100644 --- a/packages/darnit/src/darnit/config/context_writes.py +++ b/packages/darnit/src/darnit/config/context_writes.py @@ -147,6 +147,34 @@ def _repository_record( return record +def _not_an_answer(value: Any) -> str | None: + """The question option or answer_mapping marker ``value`` is (or holds), if any. + + The free-text selector offers "Skip for now" and "Answer in chat", and + answer_mapping maps them to "SKIP" and "ASK_USER_FOR_VALUE". None of them + is an answer, and an agent may forward the label the question tool + returned, so they are refused here rather than trusted to the procedure. + """ + from darnit.server.procedures import NOT_AN_ANSWER + + items = value if isinstance(value, (list, tuple, set)) else [value] + for item in items: + if isinstance(item, str) and item.strip() in NOT_AN_ANSWER: + return item.strip() + return None + + +def _refuse_not_an_answer(key: str, marker: str) -> WriteResult: + return WriteResult( + key, + "refused", + reason=( + f"{marker!r} is a question option or answer_mapping marker, not an answer; " + "leave the key out to skip it, or ask the person for the value" + ), + ) + + def record_value_confirmation( local_path: str, key: str, @@ -182,12 +210,16 @@ def record_value_confirmation( definition = _definitions(local_path, definitions).get(key) if definition is None: return WriteResult(key, "refused", reason=f"{key!r} is not a context key the framework defines") + if (marker := _not_an_answer(value)) is not None: + return _refuse_not_an_answer(key, marker) try: value = normalize_value(key, coerce_value(definition, value)) except ValueError as exc: return WriteResult(key, "refused", reason=str(exc)) if value is None: return WriteResult(key, "refused", reason="no value given") + if (marker := _not_an_answer(value)) is not None: + return _refuse_not_an_answer(key, marker) if not in_vocabulary(value, definition): return WriteResult( key, "refused", reason=f"{value!r} is not an allowed value; allowed: {', '.join(vocabulary(definition))}" diff --git a/packages/darnit/src/darnit/server/procedures.py b/packages/darnit/src/darnit/server/procedures.py index e637b4a99..16327e8d4 100644 --- a/packages/darnit/src/darnit/server/procedures.py +++ b/packages/darnit/src/darnit/server/procedures.py @@ -14,6 +14,15 @@ _OWNER = "" _REPO = "" +# The free-text selector's options and answer_mapping's markers stand for +# "no answer". record_value_confirmation refuses them as a value, since an +# agent may forward the label the question tool returns. +SKIP_OPTION = "Skip for now" +CHAT_OPTION = "Answer in chat" +SKIP_MARKER = "SKIP" +ASK_FOR_VALUE_MARKER = "ASK_USER_FOR_VALUE" +NOT_AN_ANSWER = frozenset({SKIP_OPTION, CHAT_OPTION, SKIP_MARKER, ASK_FOR_VALUE_MARKER}) + # get_pending_data takes no host, so the confirm_project_data call it # describes carries a placeholder, never a host read from the checkout. _HOST = "" @@ -67,8 +76,8 @@ def data_collection_procedure( "in Claude Code). A typed answer under Other is the person's answer. If a client has no question " "tool, ask in plain text, listing every allowed_values entry. format_hint is a format, never an " "answer. Do not pre-fill or suggest answers.", - '3. "Skip for now", or a person who says skip or does not know, leaves that key out. For ' - '"Answer in chat", ask that one question in plain text.', + f'3. "{SKIP_OPTION}", or a person who says skip or does not know, leaves that key out. For ' + f'"{CHAT_OPTION}", ask that one question in plain text. Neither label is a value.', f"4. Record the answers in one call: confirm_project_data({confirm_args}, =), using answer_mapping: Yes/No on a yes/no question is true/false; a chosen option is its " 'label as a string; Yes on a candidate is accept_candidates={"": " None: assert result.outcome == "refused" assert "github" in result.reason + @pytest.mark.parametrize("marker", ["Skip for now", "Answer in chat", "SKIP", "ASK_USER_FOR_VALUE"]) + @pytest.mark.parametrize( + ("key", "shape"), + [("security_contact", "string"), ("maintainers", "string"), ("maintainers", "list")], + ) + def test_a_question_option_or_marker_is_not_an_answer(self, repo: Path, marker: str, key: str, shape: str) -> None: + """An agent forwarding the label the question tool returned records nothing (#672 review).""" + value: Any = ["@alice", marker] if shape == "list" else marker + + result = _confirm(repo, key, value) + + assert result.outcome == "refused" + assert marker in (result.reason or "") + assert "not an answer" in (result.reason or "") + assert not (repo / ".project" / "darnit.yaml").exists() + + def test_refused_operator_side_too(self, repo: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path / "data")) + + result = _confirm(repo, "security_contact", "Skip for now", operator=_operator(trusted=False)) + + assert result.outcome == "refused" + assert not (tmp_path / "data").exists() + assert not (repo / ".project" / "darnit.yaml").exists() + + def test_a_value_that_merely_contains_a_marker_is_an_answer(self, repo: Path) -> None: + result = _confirm(repo, "security_contact", "SKIP the form, email security@example.org") + + assert result.outcome == "confirmed" + def test_untrusted_without_a_named_repository(self, repo: Path) -> None: result = _confirm(repo, "maintainers", ["@alice"], target=None, operator=_operator(trusted=False)) diff --git a/tests/darnit/server/test_procedures.py b/tests/darnit/server/test_procedures.py index 79f10fac2..01563cdc1 100644 --- a/tests/darnit/server/test_procedures.py +++ b/tests/darnit/server/test_procedures.py @@ -73,6 +73,17 @@ def test_procedures_are_short_ascii_steps(self) -> None: assert text.isascii() +@pytest.mark.unit +def test_the_free_text_options_and_markers_are_the_shared_ones() -> None: + """The selector and answer_mapping use the strings record_value_confirmation refuses.""" + from darnit.server.procedures import ASK_FOR_VALUE_MARKER, CHAT_OPTION, NOT_AN_ANSWER, SKIP_MARKER, SKIP_OPTION + + assert {SKIP_OPTION, CHAT_OPTION, SKIP_MARKER, ASK_FOR_VALUE_MARKER} == NOT_AN_ANSWER + step3 = data_collection_procedure("acme", "widget", "/w")[2] + assert SKIP_OPTION in step3 + assert CHAT_OPTION in step3 + + @pytest.mark.unit def test_no_skill_tells_the_agent_to_follow_another_skill() -> None: """A skill defers to the procedure in a tool result, never to another skill's text (which a client may not load).""" diff --git a/tests/darnit_baseline/test_agent_flow.py b/tests/darnit_baseline/test_agent_flow.py index f2422eca9..2b95dce26 100644 --- a/tests/darnit_baseline/test_agent_flow.py +++ b/tests/darnit_baseline/test_agent_flow.py @@ -17,6 +17,8 @@ import pytest +from darnit.server.procedures import CHAT_OPTION, SKIP_OPTION + OWNER, REPO = "acme", "widget" JUDGED = "OSPS-DO-01.01" EXCERPT = "Widget converts Markdown documents to HTML from the command line." @@ -80,6 +82,10 @@ def _json(result) -> dict: return json.loads(text[text.index("{") :]) +def _labels(question: dict) -> list[str]: + return [option["label"] for option in question["ask_user"]["options"]] + + async def _flow(checkout: Path, operator_config: Path) -> dict: from mcp.shared.memory import create_connected_server_and_client_session @@ -99,6 +105,16 @@ async def _flow(checkout: Path, operator_config: Path) -> dict: seen["rounds"].append(pending) if pending["status"] != "pending": break + if "forwarded_label" not in seen: + # An agent that forwards the label its question tool returned. + free_text = next( + (q["key"] for q in pending["questions"] if _labels(q) == [SKIP_OPTION, CHAT_OPTION]), None + ) + seen["forwarded_label"] = ( + free_text, + free_text + and _text(await session.call_tool("confirm_project_data", {**named, free_text: SKIP_OPTION})), + ) answers = {q["key"]: ANSWERS[q["key"]] for q in pending["questions"]} seen.setdefault("confirmations", []).append( _text(await session.call_tool("confirm_project_data", {**named, **answers})) @@ -144,6 +160,13 @@ def test_every_question_goes_through_the_question_tool(self, flow: dict) -> None assert len(pending["ask_user_batch"]) == len(pending["questions"]) assert len(pending["ask_user_batch"]) <= 4 + def test_a_forwarded_option_label_is_refused(self, flow: dict) -> None: + """A selector label is not an answer even when the agent sends it as one (#672 review).""" + key, result = flow["forwarded_label"] + assert key is not None, "no free-text question in the first batch" + assert f"{key}: refused" in result, result + assert "not an answer" in result + def test_answers_for_an_untrusted_repository_stay_out_of_the_checkout(self, flow: dict, tmp_path: Path) -> None: assert not (tmp_path / "repo" / ".project").exists()