mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-08-09 00:16:57 -07:00
feat(caretaker): update Firestore schema with error, and pr_number fields (#28467)
This commit is contained in:
@@ -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,
|
||||
}),
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
@@ -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,
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -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
|
||||
)
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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"])
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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"
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user