Skip to content

Commit 420fc11

Browse files
bgagentclaude
andcommitted
chore(linear): address PR #87 nice-to-have review items
- linear_reactions: log a single DEBUG line when the auth circuit breaker short-circuits a call, so the path isn't zero-trace once open. - config: split the `(BotoCoreError, ClientError)` catch so `AccessDeniedException` logs at ERROR instead of WARN — IAM misconfig is persistent and should page someone, not blend into transient warnings. Also drop the personal name from the inline reference to the #63 review. - linear-webhook-processor: tighten `buildCreateTaskFailureMessage` param types to `number` / `string` (no `| undefined`) — the only caller passes `APIGatewayProxyResult` fields which are always defined. Removes dead fallback-to-`'unknown'` branches. - test_config: 2 new tests covering the split exception path (AccessDenied → ERROR; ResourceNotFound → WARN). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent dd5474e commit 420fc11

4 files changed

Lines changed: 55 additions & 10 deletions

File tree

agent/src/config.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,18 @@ def resolve_linear_api_token() -> str:
7575
if token:
7676
os.environ["LINEAR_API_TOKEN"] = token
7777
return token
78-
except (BotoCoreError, ClientError) as e:
78+
except ClientError as e:
79+
# Narrowed from a broader `except` per #63 review — broader catches
80+
# hid genuine bugs in the Secrets Manager call shape. AccessDenied
81+
# is logged at ERROR because it's a persistent IAM misconfig that
82+
# should page someone, not a transient blip.
83+
code = e.response.get("Error", {}).get("Code", "")
84+
severity = "ERROR" if code == "AccessDeniedException" else "WARN"
85+
log(severity, f"resolve_linear_api_token failed: {type(e).__name__}: {e}")
86+
return ""
87+
except BotoCoreError as e:
7988
# Never let a Secrets Manager outage crash the agent. The Linear MCP
80-
# will simply fail on first call with a clear auth error. Narrowed
81-
# to botocore exceptions per Alain's #63 review — broader `except`
82-
# hid genuine bugs in the Secrets Manager call shape.
89+
# will simply fail on first call with a clear auth error.
8390
log("WARN", f"resolve_linear_api_token failed: {type(e).__name__}: {e}")
8491
return ""
8592

agent/src/linear_reactions.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,10 @@ def _graphql(query: str, variables: dict[str, Any]) -> dict[str, Any] | None:
129129
global _consecutive_auth_failures, _auth_circuit_open
130130

131131
with _auth_state_lock:
132-
if _auth_circuit_open:
133-
return None
132+
circuit_open = _auth_circuit_open
133+
if circuit_open:
134+
log("DEBUG", "linear_reactions: auth circuit still open; short-circuiting call")
135+
return None
134136

135137
token = os.environ.get("LINEAR_API_TOKEN", "")
136138
if not token:

agent/tests/test_config.py

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"""Unit tests for config.py — build_config and constants."""
22

33
import sys
4-
from unittest.mock import patch
4+
from unittest.mock import MagicMock, patch
55

66
import pytest
77

@@ -120,3 +120,39 @@ def test_import_error_degrades_gracefully(self, monkeypatch):
120120
assert mock_log.call_count == 1
121121
assert mock_log.call_args[0][0] == "WARN"
122122
assert "boto3 unavailable" in mock_log.call_args[0][1]
123+
124+
def test_access_denied_logged_at_error(self, monkeypatch):
125+
"""Persistent IAM misconfig should page someone — escalate from WARN
126+
to ERROR so alerts fire."""
127+
monkeypatch.delenv("LINEAR_API_TOKEN", raising=False)
128+
monkeypatch.setenv("LINEAR_API_TOKEN_SECRET_ARN", "arn:aws:sm:::secret/linear")
129+
130+
from botocore.exceptions import ClientError
131+
132+
err = ClientError(
133+
{"Error": {"Code": "AccessDeniedException", "Message": "no access"}},
134+
"GetSecretValue",
135+
)
136+
fake_client = MagicMock()
137+
fake_client.get_secret_value.side_effect = err
138+
with patch("boto3.client", return_value=fake_client), patch("config.log") as mock_log:
139+
assert resolve_linear_api_token() == ""
140+
assert mock_log.call_count == 1
141+
assert mock_log.call_args[0][0] == "ERROR"
142+
143+
def test_other_client_error_logged_at_warn(self, monkeypatch):
144+
monkeypatch.delenv("LINEAR_API_TOKEN", raising=False)
145+
monkeypatch.setenv("LINEAR_API_TOKEN_SECRET_ARN", "arn:aws:sm:::secret/linear")
146+
147+
from botocore.exceptions import ClientError
148+
149+
err = ClientError(
150+
{"Error": {"Code": "ResourceNotFoundException", "Message": "missing"}},
151+
"GetSecretValue",
152+
)
153+
fake_client = MagicMock()
154+
fake_client.get_secret_value.side_effect = err
155+
with patch("boto3.client", return_value=fake_client), patch("config.log") as mock_log:
156+
assert resolve_linear_api_token() == ""
157+
assert mock_log.call_count == 1
158+
assert mock_log.call_args[0][0] == "WARN"

cdk/src/handlers/linear-webhook-processor.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ function shouldTrigger(payload: LinearIssueEvent, labelFilter: string): boolean
273273
*
274274
* Falls back to a generic message if the body fails to parse — best-effort, never throws.
275275
*/
276-
function buildCreateTaskFailureMessage(statusCode: number | undefined, rawBody: string | undefined): string {
276+
function buildCreateTaskFailureMessage(statusCode: number, rawBody: string): string {
277277
let detail = '';
278278
try {
279279
if (rawBody) {
@@ -296,9 +296,9 @@ function buildCreateTaskFailureMessage(statusCode: number | undefined, rawBody:
296296
return `❌ ABCA is temporarily unavailable (status ${statusCode}). Please re-apply the trigger label in a few minutes.`;
297297
}
298298
if (detail) {
299-
return `❌ ABCA couldn't create this task (status ${statusCode ?? 'unknown'}): ${detail}`;
299+
return `❌ ABCA couldn't create this task (status ${statusCode}): ${detail}`;
300300
}
301-
return `❌ ABCA couldn't create this task (status ${statusCode ?? 'unknown'}). Check the ABCA admin logs for details.`;
301+
return `❌ ABCA couldn't create this task (status ${statusCode}). Check the ABCA admin logs for details.`;
302302
}
303303

304304
function buildTaskDescription(issue: LinearIssueEvent['data']): string {

0 commit comments

Comments
 (0)