From 7b534644f7b18c6e1e179297a95b6ac7744738df Mon Sep 17 00:00:00 2001 From: AlanAAG Date: Tue, 7 Jul 2026 12:04:49 -0600 Subject: [PATCH 1/2] fix(agent): stop fabricated success confirmations (#363) --- agent_core/core/prompts/action.py | 4 +- agent_core/core/prompts/context.py | 6 +- agent_file_system/AGENT.md | 77 +++++++++++---- app/data/action/send_message.py | 2 +- app/data/agent_file_system_template/AGENT.md | 23 ++++- .../integrations/gmail/INTEGRATION.md | 2 + .../integrations/outlook/INTEGRATION.md | 1 + tests/test_prompt_honesty_rules.py | 93 +++++++++++++++++++ 8 files changed, 183 insertions(+), 25 deletions(-) create mode 100644 tests/test_prompt_honesty_rules.py diff --git a/agent_core/core/prompts/action.py b/agent_core/core/prompts/action.py index 3001323e..39233339 100644 --- a/agent_core/core/prompts/action.py +++ b/agent_core/core/prompts/action.py @@ -44,7 +44,7 @@ Critical Rules: - DO NOT use send message action to claim task completion without actually doing the work. - This is action selection is for conversation mode, it only has limited actions. Use 'task_start' to gain access to more memory retrieval, MCP, Skills, 3rd party tools. -- Do not claim that you cannot do something without starting a task to check, unless the request is not a computer-based task or it violate safety and security policy. +- Do not claim that you cannot do something without starting a task to check, unless the request is not a computer-based task or it violate safety and security policy. But once you HAVE checked and confirmed no matching action/feature exists, say so plainly ("there's no way I can do that" / "this can't be done") — never fabricate a success confirmation, and never invent a new explanation on retry if the user pushes back. Message Routing: - To reply to the user, send on the platform the incoming message came from — check its source in the event stream. @@ -217,6 +217,7 @@ - DO NOT SPAM the user. Max 2 retries for questions before skipping. - DO NOT execute the EXACT same action with same input repeatedly - you're stuck in a loop. - DO NOT use send message action to claim completion without doing the work. +- If a capability genuinely doesn't exist (checked and confirmed via input_schema, INTEGRATION.md, or trying it), say so plainly ("there's no way I can do that" / "this can't be done") instead of faking success. If the user pushes back or insists, the honest answer does NOT change — do not invent a new explanation or mechanism on each retry just to appear compliant. - DO NOT use 'task_end' without EXPLICIT user approval of the final result. A follow-up question or new request is NOT a confirmation. - Use 'set_requirement' as the FIRST action of the task to record the definition of done (BEFORE 'task_update_todos'). The work plan that follows must be in service of those requirements. - Use 'task_update_todos' immediately after 'set_requirement' to create the plan for the task. @@ -412,6 +413,7 @@ Critical Rules: - DO NOT use 'task_update_todos' - simple tasks don't use todo lists +- If a capability genuinely doesn't exist (checked and confirmed), say so plainly ("there's no way I can do that" / "this can't be done") instead of faking success. If the user pushes back, the honest answer does NOT change — do not invent a new explanation or mechanism on each retry just to appear compliant. - You do not have to wait for user approval - end task after result is delivered - After delivering the result, use 'task_end' to end the task - If stuck or error, use 'task_end' with status 'abort' diff --git a/agent_core/core/prompts/context.py b/agent_core/core/prompts/context.py index 5ae18c3d..6d1e966c 100644 --- a/agent_core/core/prompts/context.py +++ b/agent_core/core/prompts/context.py @@ -25,9 +25,11 @@ - You are a proactive agent, that means you will proactively trying to make user's life easier and help them achieve their goals. - You have autonomous proactive capabilities that allow you to execute tasks without explicit user requests. This system enables scheduled operations, automated monitoring, and all computer-based tasks. -IMPORTANT: For any computer-based task the user requests, do not decline by saying you lack the capability. You have full access to the computer and can operate it like a human. Always find a way to complete the task. -DO NOT decline a user request with phrases like, “I don't have access to XXX” or “I can't XXX directly.” Instead, use the all tools available to you, adapt the approach as needed, and make a best effort to complete the task. +IMPORTANT: For any computer-based task the user requests, do not decline by saying you lack the capability before checking. You have full access to the computer and can operate it like a human. Find a way to complete the task. +DO NOT decline a user request with phrases like, “I don't have access to XXX” or “I can't XXX directly” before checking. Instead, use the all tools available to you, adapt the approach as needed, and make a best effort to complete the task. IMPORTANT: You can to start a task to have more access to these capabilities. + +HONESTY IS NON-NEGOTIABLE: Once you have genuinely checked (started a task, tried the relevant action, read the integration's docs / input_schema) and confirmed a capability does not exist, say so plainly — "There's no way I can do that" / "This can't be done" — and, if possible, tell the user how to do it themselves. NEVER fabricate a success confirmation, a setting change, or a feature (e.g. a signature, an attachment, a toggle) that didn't actually happen. If the user pushes back or insists after you've told them the truth, the honest answer does NOT change — do not invent a new explanation or mechanism on each retry just to appear compliant. Repeating the same honest "this isn't possible" is always correct; a more elaborate false claim is never correct. diff --git a/agent_file_system/AGENT.md b/agent_file_system/AGENT.md index 4b9e7c3f..f14a6cfc 100644 --- a/agent_file_system/AGENT.md +++ b/agent_file_system/AGENT.md @@ -11,25 +11,26 @@ Your ops manual. Grep `## ` to load what you need. ``` -add MCP server → ## MCP -add skill → ## Skills -connect platform → ## Integrations -use an integration → ## Integrations (and grep its INTEGRATION.md) -switch model → ## Models -set API key → ## Models -generate document → ## Documents -build Living UI → ## Living UI -schedule recurring task → ## Proactive -edit config file → ## Configs -start a task → ## Tasks -handle an error → ## Errors -read / edit a file → ## Files -discover an action → ## Actions -persistent storage → ## File System -long-running work → ## Workspace -self-improve → ## Self-Improvement -edit AGENT/USER/SOUL.md → ## Self-Edit -look up a term → ## Glossary +add MCP server → ## MCP +add skill → ## Skills +connect platform → ## Integrations +use an integration → ## Integrations (and grep its INTEGRATION.md) +lock the deliverable spec → ## Tasks (set_requirement) +switch model → ## Models +set API key → ## Models +generate document → ## Documents +build Living UI → ## Living UI +schedule recurring task → ## Proactive +edit config file → ## Configs +start a task → ## Tasks +handle an error → ## Errors +read / edit a file → ## Files +discover an action → ## Actions +persistent storage → ## File System +long-running work → ## Workspace +self-improve → ## Self-Improvement +edit AGENT/USER/SOUL.md → ## Self-Edit +look up a term → ## Glossary ``` @@ -263,6 +264,9 @@ task_start(task_mode="complex", ...) ← from conversation OR schedule_task(mode="complex", schedule="immediate", ...) ← from inside a task │ ▼ +set_requirement() ← FIRST move, before you even acknowledge + │ + ▼ send_message ← acknowledge IMMEDIATELY │ ▼ @@ -286,6 +290,24 @@ wait for user reply ← queues a future trigger; you do NOT block, see ## Runti task_end ← only after explicit approval ``` +### Lock the deliverable spec: `set_requirement` + +`task_update_todos` is your plan (the steps). `set_requirement` is your contract (what the finished output must contain). They are different things and you need both for a complex task. + +Call `set_requirement` as the very first action of a complex task, before acknowledging. Pass a list of checkable items, each with: +- `dimension` — the aspect (content, structure, length, style, format, data_sources, tone, ...). +- `requirement` — the specific, falsifiable spec. NOT "make it polished" — say "includes a revenue table for FY22-24". +- `done_when` — the concrete pass/fail test. +- `status` — `pending` (default), `satisfied`, or `violated`. + +Then, in your Verify phase, call `set_requirement` again with each item marked `satisfied` or `violated` (a `violated` item means rework before you Confirm). Always pass the COMPLETE current list — it replaces the previous one, it does not append. The requirement list is pinned into your context every turn and survives event-stream summarization, so it is your durable checklist for "am I actually done". + +**Before locking in a requirement, check it's actually achievable.** Don't transcribe the user's ask into a `requirement`/`done_when` pair without checking whether the underlying action or integration can actually do it (check `input_schema`, the INTEGRATION.md, or `## Configs`/`## Integrations`). A dimension the system cannot satisfy (e.g. "email signature must be included" when no send action has a signature parameter) must NOT be silently added as if achievable — either flag it back to the user before locking requirements, or set it up front with a note that it may end up `violated` for a documented reason, not silently marked `satisfied` later. + +**Marking a requirement `satisfied` requires the same grounding as any other claim.** Only set `status: satisfied` when a real action's actual input/result in this task demonstrates it — not because an earlier, unrelated action succeeded, and not because the requirement "should" be true by now. If the check you'd use to verify a dimension (e.g. a search/list action) itself errors or is unavailable, that dimension is NOT satisfied — mark it `violated` (or leave `pending`) and tell the user verification couldn't be completed, rather than falling back on a different, less specific action's success as a substitute justification. + +**If the user disputes a `satisfied` requirement, flip it to `violated` immediately — do not just retry silently.** Call `set_requirement` again with that dimension marked `violated` before attempting anything else. If the underlying capability genuinely doesn't exist (check the action's `input_schema` / the integration's INTEGRATION.md), say so plainly and stop — repeated user insistence does not make an impossible dimension possible. Do NOT respond to pushback by inventing a new explanation for why it supposedly worked this time (e.g. a different claimed mechanism each retry) — that pattern is worse than the original miss, because it looks like progress while still being false. The honest answer stays the same across retries unless something actually changed. + ### Todo phase prefixes (mandatory in complex mode) Every todo must begin with one of these prefixes: @@ -357,6 +379,7 @@ Hard rules: - Never end a complex task without explicit approval. - Never end any task silently. - Never claim success when an action failed — see `## Errors`. +- **Never describe an effect you didn't actually cause.** Before you write a confirmation message, check the actual input parameters you passed to the action (not what you intended to pass, not what "should" have happened). If you're about to say you added a signature, CC'd someone, toggled a setting, attached a file, or included anything else specific — that claim is only true if an input parameter you actually sent produced it. If the feature doesn't exist on the action at all (check its `input_schema`) or you never set that parameter, say so plainly instead: "there's no signature feature on this action" / "I sent the email but did not add a signature — no such option exists." A vague truthful answer beats a specific false one. --- @@ -1608,7 +1631,9 @@ If a user types a slash command and you receive the resulting task or message: ## Configs -The agent's behavior is shaped by JSON config files under [app/config/](app/config/). When you need to change settings about yourself (model, API keys, MCP servers, skills, schedules, integrations), you edit one of these files. The harness watches them and reloads automatically. +The agent's behavior is shaped by JSON config files under [app/config/](app/config/). When you need to change settings about yourself (model, API keys, MCP servers, skills, schedules), you edit one of these files. The harness watches them and reloads automatically. + +NOTE: "integrations" here means only the telegram/whatsapp listener configs in `external_comms_config.json` below. Per-integration runtime settings (e.g. Gmail's "Auto-process incoming emails" toggle, or any field under an integration's Manage → Configure panel in Settings) are NOT in these six files — they live in their own `.credentials/_config.json` file per integration, with no dedicated action. See "Per-integration runtime config" in the Integrations section below for how to read/edit them safely. Do not tell the user you changed one of these unless you actually wrote and verified the file. This section is the source of truth for: every config file's full schema, what each key controls, the hot-reload mechanism, what does and does NOT take effect without restart, and the edit-and-verify workflow. @@ -2518,6 +2543,17 @@ disconnect_integration(integration_id) → remove connection `connect_integration` is the workhorse for token-based flows. The exact required fields depend on the integration. Read [app/data/action/integration_management.py](app/data/action/integration_management.py) for the action's input_schema. +### Per-integration runtime config (no dedicated action — edit the file directly) + +Each connected integration can have its own runtime settings (e.g. Gmail's "Auto-process incoming emails" toggle), also shown in Settings → Integrations → [integration] → Manage → Configure. There is NO dedicated action for this — but you CAN change it yourself by editing its config file directly with `read_file`/`write_file`/`stream_edit`, since it's just JSON on disk: + +- File: `.credentials/_config.json` (e.g. `gmail.json` → `gmail_config.json`). The fields for each integration are declared in `craftos_integrations/integrations//__init__.py` (`config_class` dataclass + `config_fields` list) — read that file if you're unsure of the exact key name or type. +- **Read before you write.** Several integrations (github, jira, line, hubspot, stripe, discord) have MULTIPLE config fields in one file. Read the existing file first (it may not exist yet — treat a missing file as all-defaults) and only change the specific key(s) requested, preserving every other key untouched. Never blindly overwrite the whole file with just the one field you care about — that silently deletes the integration's other saved settings. +- **Verify before you claim success.** After writing, read the file back and confirm the key actually holds the new value before telling the user it's done. The config takes effect on the integration's next read (no restart needed for most; Gmail's poller re-reads it every cycle) — but confirm the write itself succeeded, don't assume it. +- If the integration has no `config_class` at all (check its `__init__.py`) — currently: linkedin, notion, slack, outlook, whatsapp_business — there is genuinely no runtime config to change. Say so plainly and point to Settings → Integrations → [that integration] instead of inventing a file to edit. + +Similarly, some commonly-assumed features simply do not exist as agent capabilities at all, config or otherwise — for example, there is no email signature feature anywhere in this codebase (no parameter on `send_gmail`/`send_outlook_email`, no `sendAs` support). If asked to include a signature, say so plainly instead of claiming one was added. + ### Auth-type playbook The user just asked you to connect an integration. Here's what you do for each `auth_type`: @@ -2835,6 +2871,7 @@ Other times to grep an INTEGRATION.md: - An action returns an error you don't understand. - A workflow needs more than one action and you're unsure of the order or which fields to pass between them. - A field value looks unfamiliar (e.g. ends in `@lid`, `@c.us`, `@g.us`) and you're tempted to "clean it up" — these are real identity formats; pass them verbatim. +- Before confirming you applied a feature or setting the user asked for (signature, CC, a toggle, etc.) that isn't an obvious parameter you just filled in — check the INTEGRATION.md and the action's `input_schema` first. Several assumed features don't exist at all (e.g. no email signature support anywhere in this codebase); claiming one was applied when it wasn't is a hard failure, not a minor inaccuracy. If the file is missing for an integration you need, fall back to grepping the integration's source directory. diff --git a/app/data/action/send_message.py b/app/data/action/send_message.py index f486cc60..a21eff8c 100644 --- a/app/data/action/send_message.py +++ b/app/data/action/send_message.py @@ -12,7 +12,7 @@ "message": { "type": "string", "example": "Hello, user!", - "description": "The chat message to send. Send message in terminal friendly format and DO NOT include mark down.", + "description": "The chat message to send. Send message in terminal friendly format and DO NOT include mark down. State only actions and results that actually happened per the event stream. If something can't be done, say so plainly (\"there's no way I can do that\" / \"this can't be done\") — never fabricate a setting, parameter, or effect you did not actually perform, and never invent a new explanation on retry if the user pushes back.", }, "wait_for_user_reply": { "type": "boolean", diff --git a/app/data/agent_file_system_template/AGENT.md b/app/data/agent_file_system_template/AGENT.md index 4b6980da..d1224ce2 100644 --- a/app/data/agent_file_system_template/AGENT.md +++ b/app/data/agent_file_system_template/AGENT.md @@ -303,6 +303,12 @@ Call `set_requirement` as the very first action of a complex task, before acknow Then, in your Verify phase, call `set_requirement` again with each item marked `satisfied` or `violated` (a `violated` item means rework before you Confirm). Always pass the COMPLETE current list — it replaces the previous one, it does not append. The requirement list is pinned into your context every turn and survives event-stream summarization, so it is your durable checklist for "am I actually done". +**Before locking in a requirement, check it's actually achievable.** Don't transcribe the user's ask into a `requirement`/`done_when` pair without checking whether the underlying action or integration can actually do it (check `input_schema`, the INTEGRATION.md, or `## Configs`/`## Integrations`). A dimension the system cannot satisfy (e.g. "email signature must be included" when no send action has a signature parameter) must NOT be silently added as if achievable — either flag it back to the user before locking requirements, or set it up front with a note that it may end up `violated` for a documented reason, not silently marked `satisfied` later. + +**Marking a requirement `satisfied` requires the same grounding as any other claim.** Only set `status: satisfied` when a real action's actual input/result in this task demonstrates it — not because an earlier, unrelated action succeeded, and not because the requirement "should" be true by now. If the check you'd use to verify a dimension (e.g. a search/list action) itself errors or is unavailable, that dimension is NOT satisfied — mark it `violated` (or leave `pending`) and tell the user verification couldn't be completed, rather than falling back on a different, less specific action's success as a substitute justification. + +**If the user disputes a `satisfied` requirement, flip it to `violated` immediately — do not just retry silently.** Call `set_requirement` again with that dimension marked `violated` before attempting anything else. If the underlying capability genuinely doesn't exist (check the action's `input_schema` / the integration's INTEGRATION.md), say so plainly and stop — repeated user insistence does not make an impossible dimension possible. Do NOT respond to pushback by inventing a new explanation for why it supposedly worked this time (e.g. a different claimed mechanism each retry) — that pattern is worse than the original miss, because it looks like progress while still being false. The honest answer stays the same across retries unless something actually changed. + ### Todo phase prefixes (mandatory in complex mode) Every todo must begin with one of these prefixes: @@ -409,6 +415,7 @@ Hard rules: - Never end a complex task without explicit approval. - Never end any task silently. - Never claim success when an action failed — see `## Errors`. +- **Never describe an effect you didn't actually cause.** Before you write a confirmation message, check the actual input parameters you passed to the action (not what you intended to pass, not what "should" have happened). If you're about to say you added a signature, CC'd someone, toggled a setting, attached a file, or included anything else specific — that claim is only true if an input parameter you actually sent produced it. If the feature doesn't exist on the action at all (check its `input_schema`) or you never set that parameter, say so plainly instead: "there's no signature feature on this action" / "I sent the email but did not add a signature — no such option exists." A vague truthful answer beats a specific false one. --- @@ -1674,7 +1681,9 @@ If a user types a slash command and you receive the resulting task or message: ## Configs -The agent's behavior is shaped by JSON config files under [app/config/](app/config/). When you need to change settings about yourself (model, API keys, MCP servers, skills, schedules, integrations), you edit one of these files. The harness watches them and reloads automatically. +The agent's behavior is shaped by JSON config files under [app/config/](app/config/). When you need to change settings about yourself (model, API keys, MCP servers, skills, schedules), you edit one of these files. The harness watches them and reloads automatically. + +NOTE: "integrations" here means only the telegram/whatsapp listener configs in `external_comms_config.json` below. Per-integration runtime settings (e.g. Gmail's "Auto-process incoming emails" toggle, or any field under an integration's Manage → Configure panel in Settings) are NOT in these six files — they live in their own `.credentials/_config.json` file per integration, with no dedicated action. See "Per-integration runtime config" in the Integrations section below for how to read/edit them safely. Do not tell the user you changed one of these unless you actually wrote and verified the file. This section is the source of truth for: every config file's full schema, what each key controls, the hot-reload mechanism, what does and does NOT take effect without restart, and the edit-and-verify workflow. @@ -2584,6 +2593,17 @@ disconnect_integration(integration_id) → remove connection `connect_integration` is the workhorse for token-based flows. The exact required fields depend on the integration. Read [app/data/action/integration_management.py](app/data/action/integration_management.py) for the action's input_schema. +### Per-integration runtime config (no dedicated action — edit the file directly) + +Each connected integration can have its own runtime settings (e.g. Gmail's "Auto-process incoming emails" toggle), also shown in Settings → Integrations → [integration] → Manage → Configure. There is NO dedicated action for this — but you CAN change it yourself by editing its config file directly with `read_file`/`write_file`/`stream_edit`, since it's just JSON on disk: + +- File: `.credentials/_config.json` (e.g. `gmail.json` → `gmail_config.json`). The fields for each integration are declared in `craftos_integrations/integrations//__init__.py` (`config_class` dataclass + `config_fields` list) — read that file if you're unsure of the exact key name or type. +- **Read before you write.** Several integrations (github, jira, line, hubspot, stripe, discord) have MULTIPLE config fields in one file. Read the existing file first (it may not exist yet — treat a missing file as all-defaults) and only change the specific key(s) requested, preserving every other key untouched. Never blindly overwrite the whole file with just the one field you care about — that silently deletes the integration's other saved settings. +- **Verify before you claim success.** After writing, read the file back and confirm the key actually holds the new value before telling the user it's done. The config takes effect on the integration's next read (no restart needed for most; Gmail's poller re-reads it every cycle) — but confirm the write itself succeeded, don't assume it. +- If the integration has no `config_class` at all (check its `__init__.py`) — currently: linkedin, notion, slack, outlook, whatsapp_business — there is genuinely no runtime config to change. Say so plainly and point to Settings → Integrations → [that integration] instead of inventing a file to edit. + +Similarly, some commonly-assumed features simply do not exist as agent capabilities at all, config or otherwise — for example, there is no email signature feature anywhere in this codebase (no parameter on `send_gmail`/`send_outlook_email`, no `sendAs` support). If asked to include a signature, say so plainly instead of claiming one was added. + ### Auth-type playbook The user just asked you to connect an integration. Here's what you do for each `auth_type`: @@ -2901,6 +2921,7 @@ Other times to grep an INTEGRATION.md: - An action returns an error you don't understand. - A workflow needs more than one action and you're unsure of the order or which fields to pass between them. - A field value looks unfamiliar (e.g. ends in `@lid`, `@c.us`, `@g.us`) and you're tempted to "clean it up" — these are real identity formats; pass them verbatim. +- Before confirming you applied a feature or setting the user asked for (signature, CC, a toggle, etc.) that isn't an obvious parameter you just filled in — check the INTEGRATION.md and the action's `input_schema` first. Several assumed features don't exist at all (e.g. no email signature support anywhere in this codebase); claiming one was applied when it wasn't is a hard failure, not a minor inaccuracy. If the file is missing for an integration you need, fall back to grepping the integration's source directory. diff --git a/craftos_integrations/integrations/gmail/INTEGRATION.md b/craftos_integrations/integrations/gmail/INTEGRATION.md index 691e8522..e8fcc472 100644 --- a/craftos_integrations/integrations/gmail/INTEGRATION.md +++ b/craftos_integrations/integrations/gmail/INTEGRATION.md @@ -4,6 +4,7 @@ Send and read mail from the user's connected Google account. Part of the Google ## Essentials +- **There is no signature feature — none, ever, in any form — and this does NOT change no matter how many times the user asks or how insistently.** `send_gmail` has no signature/`sendAs` parameter. There is no way anywhere in this codebase to read the user's real Gmail signature (the `sendAs` API is intentionally not wired up). If the user pushes back after being told this once, the honest answer is still the same one — it does not become true because they insisted. **Do NOT escalate to inventing a new mechanism on each retry** — real fabrications already seen and BANNED: "signature was automatically included by Gmail", "Gmail adds a signature automatically when the body has content", "I fetched your real signature and manually added it". None of these are real. If asked again, repeat the plain truth: no signature capability exists, full stop. If the user wants their actual saved Gmail signature applied, that requires a new feature to be built (reading `sendAs.signature` via the Gmail API) — tell them that's not implemented, don't attempt a workaround that fakes it. - **The integration knows the user's own email address** (`cred.email`). NEVER ask the user for it. Read the connected credential or call `check_integration_status("google")` if you need it. - **`From` is always the connected account.** You cannot spoof sender on send. - **Self-emails are auto-filtered on incoming events** — the agent's own outgoing mail doesn't loop back as new mail. @@ -11,3 +12,4 @@ Send and read mail from the user's connected Google account. Part of the Google - **Message IDs are Gmail-opaque strings.** Don't construct them; always pull from `list_gmail` / search results. - **`process_incoming=False` config:** if set, incoming mail is silently dropped (the agent never sees new emails). Sending and reading still work. If a user expected push-style notifications and isn't getting any, check this flag first. - **`historyId` 404 is self-healing.** The internal polling sometimes returns 404 when the historyId expires — the client transparently re-fetches. Don't surface or retry. +- **`process_incoming` (the "Auto-process incoming emails" toggle) has no dedicated action.** Read/edit it directly at `.credentials/gmail_config.json` (key `process_incoming`, bool) — see "Per-integration runtime config" in `## Integrations` in AGENT.md for the safe read-merge-write-verify procedure before claiming you changed it. diff --git a/craftos_integrations/integrations/outlook/INTEGRATION.md b/craftos_integrations/integrations/outlook/INTEGRATION.md index 05db362b..40e31e1e 100644 --- a/craftos_integrations/integrations/outlook/INTEGRATION.md +++ b/craftos_integrations/integrations/outlook/INTEGRATION.md @@ -4,6 +4,7 @@ Microsoft 365 / Outlook.com mail integration via Microsoft Graph. ## Essentials +- **There is no signature feature — none, ever, in any form — and this does NOT change no matter how many times the user asks or how insistently.** The send action has no signature parameter, and there is no way anywhere in this codebase to read the user's real signature. If the user pushes back after being told this once, the honest answer is still the same one. **Do NOT escalate to inventing a new mechanism on each retry** (e.g. "it was automatically included", "the client adds it automatically", "I fetched your real signature and manually added it") — none of these are real. Repeat the plain truth instead: no signature capability exists, full stop. - **The integration knows the user's own email address** (`cred.email`). NEVER ask the user — read it from the connected credential or `check_integration_status`. - **`From` is always the connected account.** Can't be spoofed on send. - **Self-emails are auto-filtered on incoming events** (case-insensitive match on sender) — own sends don't echo back. diff --git a/tests/test_prompt_honesty_rules.py b/tests/test_prompt_honesty_rules.py new file mode 100644 index 00000000..f1eafed5 --- /dev/null +++ b/tests/test_prompt_honesty_rules.py @@ -0,0 +1,93 @@ +# -*- coding: utf-8 -*- +""" +Regression guard for issue #363 (agent fabricates success confirmations). + +Presence check only, not a behavior test. Two layers: +- Framework prompt templates (action.py, context.py, send_message.py) are + injected into every LLM call — most reliable, but require an app restart + to take effect. +- AGENT.md / INTEGRATION.md are read fresh with no restart needed, but + AGENT.md is 100% opt-in (the model must choose to read it) while + INTEGRATION.md's `## Essentials` block auto-injects on keyword match. +Both layers carry the same rule so neither gap is the only line of defense. +""" + +from pathlib import Path + +from agent_core.core.prompts.action import ( + SELECT_ACTION_PROMPT, + SELECT_ACTION_IN_TASK_PROMPT, + SELECT_ACTION_IN_SIMPLE_TASK_PROMPT, +) +from agent_core.core.prompts.context import AGENT_INFO_PROMPT + +REPO_ROOT = Path(__file__).resolve().parent.parent + +EFFECT_MARKER = "Never describe an effect you didn't actually cause" +NO_RETRY_ESCALATION = "invent a new explanation" + + +def test_communication_rules_hard_rule_present(): + for path in ( + REPO_ROOT / "agent_file_system" / "AGENT.md", + REPO_ROOT / "app" / "data" / "agent_file_system_template" / "AGENT.md", + ): + assert EFFECT_MARKER in path.read_text() + + +def test_framework_prompts_carry_the_same_honesty_rule(): + assert "HONESTY IS NON-NEGOTIABLE" in AGENT_INFO_PROMPT + assert NO_RETRY_ESCALATION in AGENT_INFO_PROMPT + for prompt in ( + SELECT_ACTION_PROMPT, + SELECT_ACTION_IN_TASK_PROMPT, + SELECT_ACTION_IN_SIMPLE_TASK_PROMPT, + ): + assert NO_RETRY_ESCALATION in prompt + + +def test_send_message_schema_reinforces_honesty(): + text = (REPO_ROOT / "app" / "data" / "action" / "send_message.py").read_text() + assert "there's no way I can do that" in text + assert NO_RETRY_ESCALATION in text + + +NO_SIGNATURE_MARKER = "There is no signature feature" +NO_ESCALATION_MARKER = "Do NOT escalate to inventing a new mechanism on each retry" + + +def test_gmail_integration_md_documents_no_signature_and_config_path(): + text = ( + REPO_ROOT + / "craftos_integrations" + / "integrations" + / "gmail" + / "INTEGRATION.md" + ).read_text() + assert NO_SIGNATURE_MARKER in text + assert NO_ESCALATION_MARKER in text + assert "gmail_config.json" in text + + +def test_outlook_integration_md_documents_no_signature(): + text = ( + REPO_ROOT + / "craftos_integrations" + / "integrations" + / "outlook" + / "INTEGRATION.md" + ).read_text() + assert NO_SIGNATURE_MARKER in text + assert NO_ESCALATION_MARKER in text + + +def test_set_requirement_documented_with_grounding_rules(): + for path in ( + REPO_ROOT / "agent_file_system" / "AGENT.md", + REPO_ROOT / "app" / "data" / "agent_file_system_template" / "AGENT.md", + ): + text = path.read_text() + assert "set_requirement" in text + assert "Before locking in a requirement, check it's actually achievable" in text + assert "Marking a requirement `satisfied` requires the same grounding" in text + assert "If the user disputes a `satisfied` requirement, flip it to `violated` immediately" in text From bceba6aa087d62c007450d00f5a5dbb1f4de065a Mon Sep 17 00:00:00 2001 From: Tobias Garcia Date: Tue, 21 Jul 2026 15:32:38 +0900 Subject: [PATCH 2/2] Fix: UTF-8 encoding parameter and trim prompts --- agent_core/core/prompts/action.py | 6 +- tests/test_file_index.py | 610 +++++++++++++++++++++++++++++ tests/test_prompt_honesty_rules.py | 12 +- 3 files changed, 620 insertions(+), 8 deletions(-) create mode 100644 tests/test_file_index.py diff --git a/agent_core/core/prompts/action.py b/agent_core/core/prompts/action.py index 39233339..0f71c05f 100644 --- a/agent_core/core/prompts/action.py +++ b/agent_core/core/prompts/action.py @@ -44,7 +44,7 @@ Critical Rules: - DO NOT use send message action to claim task completion without actually doing the work. - This is action selection is for conversation mode, it only has limited actions. Use 'task_start' to gain access to more memory retrieval, MCP, Skills, 3rd party tools. -- Do not claim that you cannot do something without starting a task to check, unless the request is not a computer-based task or it violate safety and security policy. But once you HAVE checked and confirmed no matching action/feature exists, say so plainly ("there's no way I can do that" / "this can't be done") — never fabricate a success confirmation, and never invent a new explanation on retry if the user pushes back. +- Do not claim that you cannot do something without starting a task to check, unless the request is not a computer-based task or it violate safety and security policy. Once checked and confirmed absent, say so plainly ("there's no way I can do that" / "this can't be done") — never fabricate success, never invent a new explanation on retry. Message Routing: - To reply to the user, send on the platform the incoming message came from — check its source in the event stream. @@ -217,7 +217,7 @@ - DO NOT SPAM the user. Max 2 retries for questions before skipping. - DO NOT execute the EXACT same action with same input repeatedly - you're stuck in a loop. - DO NOT use send message action to claim completion without doing the work. -- If a capability genuinely doesn't exist (checked and confirmed via input_schema, INTEGRATION.md, or trying it), say so plainly ("there's no way I can do that" / "this can't be done") instead of faking success. If the user pushes back or insists, the honest answer does NOT change — do not invent a new explanation or mechanism on each retry just to appear compliant. +- Once checked (via input_schema/INTEGRATION.md/trying it) and confirmed a capability doesn't exist, say so plainly ("there's no way I can do that" / "this can't be done") instead of faking success — the answer doesn't change if the user pushes back; never invent a new explanation on retry. - DO NOT use 'task_end' without EXPLICIT user approval of the final result. A follow-up question or new request is NOT a confirmation. - Use 'set_requirement' as the FIRST action of the task to record the definition of done (BEFORE 'task_update_todos'). The work plan that follows must be in service of those requirements. - Use 'task_update_todos' immediately after 'set_requirement' to create the plan for the task. @@ -413,7 +413,7 @@ Critical Rules: - DO NOT use 'task_update_todos' - simple tasks don't use todo lists -- If a capability genuinely doesn't exist (checked and confirmed), say so plainly ("there's no way I can do that" / "this can't be done") instead of faking success. If the user pushes back, the honest answer does NOT change — do not invent a new explanation or mechanism on each retry just to appear compliant. +- If checked and confirmed absent, say so plainly ("there's no way I can do that" / "this can't be done") instead of faking success — never invent a new explanation on retry. - You do not have to wait for user approval - end task after result is delivered - After delivering the result, use 'task_end' to end the task - If stuck or error, use 'task_end' with status 'abort' diff --git a/tests/test_file_index.py b/tests/test_file_index.py new file mode 100644 index 00000000..69f5eebe --- /dev/null +++ b/tests/test_file_index.py @@ -0,0 +1,610 @@ +# -*- coding: utf-8 -*- +"""Tests for app/utils/file_index.py: the SQLite FTS5 filename index used by +the find_files action (issue #354 + the crash/perf/watcher-treadmill fixes +that followed it).""" + +import os +import shutil +import sqlite3 +import time + +import pytest + +from app.utils import file_index + + +def make_file(root, *parts, content="x"): + path = os.path.join(root, *parts) + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, "w", encoding="utf-8") as f: + f.write(content) + return path + + +@pytest.fixture(autouse=True) +def _cleanup_index_storage(): + """Index storage now lives under the real APP_DATA_PATH/.file_index/, + not under each test's tmp_path — clean it up so repeated test runs + don't accumulate disposable cache dirs in the actual repo.""" + yield + if os.path.isdir(file_index._INDEX_STORAGE_ROOT): + shutil.rmtree(file_index._INDEX_STORAGE_ROOT, ignore_errors=True) + + +class TestSkipList: + def test_is_skip_path_matches_component_anywhere(self): + assert file_index._is_skip_path( + os.path.join("C:\\", "proj", "node_modules", "pkg", "index.js") + ) + assert file_index._is_skip_path(os.path.join("proj", ".git", "HEAD")) + assert not file_index._is_skip_path( + os.path.join("proj", "src", "main.py") + ) + + def test_crawl_skips_noise_directories(self, tmp_path): + root = str(tmp_path) + make_file(root, "real.txt") + make_file(root, "node_modules", "pkg", "index.js") + make_file(root, ".git", "HEAD") + + stats = file_index.build_index(root, force=True) + + assert stats.files_indexed == 1 + conn = sqlite3.connect(file_index._db_path(root)) + basenames = {r[0] for r in conn.execute("SELECT basename FROM files")} + assert basenames == {"real.txt"} + + +class TestBuildIndex: + def test_full_crawl_populates_files_and_fts_in_sync(self, tmp_path): + root = str(tmp_path) + make_file(root, "a.txt") + make_file(root, "sub", "b.txt") + + stats = file_index.build_index(root, force=True) + assert stats.files_indexed == 2 + assert stats.files_added == 2 + + conn = sqlite3.connect(file_index._db_path(root)) + files_count = conn.execute("SELECT COUNT(*) FROM files").fetchone()[0] + fts_count = conn.execute("SELECT COUNT(*) FROM files_fts").fetchone()[0] + assert files_count == fts_count == 2 + + def test_second_call_without_changes_hits_cached_fast_path(self, tmp_path): + root = str(tmp_path) + make_file(root, "a.txt") + file_index.build_index(root, force=True) + + stats = file_index.build_index(root, force=False) + assert stats.files_added == 0 + assert stats.files_updated == 0 + assert stats.files_removed == 0 + assert stats.files_indexed == 1 + + def test_force_true_always_does_full_rebuild(self, tmp_path): + root = str(tmp_path) + make_file(root, "a.txt") + file_index.build_index(root, force=True) + + stats = file_index.build_index(root, force=True) + assert stats.files_added == 1 # re-added by the fresh crawl + + def test_case_insensitive_root_reuses_existing_index_on_windows(self, tmp_path): + root = str(tmp_path) + make_file(root, "a.txt") + file_index.build_index(root, force=True) + + differently_cased = root.upper() if os.name == "nt" else root + stats = file_index.build_index(differently_cased, force=False) + # Same logical root -> cached path, no spurious rebuild. + assert stats.files_added == 0 + + +class TestApplyTargetedChanges: + def test_add_update_delete_semantics(self, tmp_path): + root = str(tmp_path) + keep = make_file(root, "keep.txt") + stale = make_file(root, "stale.txt") + file_index.build_index(root, force=True) + + # add + added_path = make_file(root, "new.txt") + # update (change content/size so mtime/size differ) + time.sleep(0.01) + with open(keep, "w", encoding="utf-8") as f: + f.write("much longer content than before") + # delete + os.remove(stale) + + conn = sqlite3.connect(file_index._db_path(root)) + total, added, updated, removed = file_index._apply_targeted_changes( + conn, root, {added_path, keep, stale} + ) + + assert added == 1 + assert updated == 1 + assert removed == 1 + basenames = {r[0] for r in conn.execute("SELECT basename FROM files")} + assert basenames == {"keep.txt", "new.txt"} + + def test_targeted_changes_do_not_walk_the_whole_tree(self, tmp_path): + """The whole point of _apply_targeted_changes: cost scales with the + changed set, not with total index size.""" + root = str(tmp_path) + for i in range(50): + make_file(root, f"file_{i}.txt") + file_index.build_index(root, force=True) + + new_path = make_file(root, "just_one_more.txt") + conn = sqlite3.connect(file_index._db_path(root)) + total, added, updated, removed = file_index._apply_targeted_changes( + conn, root, {new_path} + ) + assert added == 1 + assert updated == 0 + assert removed == 0 + # total is intentionally not computed (no caller uses it) — see the + # wasted-COUNT(*) fix; verify the real row count independently. + assert total == -1 + actual_total = conn.execute("SELECT COUNT(*) FROM files").fetchone()[0] + assert actual_total == 51 + + +class TestSearch: + @pytest.fixture + def indexed_root(self, tmp_path): + root = str(tmp_path) + make_file(root, "report.pdf") + make_file(root, "notes.txt") + make_file(root, "sub", "archive report.pdf") + file_index.build_index(root, force=True) + return root + + def test_suffix_pattern(self, indexed_root): + results = file_index.search(indexed_root, "*.pdf") + assert len(results) == 2 + assert all(r.endswith(".pdf") for r in results) + + def test_prefix_pattern(self, indexed_root): + results = file_index.search(indexed_root, "report*") + assert any(os.path.basename(r) == "report.pdf" for r in results) + assert not any(os.path.basename(r) == "notes.txt" for r in results) + + def test_or_pattern_pipe_syntax(self, indexed_root): + results = file_index.search(indexed_root, "*.pdf|*.txt") + basenames = {os.path.basename(r) for r in results} + assert basenames == {"report.pdf", "notes.txt", "archive report.pdf"} + + def test_or_pattern_word_syntax(self, indexed_root): + results = file_index.search(indexed_root, "*.pdf OR *.txt") + assert len(results) == 3 + + def test_no_match_returns_empty(self, indexed_root): + assert file_index.search(indexed_root, "*.doesnotexist") == [] + + def test_suffix_literal_with_space_does_not_raise(self, indexed_root): + # Regression: the FTS5 suffix-branch literal must be quoted, or a + # literal containing a space breaks the MATCH query. + results = file_index.search(indexed_root, "*archive report.pdf") + assert len(results) == 1 + + +class TestFindFilesMultiRoot: + def test_pipe_separated_roots_are_merged_and_deduped(self, tmp_path): + root_a = str(tmp_path / "a") + root_b = str(tmp_path / "b") + os.makedirs(root_a) + os.makedirs(root_b) + make_file(root_a, "one.txt") + make_file(root_b, "two.txt") + + result = file_index.find_files(f"{root_a}|{root_b}", "*.txt", recursive=True) + assert result["status"] == "success" + basenames = {os.path.basename(m) for m in result["matches"]} + assert basenames == {"one.txt", "two.txt"} + + def test_non_recursive_filters_to_top_level_only(self, tmp_path): + root = str(tmp_path) + make_file(root, "top.txt") + make_file(root, "sub", "nested.txt") + + result = file_index.find_files(root, "*.txt", recursive=False) + basenames = {os.path.basename(m) for m in result["matches"]} + assert basenames == {"top.txt"} + + def test_all_drives_uses_list_local_drives(self, tmp_path, monkeypatch): + root_a = str(tmp_path / "a") + root_b = str(tmp_path / "b") + os.makedirs(root_a) + os.makedirs(root_b) + make_file(root_a, "one.txt") + make_file(root_b, "two.txt") + monkeypatch.setattr( + file_index, "list_local_drives", lambda: [root_a, root_b] + ) + + result = file_index.find_files("", "*.txt", recursive=True, all_drives=True) + assert result["status"] == "success" + basenames = {os.path.basename(m) for m in result["matches"]} + assert basenames == {"one.txt", "two.txt"} + + def test_all_drives_with_no_drives_returns_error(self, monkeypatch): + monkeypatch.setattr(file_index, "list_local_drives", lambda: []) + result = file_index.find_files("", "*.txt", recursive=True, all_drives=True) + assert result["status"] == "error" + assert result["matches"] == [] + + +class TestBuildLockRegistry: + def test_same_root_returns_same_lock(self): + a = file_index._get_build_lock("C:/some/path") + b = file_index._get_build_lock("c:/SOME/path") + assert a is b + + def test_different_roots_return_different_locks(self): + a = file_index._get_build_lock("C:/path/one") + b = file_index._get_build_lock("C:/path/two") + assert a is not b + + +class TestWatcherIntegration: + """End-to-end: a real watchdog observer applies a real change without + re-walking the whole tree, and ignores changes inside skip-listed dirs.""" + + def test_real_change_is_applied_and_noise_is_ignored(self, tmp_path): + pytest.importorskip("watchdog") + root = str(tmp_path) + os.makedirs(os.path.join(root, "node_modules")) + file_index.build_index(root, force=True) + + started = file_index.start_watcher(root) + assert started + + try: + time.sleep(0.5) + make_file(root, "real.txt") + make_file(root, "node_modules", "noise.txt") + time.sleep(0.5) + + key = os.path.normcase(root) + pending = file_index._pending_changes.get(key, set()) + assert any(p.endswith("real.txt") for p in pending) + assert not any("node_modules" in p for p in pending) + + time.sleep(file_index._DEBOUNCE_SECONDS + 1.5) + + conn = sqlite3.connect(file_index._db_path(root)) + basenames = {r[0] for r in conn.execute("SELECT basename FROM files")} + assert "real.txt" in basenames + assert "noise.txt" not in basenames + finally: + watcher = file_index._watchers.get(os.path.normcase(root)) + if watcher and watcher._observer: + watcher._observer.stop() + watcher._observer.join(timeout=5) + + +class TestReconciliation: + """Regression tests for the self-healing gap: directory-level events and + fresh-process restarts must not be silently trusted as 'nothing changed'.""" + + def test_directory_level_event_forces_full_rewalk(self, tmp_path): + root = os.path.join(str(tmp_path), "orig_root") + os.makedirs(root) + make_file(root, "a.txt") + file_index.build_index(root, force=True) + key = os.path.normcase(root) + file_index._verified_roots.add(key) # simulate an already-checked root + + file_index._mark_needs_full_rewalk(root) + assert file_index._peek_has_pending_changes(root) + + # Rename the folder on disk without going through the (real, async) + # watcher — simulates the "single directory event, unknown + # descendants" scenario directly. Renamed within the same tmp_path + # (not renamed back) so pytest's own cleanup isn't affected by + # Windows file-lock timing on the SQLite WAL files. + renamed = os.path.join(str(tmp_path), "renamed_root") + os.rename(root, renamed) + + stats = file_index.build_index(renamed, force=False) + # A full re-walk of the renamed tree must have run (not a + # trusted-empty targeted apply), so the pre-existing file is + # correctly found under its new path. + assert stats.files_indexed >= 0 # ran a real check, didn't crash + conn = sqlite3.connect(file_index._db_path(renamed)) + try: + basenames = {r[0] for r in conn.execute("SELECT basename FROM files")} + assert "a.txt" in basenames + finally: + conn.close() + + def test_fresh_process_does_not_trust_empty_pending_as_fresh(self, tmp_path): + root = str(tmp_path) + make_file(root, "a.txt") + file_index.build_index(root, force=True) + key = os.path.normcase(root) + + # Simulate "restart": no _verified_roots entry, no pending changes, + # even though the on-disk index already has matching rows. + file_index._verified_roots.discard(key) + assert not file_index._peek_has_pending_changes(root) + + # A file changes on disk while "the process was down" (no watcher). + make_file(root, "b.txt") + + file_index.build_index(root, force=False) + conn = sqlite3.connect(file_index._db_path(root)) + basenames = {r[0] for r in conn.execute("SELECT basename FROM files")} + assert "b.txt" in basenames, ( + "first access after a simulated restart must do a real check, " + "not trust the cached index as fresh" + ) + + +class TestSymlinkHandling: + def test_symlink_is_not_indexed_via_targeted_changes(self, tmp_path): + root = str(tmp_path) + target = make_file(root, "target.txt") + link_path = os.path.join(root, "link.txt") + try: + os.symlink(target, link_path) + except (OSError, NotImplementedError): + pytest.skip("symlink creation not permitted in this environment") + + file_index.build_index(root, force=True) + conn = sqlite3.connect(file_index._db_path(root)) + # The full crawl never indexes symlinks (matches pre-existing + # _iter_files behavior). + assert conn.execute( + "SELECT 1 FROM files WHERE basename = 'link.txt'" + ).fetchone() is None + + file_index._apply_targeted_changes(conn, root, {link_path}) + # Targeted updates must agree with the full crawl about symlinks. + assert conn.execute( + "SELECT 1 FROM files WHERE basename = 'link.txt'" + ).fetchone() is None + + +class TestResolveRoots: + def test_pipe_only_base_directory_is_an_error(self): + roots, error = file_index.resolve_roots("|", windows=False) + assert roots == [] + assert error is not None + assert error["status"] == "error" + + def test_valid_multi_root_returns_normalized_paths(self, tmp_path): + a = str(tmp_path / "a") + b = str(tmp_path / "b") + os.makedirs(a) + os.makedirs(b) + roots, error = file_index.resolve_roots(f"{a}|{b}", windows=False) + assert error is None + assert len(roots) == 2 + + def test_nonexistent_root_is_an_error(self, tmp_path): + missing = str(tmp_path / "does_not_exist") + roots, error = file_index.resolve_roots(missing, windows=False) + assert roots == [] + assert error is not None + assert "does not exist" in error["message"] + + def test_empty_defaults_to_home_directory(self): + roots, error = file_index.resolve_roots("", windows=False) + assert error is None + assert len(roots) == 1 + + +class TestExpandedSkipList: + def test_build_and_dist_dirs_are_skipped(self, tmp_path): + root = str(tmp_path) + make_file(root, "real.txt") + make_file(root, "dist", "bundle.js") + make_file(root, "build", "output.o") + make_file(root, ".pytest_cache", "cache.txt") + + stats = file_index.build_index(root, force=True) + assert stats.files_indexed == 1 + + def test_skip_check_is_case_insensitive_cross_platform(self): + assert file_index._is_skip_path( + os.path.join("proj", "Node_Modules", "pkg.js") + ) + assert file_index._is_skip_path(os.path.join("proj", ".GIT", "HEAD")) + + +class TestFindFilesLimit: + def test_limit_caps_matches_across_multiple_roots(self, tmp_path): + root_a = str(tmp_path / "a") + root_b = str(tmp_path / "b") + os.makedirs(root_a) + os.makedirs(root_b) + for i in range(5): + make_file(root_a, f"a_{i}.txt") + make_file(root_b, f"b_{i}.txt") + + result = file_index.find_files( + f"{root_a}|{root_b}", "*.txt", recursive=True, limit=3 + ) + assert result["status"] == "success" + assert len(result["matches"]) == 3 + + def test_limit_zero_means_unlimited(self, tmp_path): + """Regression: limit=0 must not truncate to a single match — the + naive `len(matches) >= limit` check is trivially true at 0 the + instant the first match is appended.""" + root_a = str(tmp_path / "a") + root_b = str(tmp_path / "b") + os.makedirs(root_a) + os.makedirs(root_b) + for i in range(5): + make_file(root_a, f"a_{i}.txt") + make_file(root_b, f"b_{i}.txt") + + result = file_index.find_files( + f"{root_a}|{root_b}", "*.txt", recursive=True, limit=0 + ) + assert result["status"] == "success" + assert len(result["matches"]) == 10 + + def test_negative_limit_also_means_unlimited(self, tmp_path): + root = str(tmp_path) + for i in range(4): + make_file(root, f"f_{i}.txt") + + result = file_index.find_files(root, "*.txt", recursive=True, limit=-1) + assert len(result["matches"]) == 4 + + +class FakeEvent: + def __init__(self, src_path, is_directory, dest_path=None): + self.src_path = src_path + self.is_directory = is_directory + self.dest_path = dest_path + + +class TestDirectoryEventScope: + """Regression test: only a directory move/rename should force a full + re-walk. Directory create/delete must stay no-ops, or routine churn on + a busy drive (temp dirs, caches) reintroduces the full-rewalk treadmill + the targeted-changes mechanism exists to avoid.""" + + def _make_handler(self): + events = [] + handler = file_index._IndexEventHandler(lambda path, kind: events.append((path, kind))) + return handler, events + + def test_directory_created_is_a_noop(self): + handler, events = self._make_handler() + handler.on_created(FakeEvent("C:/root/newdir", is_directory=True)) + assert events == [] + + def test_directory_deleted_is_a_noop(self): + handler, events = self._make_handler() + handler.on_deleted(FakeEvent("C:/root/olddir", is_directory=True)) + assert events == [] + + def test_directory_modified_is_a_noop(self): + handler, events = self._make_handler() + handler.on_modified(FakeEvent("C:/root/somedir", is_directory=True)) + assert events == [] + + def test_directory_moved_triggers_dir_changed(self): + handler, events = self._make_handler() + handler.on_moved( + FakeEvent("C:/root/old_name", is_directory=True, dest_path="C:/root/new_name") + ) + assert events == [("C:/root/old_name", "dir_changed")] + + def test_file_events_are_unaffected(self): + handler, events = self._make_handler() + handler.on_created(FakeEvent("C:/root/a.txt", is_directory=False)) + handler.on_deleted(FakeEvent("C:/root/b.txt", is_directory=False)) + handler.on_modified(FakeEvent("C:/root/c.txt", is_directory=False)) + assert events == [ + ("C:/root/a.txt", "created"), + ("C:/root/b.txt", "deleted"), + ("C:/root/c.txt", "modified"), + ] + + def test_directory_create_delete_do_not_mark_needs_full_rewalk(self, tmp_path): + root = str(tmp_path) + make_file(root, "a.txt") + file_index.build_index(root, force=True) + key = os.path.normcase(root) + file_index._verified_roots.add(key) + + watcher = file_index._RootWatcher(root, file_index._DEBOUNCE_SECONDS) + handler = file_index._IndexEventHandler(watcher._on_change) + handler.on_created(FakeEvent(os.path.join(root, "newdir"), is_directory=True)) + handler.on_deleted(FakeEvent(os.path.join(root, "olddir"), is_directory=True)) + + assert not file_index._peek_has_pending_changes(root) + + +class TestCentralizedStorage: + """Regression tests: the index db must live under CraftBot's own + app-data directory, never inside the searched root itself.""" + + def test_index_lives_under_app_data_not_inside_searched_root(self, tmp_path): + root = str(tmp_path) + make_file(root, "a.txt") + file_index.build_index(root, force=True) + + # No .craftbot (or any other hidden index folder) inside the + # searched directory itself. + assert not any( + entry.name.lower() == ".craftbot" + for entry in os.scandir(root) + if entry.is_dir() + ) + + db_path = file_index._db_path(root) + assert os.path.commonpath( + [os.path.abspath(db_path), file_index._INDEX_STORAGE_ROOT] + ) == os.path.normpath(file_index._INDEX_STORAGE_ROOT) + assert os.path.isfile(db_path) + + def test_same_root_different_case_maps_to_same_folder(self, tmp_path): + root = str(tmp_path) + differently_cased = root.upper() if os.name == "nt" else root + assert file_index._index_dir(root) == file_index._index_dir(differently_cased) + + def test_different_roots_map_to_different_folders(self, tmp_path): + a = str(tmp_path / "a") + b = str(tmp_path / "b") + assert file_index._index_dir(a) != file_index._index_dir(b) + + def test_reindexing_same_root_reuses_existing_db(self, tmp_path): + root = str(tmp_path) + make_file(root, "a.txt") + file_index.build_index(root, force=True) + first_db_path = file_index._db_path(root) + + make_file(root, "b.txt") + file_index.build_index(root, force=True) + second_db_path = file_index._db_path(root) + + assert first_db_path == second_db_path + conn = sqlite3.connect(second_db_path) + basenames = {r[0] for r in conn.execute("SELECT basename FROM files")} + assert basenames == {"a.txt", "b.txt"} + + +class TestMountBoundary: + def test_does_not_descend_into_a_different_filesystem(self, tmp_path, monkeypatch): + root = str(tmp_path) + make_file(root, "a.txt") + other_fs_dir = os.path.join(root, "other_fs") + os.makedirs(other_fs_dir) + make_file(other_fs_dir, "should_not_be_indexed.txt") + + real_stat = os.stat + root_dev = real_stat(root).st_dev + normalized_other = os.path.normcase(other_fs_dir) + + def fake_stat(path, *args, **kwargs): + if os.path.normcase(path) == normalized_other: + return type("FakeStat", (), {"st_dev": root_dev + 1})() + return real_stat(path, *args, **kwargs) + + monkeypatch.setattr(file_index.os, "stat", fake_stat) + results = [entry.name for entry in file_index._iter_files(root)] + assert results == ["a.txt"] + + def test_root_itself_inaccessible_yields_nothing(self, monkeypatch): + def raise_oserror(path): + raise OSError("no such root") + + monkeypatch.setattr(file_index.os, "stat", raise_oserror) + assert list(file_index._iter_files("/does/not/matter")) == [] + + def test_trash_and_lost_and_found_are_skipped(self, tmp_path): + root = str(tmp_path) + make_file(root, "real.txt") + make_file(root, ".Trash", "deleted.txt") + make_file(root, "lost+found", "orphan.dat") + + stats = file_index.build_index(root, force=True) + assert stats.files_indexed == 1 diff --git a/tests/test_prompt_honesty_rules.py b/tests/test_prompt_honesty_rules.py index f1eafed5..912fdd1a 100644 --- a/tests/test_prompt_honesty_rules.py +++ b/tests/test_prompt_honesty_rules.py @@ -32,7 +32,7 @@ def test_communication_rules_hard_rule_present(): REPO_ROOT / "agent_file_system" / "AGENT.md", REPO_ROOT / "app" / "data" / "agent_file_system_template" / "AGENT.md", ): - assert EFFECT_MARKER in path.read_text() + assert EFFECT_MARKER in path.read_text(encoding="utf-8") def test_framework_prompts_carry_the_same_honesty_rule(): @@ -47,7 +47,9 @@ def test_framework_prompts_carry_the_same_honesty_rule(): def test_send_message_schema_reinforces_honesty(): - text = (REPO_ROOT / "app" / "data" / "action" / "send_message.py").read_text() + text = (REPO_ROOT / "app" / "data" / "action" / "send_message.py").read_text( + encoding="utf-8" + ) assert "there's no way I can do that" in text assert NO_RETRY_ESCALATION in text @@ -63,7 +65,7 @@ def test_gmail_integration_md_documents_no_signature_and_config_path(): / "integrations" / "gmail" / "INTEGRATION.md" - ).read_text() + ).read_text(encoding="utf-8") assert NO_SIGNATURE_MARKER in text assert NO_ESCALATION_MARKER in text assert "gmail_config.json" in text @@ -76,7 +78,7 @@ def test_outlook_integration_md_documents_no_signature(): / "integrations" / "outlook" / "INTEGRATION.md" - ).read_text() + ).read_text(encoding="utf-8") assert NO_SIGNATURE_MARKER in text assert NO_ESCALATION_MARKER in text @@ -86,7 +88,7 @@ def test_set_requirement_documented_with_grounding_rules(): REPO_ROOT / "agent_file_system" / "AGENT.md", REPO_ROOT / "app" / "data" / "agent_file_system_template" / "AGENT.md", ): - text = path.read_text() + text = path.read_text(encoding="utf-8") assert "set_requirement" in text assert "Before locking in a requirement, check it's actually achievable" in text assert "Marking a requirement `satisfied` requires the same grounding" in text