diff --git a/CHANGELOG.md b/CHANGELOG.md index 80a1be814..fd9306652 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -442,6 +442,46 @@ 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. `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 + 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` + 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`, 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/docs/architecture/framework-design.md b/docs/architecture/framework-design.md index 80f938854..ad56d9581 100644 --- a/docs/architecture/framework-design.md +++ b/docs/architecture/framework-design.md @@ -1405,7 +1405,9 @@ 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. 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,8 +1759,8 @@ 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. -- `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: `. +- `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. 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/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..b84ed2f86 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,27 @@ 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 ( + 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 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), control_ids) # Auto-detect owner/repo from git if owner is None or repo is None: @@ -842,6 +834,7 @@ def get_pending_data( complete: dict = { "status": "complete", "message": "All context has been confirmed. No additional input needed.", + "procedure": procedure, "questions": [], } if stored: @@ -869,10 +862,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"], @@ -880,10 +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_MARKER, CHAT_OPTION: ASK_FOR_VALUE_MARKER} answer_mapping.append(mapping) # Determine answered count from total minus pending @@ -891,6 +883,7 @@ def get_pending_data( response: dict = { "status": "pending", + "procedure": procedure, "progress": { "answered": answered, "total": total, @@ -1005,23 +998,24 @@ 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. +def _build_ask_user_params(key: str, question_dict: dict, definition) -> dict: + """AskUserQuestion parameters (question/header/options/multiSelect) for every question. - 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``. + 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``. """ + 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) if input_type == "confirm": @@ -1038,26 +1032,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/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 new file mode 100644 index 000000000..16327e8d4 --- /dev/null +++ b/packages/darnit/src/darnit/server/procedures.py @@ -0,0 +1,115 @@ +"""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 = "" +# 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 = "" + + +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)}), 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, + control_ids: list[str] | None = None, +) -> list[str]: + """Ordered steps for collecting project data from the person (``get_pending_data`` takes no host).""" + 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.", + "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.", + 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={"": ""}, 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 " + "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.", + "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 74c920076..108196373 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 @@ -201,8 +202,27 @@ 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", "files": {}, "requests": [], "remaining": []} + page: dict[str, Any] = { + "status": "ok", + "procedure": judgment_procedure(owner, repo, host, str(resolved_path)), + "files": {}, + "requests": [], + "remaining": [], + "skipped": skipped, + } oversized = False for index, result in enumerate(pending): files = dict(page["files"]) @@ -250,11 +270,13 @@ 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 - ``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/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..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.0" + version: "2.2" --- # Compliance Audit @@ -22,14 +22,18 @@ 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. -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..55e49e1aa 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.3" --- # Full Compliance Pipeline @@ -19,32 +19,40 @@ 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. 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 `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 + +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. Collect data -### 2. Collect data (if needed) +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. -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" +### 4. Remediation plan -### 3. 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` +- 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 +63,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 +72,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. @@ -74,7 +82,7 @@ Show before/after compliance comparison, list of changes made, and remaining man - Always show the remediation plan and get confirmation before applying changes. Never auto-apply. - Use `output_format: "summary"` for audits to keep token usage low. -- Do NOT run a separate audit before calling `remediate_audit_findings` — it handles audit internally. +- Do NOT run an audit just to feed `remediate_audit_findings`: it audits internally. The one audit after collecting data (step 4) is for showing the person what changed. - Do NOT call `create_remediation_branch` or `commit_remediation_changes` separately — use the built-in `branch_name` and `auto_commit` params. - Unsafe remediations, steps that cannot be previewed exactly, and high-impact platform changes run only when the person approved their own digest; a batch approval never covers them. - Platform changes follow the operator's `[remediation]` policy (`prompt`, `manual`, or `auto`), stated in the report. Never try to change it from the repository. diff --git a/packages/darnit/src/darnit/skills/darnit-data/SKILL.md b/packages/darnit/src/darnit/skills/darnit-data/SKILL.md index fca07e2f4..b2786768f 100644 --- a/packages/darnit/src/darnit/skills/darnit-data/SKILL.md +++ b/packages/darnit/src/darnit/skills/darnit-data/SKILL.md @@ -4,57 +4,38 @@ 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.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. -## 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. +## The repository -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. +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. -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. +## Workflow -4. Call `get_pending_data` again. If `status` is `"complete"`, move to step 5. Otherwise repeat from step 2. +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. -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. +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. -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..cf86d8d92 100644 --- a/packages/darnit/src/darnit/skills/darnit-remediate/SKILL.md +++ b/packages/darnit/src/darnit/skills/darnit-remediate/SKILL.md @@ -4,18 +4,22 @@ 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.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 -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 +36,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 +102,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/specs/042-candidate-integrity/contracts/context-confirmation-tools.md b/specs/042-candidate-integrity/contracts/context-confirmation-tools.md index f87fe816e..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,9 @@ 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. Response also includes: diff --git a/tests/darnit/config/test_context_writes.py b/tests/darnit/config/test_context_writes.py index 68a9a55cf..c6b885867 100644 --- a/tests/darnit/config/test_context_writes.py +++ b/tests/darnit/config/test_context_writes.py @@ -253,6 +253,36 @@ def test_value_outside_the_vocabulary(self, repo: Path) -> 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_judgment_requests.py b/tests/darnit/server/test_judgment_requests.py index 4cbc74f18..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" @@ -228,3 +231,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..01563cdc1 --- /dev/null +++ b/tests/darnit/server/test_procedures.py @@ -0,0 +1,109 @@ +"""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", 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: + 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") + 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")) + + 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, ensure_ascii=False) + assert len(text) < 2_000 + 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).""" + 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 == [] + + +@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 diff --git a/tests/darnit_baseline/test_agent_flow.py b/tests/darnit_baseline/test_agent_flow.py index d4149c9ec..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})) @@ -139,6 +155,18 @@ 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_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() diff --git a/tests/darnit_baseline/test_get_pending_context.py b/tests/darnit_baseline/test_get_pending_context.py index 473bb0796..6cf48ce83 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, @@ -251,8 +254,8 @@ def test_boolean_question_has_ask_user_with_yes_no(self) -> 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, @@ -268,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).""" @@ -313,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?", @@ -331,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.""" @@ -512,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?", @@ -531,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: @@ -602,3 +617,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", 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")) + @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 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