From 66708e3c4cdb44cdcf8092afc4690311893fbdc6 Mon Sep 17 00:00:00 2001 From: Chad Date: Fri, 7 Aug 2026 13:01:11 -0500 Subject: [PATCH] feat(caretaker): update Firestore schema with error, and pr_number fields (#28467) --- .../cloudrun/ingestion-service/db/issuesStore.test.ts | 2 ++ .../cloudrun/ingestion-service/db/issuesStore.ts | 6 +++++- .../cloudrun/triage-worker/db/issues_store.py | 11 +++++++++-- tools/caretaker-agent/cloudrun/triage-worker/main.py | 9 ++++++--- .../triage-worker/tests/test_integration_main.py | 7 ++++++- .../cloudrun/triage-worker/tests/test_issues_store.py | 10 +++++++++- .../cloudrun/triage-worker/tests/test_main.py | 2 +- 7 files changed, 38 insertions(+), 9 deletions(-) diff --git a/tools/caretaker-agent/cloudrun/ingestion-service/db/issuesStore.test.ts b/tools/caretaker-agent/cloudrun/ingestion-service/db/issuesStore.test.ts index 86ac17ea24..231c68e67d 100644 --- a/tools/caretaker-agent/cloudrun/ingestion-service/db/issuesStore.test.ts +++ b/tools/caretaker-agent/cloudrun/ingestion-service/db/issuesStore.test.ts @@ -55,11 +55,13 @@ describe('IssuesStore', () => { expect.anything(), expect.objectContaining({ status: 'UNTRIAGED', + error: null, github_metadata: expect.objectContaining({ owner: 'google', repo: 'gemini-cli', issue_number: 123, title: 'Test Title', + pr_number: null, }), }), ); diff --git a/tools/caretaker-agent/cloudrun/ingestion-service/db/issuesStore.ts b/tools/caretaker-agent/cloudrun/ingestion-service/db/issuesStore.ts index b981cf511f..6dc7450e91 100644 --- a/tools/caretaker-agent/cloudrun/ingestion-service/db/issuesStore.ts +++ b/tools/caretaker-agent/cloudrun/ingestion-service/db/issuesStore.ts @@ -18,10 +18,11 @@ export type IssueStatus = | 'NEEDS_INFO' | 'TRIAGED' | 'NEEDS_HUMAN' - | 'LOW_QUALITY'; + | 'AUTO_CLOSE'; export interface IssueDocument { status: IssueStatus; + error?: string | null; triage_attempts: number; // The ingestion layer does not enforce the schema of workable_spec workable_spec: Record; @@ -36,6 +37,7 @@ export interface IssueDocument { repo: string; issue_number: number; title: string; + pr_number?: number | null; }; } @@ -74,6 +76,7 @@ export class IssuesStore { if (!snapshot.exists) { const newIssue: IssueDocument = { status: 'UNTRIAGED', + error: null, triage_attempts: 0, workable_spec: {}, lock: { @@ -87,6 +90,7 @@ export class IssuesStore { repo, issue_number: issueNumber, title, + pr_number: null, }, }; diff --git a/tools/caretaker-agent/cloudrun/triage-worker/db/issues_store.py b/tools/caretaker-agent/cloudrun/triage-worker/db/issues_store.py index b9bfe2643e..33a9a4d965 100644 --- a/tools/caretaker-agent/cloudrun/triage-worker/db/issues_store.py +++ b/tools/caretaker-agent/cloudrun/triage-worker/db/issues_store.py @@ -72,7 +72,8 @@ class IssuesStore: if attempts >= 2: transaction.update(doc_ref, { - "status": "NEEDS_HUMAN", + "status": "NEEDS_HUMAN", + "error": "Max triage attempts (2) exceeded due to prior worker crash or timeout", "updated_at": firestore.SERVER_TIMESTAMP }) return ClaimAction.NEEDS_HUMAN @@ -152,6 +153,7 @@ class IssuesStore: success: bool, workable_spec: dict = None, status: str = None, + error: str = None, ) -> ReleaseAction: """Internal transactional handler to release processing lock.""" snapshot = doc_ref.get(transaction=transaction) @@ -173,6 +175,7 @@ class IssuesStore: if success: updates["status"] = status updates["workable_spec"] = workable_spec or {} + updates["error"] = None transaction.update(doc_ref, updates) return ReleaseAction.COMPLETE @@ -184,6 +187,7 @@ class IssuesStore: return ReleaseAction.RETRY updates["status"] = "NEEDS_HUMAN" + updates["error"] = error or "Max triage attempts (2) exceeded." transaction.update(doc_ref, updates) return ReleaseAction.COMPLETE @@ -196,6 +200,7 @@ class IssuesStore: success: bool, workable_spec: dict = None, status: str = None, + error: str = None, ) -> ReleaseAction: """ Releases the processing lock for an issue and updates its final status. @@ -211,6 +216,8 @@ class IssuesStore: is TRIAGED. status: Target issue status (TRIAGED, NEEDS_INFO, AUTO_CLOSE, or NEEDS_HUMAN). + error: Error string or failure details to store when status + transitions to NEEDS_HUMAN. Returns: ReleaseAction indicating COMPLETE or RETRY. @@ -218,5 +225,5 @@ class IssuesStore: doc_ref = self._get_issue_ref(owner, repo, issue_number) transaction = self.db.transaction() return self._release_lock_tx( - transaction, doc_ref, lock_holder, success, workable_spec, status + transaction, doc_ref, lock_holder, success, workable_spec, status, error ) diff --git a/tools/caretaker-agent/cloudrun/triage-worker/main.py b/tools/caretaker-agent/cloudrun/triage-worker/main.py index 87ef3823c5..f383d4d4d4 100644 --- a/tools/caretaker-agent/cloudrun/triage-worker/main.py +++ b/tools/caretaker-agent/cloudrun/triage-worker/main.py @@ -88,8 +88,9 @@ def main() -> None: success, raw_output = process_issue_triage(payload) except Exception as e: print(f"[WORKER] Triage process failed with exception: {e}") - success, raw_output = False, "" + success, raw_output = False, f"Exception during triage execution: {e}" + error_message = None if success: try: triage_result = json.loads(raw_output) @@ -158,13 +159,15 @@ def main() -> None: except Exception as e: print(f"[WORKER] Validation failed: {e}") - success = False + success, error_message = False, f"Validation Error: {e}" + else: + error_message = raw_output # If an exception happens in json.loads or validate_triage_result # If LLM inference itself fails inside process_issue_triage if not success: release_action = store.release_lock( - owner, repo, issue_number, lock_holder, success=False + owner, repo, issue_number, lock_holder, success=False, error=error_message ) sys.exit(1 if release_action == ReleaseAction.RETRY else 0) diff --git a/tools/caretaker-agent/cloudrun/triage-worker/tests/test_integration_main.py b/tools/caretaker-agent/cloudrun/triage-worker/tests/test_integration_main.py index b12757edd7..c267323f76 100644 --- a/tools/caretaker-agent/cloudrun/triage-worker/tests/test_integration_main.py +++ b/tools/caretaker-agent/cloudrun/triage-worker/tests/test_integration_main.py @@ -275,7 +275,12 @@ class TestIntegrationMain(unittest.TestCase): "owner", "repo", 42, "test-workflow-exec-101" ) self.mock_store.release_lock.assert_called_once_with( - "owner", "repo", 42, "test-workflow-exec-101", success=False + "owner", + "repo", + 42, + "test-workflow-exec-101", + success=False, + error="Validation Error: Invalid or missing 'effort_estimate': HUGE", ) self.assertEqual(self.stored_data["status"], "UNTRIAGED") self.assertIsNone(self.stored_data["lock"]["holder"]) diff --git a/tools/caretaker-agent/cloudrun/triage-worker/tests/test_issues_store.py b/tools/caretaker-agent/cloudrun/triage-worker/tests/test_issues_store.py index 6bfed65d49..c4e601b82d 100644 --- a/tools/caretaker-agent/cloudrun/triage-worker/tests/test_issues_store.py +++ b/tools/caretaker-agent/cloudrun/triage-worker/tests/test_issues_store.py @@ -49,6 +49,10 @@ class TestIssuesStore(unittest.TestCase): self.transaction.update.assert_called_once() args, _ = self.transaction.update.call_args self.assertEqual(args[1]["status"], "NEEDS_HUMAN") + self.assertEqual( + args[1]["error"], + "Max triage attempts (2) exceeded due to prior worker crash or timeout", + ) def test_acquire_lock_active_lock_by_other_holder(self): """acquire lock when active lock held by another worker should skip""" @@ -131,6 +135,7 @@ class TestIssuesStore(unittest.TestCase): updates = args[1] self.assertEqual(updates["status"], "TRIAGED") self.assertEqual(updates["workable_spec"], workable_spec) + self.assertIsNone(updates["error"]) self.assertIsNone(updates["lock.holder"]) self.assertIsNone(updates["lock.expires_at"]) @@ -158,13 +163,16 @@ class TestIssuesStore(unittest.TestCase): "triage_attempts": 2, } - action = self.store.release_lock("owner", "repo", 123, self.lock_holder, success=False) + action = self.store.release_lock( + "owner", "repo", 123, self.lock_holder, success=False, error="LLM failed" + ) self.assertEqual(action, ReleaseAction.COMPLETE) self.transaction.update.assert_called_once() args, _ = self.transaction.update.call_args updates = args[1] self.assertEqual(updates["status"], "NEEDS_HUMAN") + self.assertEqual(updates["error"], "LLM failed") if __name__ == "__main__": unittest.main() diff --git a/tools/caretaker-agent/cloudrun/triage-worker/tests/test_main.py b/tools/caretaker-agent/cloudrun/triage-worker/tests/test_main.py index e255ae8130..1f3ba513f7 100644 --- a/tools/caretaker-agent/cloudrun/triage-worker/tests/test_main.py +++ b/tools/caretaker-agent/cloudrun/triage-worker/tests/test_main.py @@ -175,7 +175,7 @@ class TestMainExecutionLoop(unittest.TestCase): self.assertEqual(ctx.exception.code, 1) self.mock_store.release_lock.assert_called_once_with( - "owner", "repo", 42, "exec-123", success=False + "owner", "repo", 42, "exec-123", success=False, error="LLM failed" )