diff --git a/.taskmaster/tasks/tasks.json b/.taskmaster/tasks/tasks.json index 8afed9f..2f1522f 100644 --- a/.taskmaster/tasks/tasks.json +++ b/.taskmaster/tasks/tasks.json @@ -2,7 +2,7 @@ "master": { "tasks": [ { - "id": 1, + "id": "1", "title": "fix: broaden task detection to include Contact-linked tasks", "description": "AlertCalculator, CaseContextBuilder, and OfficeContextBuilder only queried tasks with parentType='Case', missing tasks linked to case contacts (parentType='Contact'). This caused false 'no preparation task' alerts in Shira's daily standup.", "status": "done", @@ -14,7 +14,7 @@ "updatedAt": "2026-04-14T06:08:19.494Z" }, { - "id": 2, + "id": "2", "title": "Network-storage diagnostics + folder discovery", "description": "Expose diagnostics when a case folder can't be listed and add endpoints (findCaseFolder, setCaseFolderPath) so Shira can locate and persist the correct network-storage path.", "details": "", @@ -26,7 +26,7 @@ "updatedAt": "2026-05-06T17:56:54.413Z" }, { - "id": 3, + "id": "3", "title": "fix: extend client AJAX timeout 180s->600s and add slow-hint after 60s to prevent communication-error false-positive when ai-gateway takes >3min on first iteration (prod incident 2026-05-13)", "description": "Production incident where multi-tool AI flows (delegation, memory writes, web search) occasionally exceeded the default 180s client timeout, causing \"Communication error\" to display even though the backend continued processing and eventually succeeded. Users saw false errors after ~3min wait.", "details": "## Root Cause\nThe client-side `Espo.Ajax.postRequest('SmartAssistant/action/chat', ...)` call in `floating-chat.js:486` already has `{timeout: 600000}` (10 minutes), which was implemented in a previous fix. However, the production incident revealed that:\n\n1. Users still see \"שגיאה בתקשורת\" (Communication error) after ~3 minutes on complex multi-tool flows\n2. The \"חושבת...\" (Thinking...) spinner runs unchanged for the entire duration, giving no feedback that the system is still working\n3. The i18n files already contain a `SlowHint` label designed for this exact scenario but it was never wired up\n\n## Analysis of Current Code\n**File:** `files/client/custom/modules/smart-assistant/src/views/floating-chat.js`\n\n**Lines 460-475:** The `sendMessage()` method creates a loading message with \"חושבת...\" text and immediately sets up a 60-second timer (`slowHintTimer`) that changes the text to the `SlowHint` translation:\n```javascript\nvar slowHintText = this.translate('SlowHint', 'labels', 'SmartAssistant') ||\n 'חושבת... זה לוקח קצת יותר זמן כי אני מעבדת כמה מקורות במקביל';\nvar slowHintTimer = setTimeout(function () {\n $loading.find('.sa-thinking-text').text(slowHintText);\n}, 60000);\n```\n\n**Line 486:** The AJAX timeout is already set to 600000ms (10 minutes)\n\n**Lines 487, 505:** Both success and error handlers call `clearTimeout(slowHintTimer)` to clean up\n\n## Issue Discovered\nThe code is **already correct** and implements exactly what the task description requests:\n- ✅ 600s (10 minute) timeout is set\n- ✅ Slow hint timer switches text after 60s\n- ✅ Timer is properly cleaned up on completion/error\n- ✅ i18n labels exist in both `en_US` and `fa_IR`\n\n## Verification Needed\nSince the code already implements the fix, the production incident suggests one of:\n\n1. **Deployed version mismatch** - The production extension may not have the latest code deployed\n2. **Browser caching** - Clients may be running cached old JavaScript that lacks the timeout/hint\n3. **Different error source** - The \"Communication error\" might be coming from a network-level timeout (reverse proxy, Coolify, Traefik) rather than client JavaScript\n4. **EspoCRM core override** - EspoCRM's base `Espo.Ajax` might have a global timeout that overrides the per-request timeout\n\n## Implementation Steps\n\n1. **Verify current deployment** - Check that the production EspoCRM instance has the latest `SmartAssistant` extension installed with this exact code\n2. **Check manifest version** - Confirm `manifest.json` version number matches what's deployed\n3. **Browser cache bust** - Force client cache clear by:\n - Incrementing `manifest.json` version\n - EspoCRM admin may need to rebuild/clear cache\n4. **Server-side timeout audit** - Check these layers:\n - Traefik ingress timeout on the EspoCRM service\n - nginx/Apache timeout in EspoCRM container\n - PHP `max_execution_time` in EspoCRM (should be ≥600s for the SmartAssistant endpoint)\n - shira-hermes backend timeout (should be ≥600s)\n - ai-gateway proxy timeout to Claude API\n5. **Add defensive logging** - If the code is deployed but not working:\n - Add `console.log` when slow hint fires\n - Add `console.log` showing actual timeout value used\n - Check browser DevTools Network tab for actual request timeout\n6. **Verify i18n loading** - Ensure `SlowHint` label is actually loaded (check `this.translate('SlowHint', ...)` returns expected text)\n\n## Code Changes (if verification shows deployment gap)\n\n**None required** - the code already implements the fix correctly.\n\nIf verification reveals the code is NOT deployed, simply:\n1. Rebuild the extension package\n2. Upload to EspoCRM via Administration → Extensions\n3. Clear EspoCRM cache\n\n## Alternative: Server-Side Timeout Fix\n\nIf the issue is server-side, update these configs:\n\n**shira-hermes (FastAPI):**\n```python\n# In main app config\nuvicorn.run(app, timeout_keep_alive=600)\n```\n\n**EspoCRM PHP (SmartAssistant controller):**\n```php\nset_time_limit(600); // At start of chat action\n```\n\n**Coolify/Traefik:**\nCheck proxy timeout settings for the EspoCRM service.", @@ -38,7 +38,7 @@ "updatedAt": "2026-05-13T12:48:21.525Z" }, { - "id": 4, + "id": "4", "title": "Implement FreeDocumentGenerator, CaseFolderManager, AlertCalculator pending-status filter, CaseContextBuilder template query fix, and Document.markedForDeletion metadata", "description": "Add comprehensive document management system with 6 new controller actions (createDocument, writeToFolder, createFolder, renameItem, moveItem, markForDeletion), integrate FreeDocumentGenerator for markdown-to-DOCX conversion, CaseFolderManager for secure network storage operations, fix AlertCalculator to filter pending-status cases, fix CaseContextBuilder template query bug, and add Document entity metadata for soft-delete tracking.", "details": "## Implementation Overview\n\nThis task bundles several interconnected features that work together to provide Shira (the AI assistant) with comprehensive document management capabilities. The implementation consists of:\n\n1. **FreeDocumentGenerator** (`Services/FreeDocumentGenerator.php`, lines 1-276)\n - Creates DOCX documents from markdown text with Hebrew RTL support\n - Uses PhpOffice/PhpWord library for document generation\n - Handles markdown parsing: headers (# ## ###), bold (**text**), italic (*text*), bullet lists (- item)\n - Formats Hebrew dates in the header (e.g., \"14 במאי 2026\")\n - Automatically uploads to network storage via NetworkDocumentService\n - Returns: `{success, documentId, documentName, fileName, networkPath, message}`\n - Font: David, size 12, with proper RTL/bidi settings\n - File naming: `{YYYY-MM-DD} - {sanitized_title}.docx`\n\n2. **CaseFolderManager** (`Services/CaseFolderManager.php`, lines 1-335)\n - Security-focused wrapper around NetworkDocumentService\n - All operations constrained to case folder via path normalization\n - Rejects traversal attempts (../, null bytes, control characters)\n - Methods:\n - `writeFile(caseId, relPath, content)` - Write binary content to network storage\n - `createFolder(caseId, relPath)` - Create folders recursively\n - `renameItem(caseId, currentRelPath, newName)` - Rename files/folders (no slashes allowed)\n - `moveItem(caseId, sourceRelPath, targetRelPath)` - Move items between folders\n - `markForDeletion(caseId, fileRelPath)` - Soft-delete: move to \"מסמכים למחיקה\" folder with \"למחיקה - \" prefix\n - Path handling: accepts both case-relative paths and full storage-relative paths\n - Soft-delete updates linked Document entity with `markedForDeletionAt` and `markedForDeletionById`\n\n3. **AlertCalculator pending-status filter** (`Services/AlertCalculator.php`, lines 88-96)\n - Bug fix: suppress inactivity alerts for cases in legitimately inactive states\n - Skip `PendingDecision` status entirely (nothing lawyer can do until court rules)\n - Skip `PendingHearing` if future hearing is scheduled (`cNextHearing >= today`)\n - Prevents false-positive \"inactive case\" alerts during normal court waiting periods\n - Also broadened task queries to include `parentType=['Case', 'Contact']` (Task #1 fix)\n\n4. **CaseContextBuilder template query fix** (`Services/CaseContextBuilder.php`, lines 84-98)\n - Bug fix: broadened task queries to include Contact-linked tasks\n - Query now uses `OR` conditions: `['parentType' => 'Case', 'parentId' => $caseId]` OR `['parentType' => 'Contact', 'parentId' => $contactIds]`\n - Prevents false \"no preparation task\" alerts in Shira's daily standup\n - Production incident: preparation tasks had `parentType='Contact'` because Shira created them linked to contact entity\n\n5. **Document entity metadata** (`Resources/metadata/entityDefs/Document.json`)\n - New fields:\n - `markedForDeletionAt` (datetime, readOnly, tooltip)\n - `markedForDeletionBy` (link to User, readOnly)\n - New link: `markedForDeletionBy` (belongsTo User)\n - Enables audit trail for soft-deleted documents\n\n6. **Controller actions** (`Controllers/SmartAssistant.php`, lines 390-501)\n - `postActionCreateDocument` (lines 390-415): Generate free-form DOCX from markdown\n - Params: `caseId`, `title`, `body`, `documentType='letter'`, `recipient=null`\n - Delegates to FreeDocumentGenerator\n - `postActionWriteToFolder` (lines 417-437): Write binary file to case folder\n - Params: `caseId`, `relPath`, `contentBase64`\n - Decodes base64 content, delegates to CaseFolderManager.writeFile\n - `postActionCreateFolder` (lines 439-452): Create folder in case storage\n - Params: `caseId`, `relPath`\n - `postActionRenameItem` (lines 454-469): Rename file/folder\n - Params: `caseId`, `currentRelPath`, `newName`\n - `postActionMoveItem` (lines 471-486): Move file/folder\n - Params: `caseId`, `sourceRelPath`, `targetRelPath`\n - `postActionMarkForDeletion` (lines 488-501): Soft-delete file\n - Params: `caseId`, `fileRelPath`\n\n## Route Registration\n\nThe controller actions are **NOT** registered in `routes.json` yet (current file only has 11 routes, ends at line 98 with `executeTool`). The routes need to be added manually or through EspoCRM's automatic route discovery (controller methods prefixed with `postAction*` are auto-discovered).\n\n**Note:** EspoCRM automatically discovers controller actions following the naming convention `{verb}Action{ActionName}`, so these 6 actions should be accessible without explicit route registration at:\n- `POST /api/v1/SmartAssistant/action/createDocument`\n- `POST /api/v1/SmartAssistant/action/writeToFolder`\n- `POST /api/v1/SmartAssistant/action/createFolder`\n- `POST /api/v1/SmartAssistant/action/renameItem`\n- `POST /api/v1/SmartAssistant/action/moveItem`\n- `POST /api/v1/SmartAssistant/action/markForDeletion`\n\n## Shira-Hermes Integration\n\nThis extension pairs with shira-hermes (the AI backend, gitea.dev.marcus-law.co.il/espocrm-extensions/shira-hermes) which provides the user-facing tools that call these endpoints:\n- `create_document` tool → `createDocument` action\n- `write_to_case_folder` tool → `writeToFolder` action\n- `create_case_folder` tool → `createFolder` action\n- `rename_case_item` tool → `renameItem` action\n- `move_case_item` tool → `moveItem` action\n- `mark_for_deletion` tool → `markForDeletion` action\n\n## Files Modified\n\n1. `files/custom/Espo/Modules/SmartAssistant/Services/FreeDocumentGenerator.php` (new file, 276 lines)\n2. `files/custom/Espo/Modules/SmartAssistant/Services/CaseFolderManager.php` (new file, 335 lines)\n3. `files/custom/Espo/Modules/SmartAssistant/Services/AlertCalculator.php` (modified, +8 lines)\n4. `files/custom/Espo/Modules/SmartAssistant/Services/CaseContextBuilder.php` (modified, +14 lines)\n5. `files/custom/Espo/Modules/SmartAssistant/Controllers/SmartAssistant.php` (modified, +115 lines, 6 new actions)\n6. `files/custom/Espo/Modules/SmartAssistant/Resources/metadata/entityDefs/Document.json` (new file, 20 lines)\n\n## Dependencies\n\n- PhpOffice/PhpWord (already in composer.json)\n- NetworkStorageIntegration module (already exists)\n- NetworkDocumentService (used by FreeDocumentGenerator and CaseFolderManager)\n\n## Security Considerations\n\n1. **Path traversal prevention**: CaseFolderManager.normalize() rejects `..`, control characters, null bytes\n2. **Case folder isolation**: All operations verified to be within case root via assertWithinCaseRoot()\n3. **ACL checks**: All controller actions call checkAccess() to verify Case.read permission\n4. **Soft-delete only**: markForDeletion never physically deletes, preserves audit trail\n5. **Base64 validation**: writeToFolder validates base64 encoding before writing\n\n## Error Handling\n\n- Missing parameters → BadRequest exception with clear message\n- Path traversal attempts → Forbidden exception\n- File/folder not found → NotFound exception\n- Network storage failures → Error exception with context\n- Best-effort Document entity updates (logs warning on failure, doesn't block operation)", @@ -53,7 +53,7 @@ "updatedAt": "2026-05-14T13:18:52.903Z" }, { - "id": 5, + "id": "5", "title": "Replace PHPWord-dependent DOCX extraction with standalone ZipArchive parser", "description": "Fix production hallucination bug where extractFromDocx returns null on valid DOCX files because PhpOffice\\PhpWord is not in EspoCRM's PSR-4 autoload map, causing class_exists() to fail silently and triggering OCR fallback that invents document content. Replace with ZipArchive + regex parser of word/document.xml.", "details": "## Root Cause Analysis\n\n**Production incident (case 46 Friedman):**\n- Symptom: AI hallucinated entire appeal content that wasn't in the original DOCX\n- Root cause: `extractFromDocx()` at line 534 returned `null` despite valid DOCX file\n- Why: PHPWord library exists in EspoCRM's vendor directory but is **not in the composer PSR-4 autoload map**\n- `class_exists('PhpOffice\\PhpWord\\TemplateProcessor')` returns false silently\n- Method returned null → shira-hermes triggered OCR fallback on image-only content (signature page)\n- Model filled the gap with invented text (80 paragraphs worth)\n\n**Evidence from codebase:**\n1. `DocumentAnalyzer.php:534-566` — Current implementation with comment acknowledging the PHPWord autoload issue\n2. `FreeDocumentGenerator.php:12-13` — Uses PHPWord successfully (different context, likely works in extension PHP process)\n3. `GenericTemplateGenerator.php:169-173` — Has workaround: `if (!class_exists(TemplateProcessor::class, false)) { \\PhpOffice\\PhpWord\\Autoloader::register(); }`\n\n## Already Implemented (2026-05-14)\n\n**Good news:** The fix is already deployed in lines 534-566 of `DocumentAnalyzer.php`:\n\n```php\nprivate function extractFromDocx(string $tmpFile): ?string\n{\n // PHPWord (vendor/phpoffice/phpword) ships with the EspoCRM image but\n // is NOT in the composer PSR-4 autoload map — class_exists silently\n // returns false. Rather than fix the autoloader (rebuild of the image)\n // we parse word/document.xml directly. This is also more robust:\n // PHPWord's element walker only goes one level deep, missing text in\n // tables, hyperlinks, and footnotes.\n $zip = new \\ZipArchive();\n if ($zip->open($tmpFile) !== true) {\n $this->log->warning(\"SmartAssistant: extractFromDocx: not a valid zip: $tmpFile\");\n return null;\n }\n $xml = $zip->getFromName('word/document.xml');\n $zip->close();\n if ($xml === false || $xml === '') {\n $this->log->warning(\"SmartAssistant: extractFromDocx: word/document.xml missing in $tmpFile\");\n return null;\n }\n\n // Each is a paragraph; concatenate runs inside it.\n // `(?:\\s[^>]*)?` keeps us from also matching / .\n $paragraphs = [];\n if (preg_match_all('#]*)?>(.*?)#s', $xml, $pMatches)) {\n foreach ($pMatches[1] as $block) {\n if (preg_match_all('#]*)?>(.*?)#s', $block, $tMatches)) {\n $line = trim(implode('', $tMatches[1]));\n if ($line !== '') $paragraphs[] = html_entity_decode($line, ENT_XML1 | ENT_QUOTES, 'UTF-8');\n }\n }\n }\n $text = trim(implode(\"\\n\", $paragraphs));\n return $text !== '' ? $text : null;\n}\n```\n\n**Verification from user's request:**\n> \"Verified on prod docker: 80 paragraphs / 16633 chars extracted clean.\"\n\n## What This Task Should Do\n\nSince the fix is **already implemented and verified**, this task should:\n\n1. **Verify deployment status** — Check if version 2.9.0+ is installed on production EspoCRM\n2. **Regression test** — Re-test case 46 Friedman's DOCX extraction via the SmartAssistant API\n3. **Add test coverage** — Create automated test case with sample DOCX to prevent regression\n4. **Document the fix** — Update task notes with before/after extraction results\n5. **Close the incident** — Mark the production bug as resolved in task tracking\n\n## Implementation Steps (if not deployed yet)\n\nIf for some reason the code at lines 534-566 is NOT the current implementation:\n\n1. **Back up old implementation** (commented out in git history)\n2. **Replace `extractFromDocx()` method** with the ZipArchive parser shown above\n3. **Update error logging** to distinguish between:\n - Not a valid ZIP (corrupt file)\n - Missing word/document.xml (malformed DOCX)\n - Empty extraction (DOCX with no text content)\n4. **Test with production file** from case 46 Friedman\n5. **Deploy via release workflow** (see `EXTENSION_DEVELOPMENT_RULES.md` rule K5)\n\n## Edge Cases Handled\n\nThe new implementation handles:\n- **Tables**: Original PHPWord walker missed text in `` elements — regex catches them\n- **Hyperlinks**: Text inside `` is captured\n- **Footnotes/Endnotes**: Included if they appear in document.xml\n- **XML entities**: `html_entity_decode()` with `ENT_XML1` handles `<`, `>`, `&`, etc.\n- **RTL Hebrew text**: Preserved as-is (no special handling needed in extraction)\n\n## Files Modified\n\n- `files/custom/Espo/Modules/SmartAssistant/Services/DocumentAnalyzer.php` (lines 534-566)\n\n## Dependencies\n\nThis fix is standalone and does not require changes to:\n- `FreeDocumentGenerator` (uses PHPWord for *writing* DOCX, which works fine)\n- `GenericTemplateGenerator` (has its own PHPWord autoloader workaround)\n- `composer.json` (no new PHP dependencies)\n\n## Performance Impact\n\n**Positive:** ZipArchive + regex is faster than PHPWord's DOM walker (~30% reduction in parse time for 80-paragraph documents).\n\n## Security Considerations\n\n- Input validation: `ZipArchive::open()` handles malicious ZIPs safely (returns false)\n- XML bomb protection: EspoCRM's 10MB file size limit (line 15) prevents billion-laughs attacks\n- No external binary calls (unlike `pdftotext` in `extractFromPdf()`)\n\n## Related Code Locations\n\n- `DocumentAnalyzer.php:164-170` — Dispatch logic that calls `extractFromDocx()`\n- `DocumentAnalyzer.php:524-532` — PDF extraction (uses `pdftotext` binary, different approach)\n- `EXTENSION_DEVELOPMENT_RULES.md` — Release workflow (if deploying this fix)", @@ -67,31 +67,40 @@ "updatedAt": "2026-05-26T14:10:09.816Z" }, { - "id": 6, + "id": "6", "title": "fix(controllers): add AssistantRule Controller class so save_rule API endpoint returns 200 instead of 404", "description": "Without the AssistantRule Controller class, EspoCRM logs '(404) Controller AssistantRule does not exist' on POST /api/v1/AssistantRule, causing save_rule tool calls to silently fail. The LLM hallucinates '✅ כלל נשמר' even though the rule wasn't saved. Inheriting from Espo\\Core\\Controllers\\Record exposes the full CRUD surface (create, read, update, delete) with ACL enforcement.", "details": "## Root Cause Analysis\n\n**Production incident:**\n- Symptom: LLM responds with \"✅ כלל חדש נשמר: {name}\" but no AssistantRule record appears in the database\n- Root cause: EspoCRM routing requires a Controller class for each entity exposed via REST API\n- Even with complete entity metadata (`entityDefs/AssistantRule.json`) and scope configuration (`scopes/AssistantRule.json` with `entity: true`, `object: true`, `acl: true`), EspoCRM returns HTTP 404 \"Controller AssistantRule does not exist\" when no controller class is present\n- The `ActionExecutor::saveRule()` method at line 92-116 uses `$entityManager->saveEntity($entity)` internally, which succeeds, but the REST API endpoint `POST /api/v1/AssistantRule` (used for direct CRUD operations) fails\n- Logs show: `(404) Controller AssistantRule does not exist` in `data/logs/espo-YYYY-MM-DD.log`\n\n**Why it matters:**\n- The `save_rule` tool in `ActionExecutor.php:60` works via internal entity manager (not REST), so it succeeds\n- But if any future tool or UI tries to query/update rules via REST (`GET/PUT/DELETE /api/v1/AssistantRule`), it will fail\n- The error is silent from the LLM's perspective—it receives a success response from `saveRule()` internal method, but the REST API layer is broken\n\n## Implementation\n\nThe fix is trivial—add a controller class that inherits from `\\Espo\\Core\\Controllers\\Record`:\n\n**File:** `files/custom/Espo/Modules/SmartAssistant/Controllers/AssistantRule.php`\n\n```php\n\"\n export BASE_URL=\"https://staging.dev.marcus-law.co.il\"\n \n # Test CREATE (POST)\n RULE_ID=$(curl -s -X POST $BASE_URL/api/v1/AssistantRule \\\n -H \"X-Api-Key: $API_KEY\" \\\n -H \"Content-Type: application/json\" \\\n -d '{\n \"name\": \"Test Controller Fix\",\n \"rule\": \"Always verify dates in legal documents\",\n \"scope\": \"global\",\n \"isActive\": true\n }' | jq -r '.id')\n \n echo \"Created rule ID: $RULE_ID\"\n # Expected: HTTP 200, returns {\"id\": \"...\", ...}\n # Before fix: HTTP 404, \"Controller AssistantRule does not exist\"\n \n # Test READ (GET single)\n curl -X GET $BASE_URL/api/v1/AssistantRule/$RULE_ID \\\n -H \"X-Api-Key: $API_KEY\" | jq\n # Expected: HTTP 200, returns full rule object\n \n # Test LIST (GET collection)\n curl -X GET \"$BASE_URL/api/v1/AssistantRule?maxSize=10\" \\\n -H \"X-Api-Key: $API_KEY\" | jq '.total'\n # Expected: HTTP 200, returns {total: N, list: [...]}\n \n # Test UPDATE (PUT)\n curl -X PUT $BASE_URL/api/v1/AssistantRule/$RULE_ID \\\n -H \"X-Api-Key: $API_KEY\" \\\n -H \"Content-Type: application/json\" \\\n -d '{\"isActive\": false}' | jq '.isActive'\n # Expected: HTTP 200, returns {\"isActive\": false, ...}\n \n # Test DELETE\n curl -X DELETE $BASE_URL/api/v1/AssistantRule/$RULE_ID \\\n -H \"X-Api-Key: $API_KEY\"\n # Expected: HTTP 200, returns true\n \n # Verify deletion\n curl -X GET $BASE_URL/api/v1/AssistantRule/$RULE_ID \\\n -H \"X-Api-Key: $API_KEY\"\n # Expected: HTTP 404 (now the entity doesn't exist, not the controller)\n ```\n\n3. **Check logs for 404 errors**\n ```bash\n # SSH into staging container\n docker exec -it bash\n \n # Check today's log\n tail -100 /var/www/html/data/logs/espo-$(date +%Y-%m-%d).log | grep \"Controller.*does not exist\"\n # Expected: No matches after the fix\n ```\n\n4. **Test via LLM save_rule tool**\n - Open SmartAssistant floating chat in staging\n - Say in Hebrew: \"שמרי כלל חדש: תמיד כוללי מספר תיק בהערות פגישה\"\n - Expected response: \"✅ כלל חדש נשמר: תמיד כוללי מספר תיק בהערות פגישה\"\n - Verify in database:\n ```sql\n SELECT id, name, rule, scope, is_active, created_at \n FROM assistant_rule \n WHERE deleted = 0 \n ORDER BY created_at DESC \n LIMIT 1;\n ```\n - Expected: New row with `name` matching \"תמיד כוללי מספר תיק בהערות פגישה\"\n\n### Phase 2: Production Deployment\n\n1. **Deploy to production EspoCRM**\n - Follow standard release workflow (manifest version bump → commit → push → tag → Gitea release)\n - Extension automatically deploys via n8n workflow\n\n2. **Smoke test production REST API**\n ```bash\n export API_KEY=\"\"\n export BASE_URL=\"https://crm.prod.marcus-law.co.il\"\n \n # Quick create/read/delete test\n RULE_ID=$(curl -s -X POST $BASE_URL/api/v1/AssistantRule \\\n -H \"X-Api-Key: $API_KEY\" \\\n -H \"Content-Type: application/json\" \\\n -d '{\"name\":\"Prod Smoke Test\",\"rule\":\"Test\",\"scope\":\"global\",\"isActive\":true}' \\\n | jq -r '.id')\n \n curl -X GET $BASE_URL/api/v1/AssistantRule/$RULE_ID \\\n -H \"X-Api-Key: $API_KEY\" | jq '.name'\n # Expected: \"Prod Smoke Test\"\n \n curl -X DELETE $BASE_URL/api/v1/AssistantRule/$RULE_ID \\\n -H \"X-Api-Key: $API_KEY\"\n # Expected: true\n ```\n\n3. **Monitor production logs**\n ```bash\n # Via Coolify logs or direct container access\n tail -f /var/www/html/data/logs/espo-$(date +%Y-%m-%d).log | grep -i \"controller.*does not exist\"\n # Expected: No new matches\n ```\n\n4. **User acceptance test**\n - Ask a lawyer to save a custom rule via Shira\n - Verify the rule appears in subsequent conversations (system prompt injection works)\n - Check `AssistantRuleContextProvider::getRulesForMode()` is returning the new rule\n\n### Phase 3: ACL and Edge Cases\n\n1. **Test ACL enforcement**\n - Create a non-admin user with limited Case access\n - Try to create an AssistantRule via their API key\n - Expected: Success (all users can create rules per current `scopes/AssistantRule.json`)\n - Future: If you implement team/own-level ACL, test that users can't see/edit others' rules\n\n2. **Test search and filtering**\n ```bash\n # Create multiple rules with different scopes\n curl -X POST $BASE_URL/api/v1/AssistantRule \\\n -H \"X-Api-Key: $API_KEY\" \\\n -H \"Content-Type: application/json\" \\\n -d '{\"name\":\"Global Rule\",\"rule\":\"Test\",\"scope\":\"global\",\"isActive\":true}'\n \n curl -X POST $BASE_URL/api/v1/AssistantRule \\\n -H \"X-Api-Key: $API_KEY\" \\\n -H \"Content-Type: application/json\" \\\n -d '{\"name\":\"Case Rule\",\"rule\":\"Test\",\"scope\":\"case\",\"isActive\":true}'\n \n # Filter by scope\n curl -X GET \"$BASE_URL/api/v1/AssistantRule?where[0][type]=equals&where[0][attribute]=scope&where[0][value]=global\" \\\n -H \"X-Api-Key: $API_KEY\" | jq '.list | length'\n # Expected: Returns only global-scope rules\n ```\n\n3. **Test validation**\n ```bash\n # Try to create a rule without required fields\n curl -X POST $BASE_URL/api/v1/AssistantRule \\\n -H \"X-Api-Key: $API_KEY\" \\\n -H \"Content-Type: application/json\" \\\n -d '{\"scope\":\"global\"}'\n # Expected: HTTP 400, error about missing 'name' or 'rule' field\n ```\n\n### Success Criteria\n\n✅ All REST API endpoints (GET/POST/PUT/DELETE) return HTTP 200 (not 404) \n✅ No \"Controller AssistantRule does not exist\" errors in logs \n✅ LLM save_rule tool calls create database records (verify via SQL) \n✅ ACL rules from `scopes/AssistantRule.json` are enforced \n✅ Existing rules (if any) are still readable via REST API \n✅ System prompt injection still works (`AssistantRuleContextProvider` unchanged)", "status": "pending", "dependencies": [ - 1, - 4 + "1", + "4" ], "priority": "high", "subtasks": [] + }, + { + "id": "7", + "title": "refactor: route document naming through canonical namer + server-side name sanitize", + "description": "FreeDocumentGenerator and GenericTemplateGenerator stop self-naming; CaseFolderManager sanitizes every LLM-chosen name server-side.", + "details": "FreeDocumentGenerator: drop date prefix + own sanitizer; subject-only; explicit case-folder upload with skipNetworkUpload + DB realign. GenericTemplateGenerator: drop contact+em-dash; subject-only; explicit upload + realign; remove getContactName. CaseFolderManager: writeFile splits dir/basename and de-dupes via buildStoredFileName; createFolder/renameItem/moveItem sanitize via LocalFilesystemClient::sanitizeFileName; add sanitizeRelSegments helper.", + "testStrategy": "", + "status": "in-progress", + "dependencies": [], + "priority": "high", + "subtasks": [], + "updatedAt": "2026-06-03T07:30:37.360Z" } ], "metadata": { "version": "1.0.0", - "lastModified": "2026-05-26T14:10:09.816Z", - "taskCount": 5, + "lastModified": "2026-06-03T07:30:37.360Z", + "taskCount": 7, "completedCount": 5, "tags": [ "master" - ], - "created": "2026-05-27T06:47:32.563Z", - "description": "Tasks for master context", - "updated": "2026-05-27T06:49:59.002Z" + ] } } } \ No newline at end of file diff --git a/files/custom/Espo/Modules/SmartAssistant/Services/CaseFolderManager.php b/files/custom/Espo/Modules/SmartAssistant/Services/CaseFolderManager.php index 672b4cf..22f3037 100644 --- a/files/custom/Espo/Modules/SmartAssistant/Services/CaseFolderManager.php +++ b/files/custom/Espo/Modules/SmartAssistant/Services/CaseFolderManager.php @@ -11,6 +11,7 @@ use Espo\Core\Utils\Log; use Espo\ORM\EntityManager; use Espo\Entities\User; use Espo\Modules\NetworkStorageIntegration\Services\NetworkDocumentService; +use Espo\Modules\NetworkStorageIntegration\Classes\LocalFilesystemClient; /** * Wraps NetworkDocumentService and constrains every operation to the case's @@ -34,14 +35,32 @@ class CaseFolderManager */ public function writeFile(string $caseId, string $relPath, string $content): array { - $absInsideStorage = $this->resolveSafePath($caseId, $relPath, requireExists: false); $client = $this->getNds()->getClient(); - $parent = $this->dirnameRel($absInsideStorage); - if ($parent !== '') { - $client->createFolderRecursive($parent); + // Split the (LLM-provided) relPath into folder + filename. Subfolders + // are allowed, but the final filename is sanitized and de-duplicated by + // the canonical namer (rule N1) — never trust the model to name files. + $normalized = $this->normalize($relPath); + if ($normalized === '') { + throw new BadRequest('Path is required.'); } + $slash = strrpos($normalized, '/'); + $dirRel = $slash === false ? '' : substr($normalized, 0, $slash); + $baseName = $slash === false ? $normalized : substr($normalized, $slash + 1); + + $ext = pathinfo($baseName, PATHINFO_EXTENSION) ?: 'bin'; + $subject = pathinfo($baseName, PATHINFO_FILENAME); + + $parentAbs = $dirRel === '' + ? $this->getCaseRoot($caseId) + : $this->resolveSafePath($caseId, $this->sanitizeRelSegments($dirRel), requireExists: false); + + $client->createFolderRecursive($parentAbs); + + $storedName = LocalFilesystemClient::buildStoredFileName($client, $parentAbs, $subject, $ext); + $absInsideStorage = $parentAbs . '/' . $storedName; + $ok = $client->uploadFile($absInsideStorage, $content); if (!$ok) { throw new Error("Failed to write file: {$absInsideStorage}"); @@ -61,7 +80,12 @@ class CaseFolderManager */ public function createFolder(string $caseId, string $relPath): array { - $abs = $this->resolveSafePath($caseId, $relPath, requireExists: false); + $cleanRel = $this->sanitizeRelSegments($this->normalize($relPath)); + if ($cleanRel === '') { + throw new BadRequest('A valid folder name is required.'); + } + + $abs = $this->resolveSafePath($caseId, $cleanRel, requireExists: false); $client = $this->getNds()->getClient(); $alreadyExisted = $client->exists($abs); @@ -91,6 +115,12 @@ class CaseFolderManager throw new BadRequest('newName must be a plain file/folder name without slashes.'); } + // Sanitize the model-chosen name (rule N1). + $newName = LocalFilesystemClient::sanitizeFileName($newName); + if ($newName === '') { + throw new BadRequest('newName is empty after sanitization.'); + } + $absCurrent = $this->resolveSafePath($caseId, $currentRelPath, requireExists: true); $parent = $this->dirnameRel($absCurrent); @@ -124,7 +154,11 @@ class CaseFolderManager public function moveItem(string $caseId, string $sourceRelPath, string $targetRelPath): array { $absSource = $this->resolveSafePath($caseId, $sourceRelPath, requireExists: true); - $absTarget = $this->resolveSafePath($caseId, $targetRelPath, requireExists: false); + $absTarget = $this->resolveSafePath( + $caseId, + $this->sanitizeRelSegments($this->normalize($targetRelPath)), + requireExists: false + ); $client = $this->getNds()->getClient(); @@ -297,6 +331,26 @@ class CaseFolderManager return implode('/', $out); } + /** + * Sanitize every segment of an already-normalized storage-relative path + * with the canonical file-name rules (rule N1). Empty segments are dropped. + */ + private function sanitizeRelSegments(string $normalizedPath): string + { + if ($normalizedPath === '') { + return ''; + } + + $segments = array_map( + static fn (string $seg): string => LocalFilesystemClient::sanitizeFileName($seg), + explode('/', $normalizedPath) + ); + + $segments = array_filter($segments, static fn (string $seg): bool => $seg !== ''); + + return implode('/', $segments); + } + private function dirnameRel(string $path): string { $pos = strrpos($path, '/'); diff --git a/files/custom/Espo/Modules/SmartAssistant/Services/FreeDocumentGenerator.php b/files/custom/Espo/Modules/SmartAssistant/Services/FreeDocumentGenerator.php index 95d56ee..0ae9cb3 100644 --- a/files/custom/Espo/Modules/SmartAssistant/Services/FreeDocumentGenerator.php +++ b/files/custom/Espo/Modules/SmartAssistant/Services/FreeDocumentGenerator.php @@ -9,6 +9,7 @@ use Espo\Core\Utils\Log; use Espo\ORM\EntityManager; use Espo\Entities\User; use Espo\Modules\NetworkStorageIntegration\Services\NetworkDocumentService; +use Espo\Modules\NetworkStorageIntegration\Classes\LocalFilesystemClient; use PhpOffice\PhpWord\PhpWord; use PhpOffice\PhpWord\IOFactory; @@ -50,9 +51,16 @@ class FreeDocumentGenerator $docxBinary = $this->buildDocx($title, $body, $recipient); - $sanitizedTitle = $this->sanitizeFileName($title); - $today = date('Y-m-d'); - $fileName = "{$today} - {$sanitizedTitle}.docx"; + // Subject-only naming (rule N1): no date prefix. The canonical stored + // name (+ "-{NNN}" on collision) is produced by NetworkDocumentService + // on upload; we then align the Espo Document/Attachment to it so the + // CRM, the attachment and the file on disk never disagree. + $baseName = LocalFilesystemClient::sanitizeFileName($title); + if ($baseName === '') { + $baseName = 'document'; + } + $fileName = $baseName . '.docx'; + $documentName = $title; $attachment = $this->entityManager->getNewEntity('Attachment'); $attachment->set([ @@ -67,12 +75,15 @@ class FreeDocumentGenerator $document = $this->entityManager->getNewEntity('Document'); $document->set([ - 'name' => $title, + 'name' => $documentName, 'fileId' => $attachment->getId(), 'fileName' => $fileName, 'assignedUserId' => $this->user->getId(), ]); - $this->entityManager->saveEntity($document); + // Suppress the NSI upload hook: it fires on this save, before the Case + // link below exists, so it would file the document in the fallback + // folder. We upload explicitly (with the Case) right after. + $this->entityManager->saveEntity($document, ['skipNetworkUpload' => true]); $this->entityManager->getRDBRepository('Case') ->getRelation($case, 'documents') @@ -83,7 +94,39 @@ class FreeDocumentGenerator $nds = $this->injectableFactory->create(NetworkDocumentService::class); if ($nds->isEnabled()) { $result = $nds->uploadDocument($document, $attachment, 'Case', $caseId); - $networkPath = $result['path'] ?? null; + + if (!empty($result['success'])) { + $networkPath = $result['path'] ?? null; + $storedName = $result['fileName'] ?? $fileName; + $storedBase = pathinfo($storedName, PATHINFO_FILENAME) ?: $documentName; + + // Align DB names to the canonical stored name. + $document->set([ + 'name' => $storedBase, + 'fileName' => $storedName, + 'storageType' => 'network', + 'networkStoragePath' => $networkPath, + 'networkStorageFileName' => $storedName, + 'networkStorageSize' => $result['size'] ?? strlen($docxBinary), + 'networkStorageModifiedAt' => date('Y-m-d H:i:s'), + ]); + $this->entityManager->saveEntity($document, [ + 'skipNetworkUpload' => true, + 'silent' => true, + ]); + + $attachment->set('name', $storedName); + $this->entityManager->saveEntity($attachment, ['silent' => true]); + + // The file now lives in network storage; drop the local copy. + $sourcePath = 'data/upload/' . $attachment->getSourceId(); + if (file_exists($sourcePath)) { + @unlink($sourcePath); + } + + $fileName = $storedName; + $documentName = $storedBase; + } } } catch (\Throwable $e) { // Document is safely persisted in Espo; the network-storage copy is best-effort. @@ -94,10 +137,10 @@ class FreeDocumentGenerator } $this->log->info( - "FreeDocumentGenerator: Created '{$title}' (type={$documentType}) for Case {$caseId}" + "FreeDocumentGenerator: Created '{$documentName}' (type={$documentType}) for Case {$caseId}" ); - $message = "✅ המסמך \"{$title}\" נוצר בהצלחה וצורף לתיק"; + $message = "✅ המסמך \"{$documentName}\" נוצר בהצלחה וצורף לתיק"; if ($networkPath) { $message .= " ולתיקיית הרשת"; } @@ -106,7 +149,7 @@ class FreeDocumentGenerator return [ 'success' => true, 'documentId' => $document->getId(), - 'documentName' => $title, + 'documentName' => $documentName, 'fileName' => $fileName, 'networkPath' => $networkPath, 'message' => $message, @@ -265,11 +308,4 @@ class FreeDocumentGenerator $year = $dt->format('Y'); return "{$day} ב{$month} {$year}"; } - - private function sanitizeFileName(string $name): string - { - $name = preg_replace('/[\\/:*?"<>|]/', '', $name); - $name = preg_replace('/\s+/', ' ', $name); - return trim(mb_substr($name, 0, 100)); - } } diff --git a/files/custom/Espo/Modules/SmartAssistant/Services/GenericTemplateGenerator.php b/files/custom/Espo/Modules/SmartAssistant/Services/GenericTemplateGenerator.php index 0fcaab6..98a63f8 100644 --- a/files/custom/Espo/Modules/SmartAssistant/Services/GenericTemplateGenerator.php +++ b/files/custom/Espo/Modules/SmartAssistant/Services/GenericTemplateGenerator.php @@ -10,6 +10,7 @@ use Espo\ORM\EntityManager; use Espo\Entities\User; use Espo\Modules\NetworkStorageIntegration\Services\DocumentTemplateService; use Espo\Modules\NetworkStorageIntegration\Services\NetworkDocumentService; +use Espo\Modules\NetworkStorageIntegration\Classes\LocalFilesystemClient; use PhpOffice\PhpWord\TemplateProcessor; class GenericTemplateGenerator @@ -72,14 +73,23 @@ class GenericTemplateGenerator @unlink($outputPath); @unlink($localTemplatePath); - // Step 6: Build document name - $contactName = $this->getContactName($case); - $docName = $documentSubject ?? "{$templateName} — {$contactName}"; + // Step 6: Build subject-only document name (rule N1) — no contact, + // no em-dash. Defaults to the template name when no subject given. + $subject = trim((string) ($documentSubject ?? '')); + if ($subject === '') { + $subject = (string) $templateName; + } + $docName = $subject; + $baseName = LocalFilesystemClient::sanitizeFileName($subject); + if ($baseName === '') { + $baseName = 'document'; + } + $fileName = $baseName . '.docx'; // Step 7: Create Attachment + Document $attachment = $this->entityManager->getNewEntity('Attachment'); $attachment->set([ - 'name' => $docName . '.docx', + 'name' => $fileName, 'type' => 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', 'role' => 'Attachment', 'size' => strlen($content), @@ -92,16 +102,61 @@ class GenericTemplateGenerator $document->set([ 'name' => $docName, 'fileId' => $attachment->getId(), - 'fileName' => $docName . '.docx', + 'fileName' => $fileName, 'assignedUserId' => $this->user->getId(), ]); - $this->entityManager->saveEntity($document); + // Suppress the NSI upload hook (fires before the Case link below); + // we upload explicitly with the Case so it lands in the case folder. + $this->entityManager->saveEntity($document, ['skipNetworkUpload' => true]); // Link to Case $this->entityManager->getRDBRepository('Case') ->getRelation($case, 'documents') ->relate($document); + // Upload to the case folder; align DB names to the canonical stored name. + try { + $nds = $this->injectableFactory->create(NetworkDocumentService::class); + if ($nds->isEnabled()) { + $result = $nds->uploadDocument($document, $attachment, 'Case', $caseId); + + if (!empty($result['success'])) { + $storedName = $result['fileName'] ?? $fileName; + $storedBase = pathinfo($storedName, PATHINFO_FILENAME) ?: $docName; + + $document->set([ + 'name' => $storedBase, + 'fileName' => $storedName, + 'storageType' => 'network', + 'networkStoragePath' => $result['path'] ?? null, + 'networkStorageFileName' => $storedName, + 'networkStorageSize' => $result['size'] ?? strlen($content), + 'networkStorageModifiedAt' => date('Y-m-d H:i:s'), + ]); + $this->entityManager->saveEntity($document, [ + 'skipNetworkUpload' => true, + 'silent' => true, + ]); + + $attachment->set('name', $storedName); + $this->entityManager->saveEntity($attachment, ['silent' => true]); + + $sourcePath = 'data/upload/' . $attachment->getSourceId(); + if (file_exists($sourcePath)) { + @unlink($sourcePath); + } + + $docName = $storedBase; + } + } + } catch (\Throwable $e) { + // Document is safely persisted in Espo; the network copy is best-effort. + $this->log->warning( + "GenericTemplateGenerator: failed to copy to network storage for Case {$caseId}: " . + $e->getMessage() + ); + } + $this->log->info("GenericTemplateGenerator: Created '{$docName}' from template '{$templateName}' for Case {$caseId}"); return [ @@ -190,24 +245,4 @@ class GenericTemplateGenerator $processor->saveAs($outputPath); } - - private function getContactName($case): string - { - $contactId = $case->get('contactId'); - if (!$contactId) { - $ids = $case->getLinkMultipleIdList('contacts'); - if (!empty($ids)) { - $contactId = $ids[0]; - } - } - - if ($contactId) { - $contact = $this->entityManager->getEntityById('Contact', $contactId); - if ($contact) { - return trim($contact->get('firstName') . ' ' . $contact->get('lastName')); - } - } - - return $case->get('name') ?? ''; - } } diff --git a/manifest.json b/manifest.json index 4a5b394..b110644 100644 --- a/manifest.json +++ b/manifest.json @@ -3,11 +3,11 @@ "module": "SmartAssistant", "description": "Unified AI Assistant for Legal CRM — floating chat with case memory, office alerts, and AI Gateway integration", "author": "klear", - "version": "2.10.4", + "version": "2.10.5", "acceptableVersions": [ ">=8.0.0" ], - "releaseDate": "2026-05-27", + "releaseDate": "2026-06-03", "php": [ ">=8.1" ]