Skip to content

Commit 3ab885d

Browse files
authored
Merge branch 'main' into feat/246-registry-ddb-s3
2 parents 773764f + beef46a commit 3ab885d

17 files changed

Lines changed: 2192 additions & 266 deletions

agent/src/pipeline.py

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import asyncio
66
import hashlib
77
import os
8-
import subprocess
98
import sys
109
import time
1110
from typing import TYPE_CHECKING
@@ -822,19 +821,17 @@ def _on_trace_truncated(max_bytes: int, first_dropped: int) -> None:
822821
system_prompt_overrides=system_prompt_overrides,
823822
)
824823

825-
# Configure git and gh auth before setup_repo() uses them
826-
subprocess.run(
827-
["git", "config", "--global", "user.name", "bgagent"],
828-
check=True,
829-
capture_output=True,
830-
timeout=60,
831-
)
832-
subprocess.run(
833-
["git", "config", "--global", "user.email", "bgagent@noreply.github.com"],
834-
check=True,
835-
capture_output=True,
836-
timeout=60,
837-
)
824+
# Configure git identity and gh auth before setup_repo() uses them.
825+
# Use GIT_AUTHOR_*/GIT_COMMITTER_* env vars rather than
826+
# `git config --global`: git honors these for every commit (inherited
827+
# by Claude Code and the safety-net commit in post_hooks) WITHOUT
828+
# writing to any on-disk config. `--global` would clobber the real
829+
# ~/.gitconfig — harmless in the ephemeral container, but destructive
830+
# when this pipeline runs on a developer workstation (#622).
831+
os.environ["GIT_AUTHOR_NAME"] = "bgagent"
832+
os.environ["GIT_AUTHOR_EMAIL"] = "bgagent@noreply.github.com"
833+
os.environ["GIT_COMMITTER_NAME"] = "bgagent"
834+
os.environ["GIT_COMMITTER_EMAIL"] = "bgagent@noreply.github.com"
838835
os.environ["GITHUB_TOKEN"] = config.github_token
839836
os.environ["GH_TOKEN"] = config.github_token
840837

agent/tests/test_pipeline.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Unit tests for pipeline.py — cedar_policies injection and pure helpers."""
22

3+
import os
34
from unittest.mock import MagicMock, patch
45

56
import pytest
@@ -142,6 +143,78 @@ async def fake_run_agent(_prompt, _system_prompt, config, cwd=None, trajectory=N
142143
assert captured_config is not None
143144
assert captured_config.cedar_policies == []
144145

146+
@patch("runner.run_agent")
147+
@patch("pipeline.build_system_prompt")
148+
@patch("pipeline.discover_project_config")
149+
@patch("repo.setup_repo")
150+
@patch("pipeline.task_span")
151+
@patch("pipeline.task_state")
152+
def test_git_identity_uses_env_vars_not_global_config(
153+
self,
154+
_mock_task_state,
155+
mock_task_span,
156+
mock_setup_repo,
157+
_mock_discover,
158+
_mock_build_prompt,
159+
mock_run_agent,
160+
monkeypatch,
161+
):
162+
"""Git identity is set via GIT_AUTHOR/COMMITTER env vars, never
163+
`git config --global`, so a developer's ~/.gitconfig is never
164+
clobbered when the pipeline runs on a workstation (#622)."""
165+
monkeypatch.setenv("GITHUB_TOKEN", "ghp_test")
166+
monkeypatch.setenv("AWS_REGION", "us-east-1")
167+
# Ensure a clean slate so the assertion proves the pipeline set them.
168+
for var in (
169+
"GIT_AUTHOR_NAME",
170+
"GIT_AUTHOR_EMAIL",
171+
"GIT_COMMITTER_NAME",
172+
"GIT_COMMITTER_EMAIL",
173+
):
174+
monkeypatch.delenv(var, raising=False)
175+
176+
mock_setup_repo.return_value = RepoSetup(
177+
repo_dir="/workspace/repo",
178+
branch="bgagent/test/branch",
179+
build_before=True,
180+
)
181+
182+
async def fake_run_agent(_prompt, _system_prompt, config, cwd=None, trajectory=None):
183+
return AgentResult(status="success", turns=1, cost_usd=0.01, num_turns=1)
184+
185+
mock_run_agent.side_effect = fake_run_agent
186+
187+
mock_span = MagicMock()
188+
mock_span.__enter__ = MagicMock(return_value=mock_span)
189+
mock_span.__exit__ = MagicMock(return_value=False)
190+
mock_task_span.return_value = mock_span
191+
192+
with (
193+
patch("pipeline.ensure_committed", return_value=False),
194+
patch("pipeline.verify_build", return_value=True),
195+
patch("pipeline.verify_lint", return_value=True),
196+
patch(
197+
"pipeline.ensure_pr",
198+
return_value="https://github.com/org/repo/pull/1",
199+
),
200+
patch("pipeline.get_disk_usage", return_value=0),
201+
patch("pipeline.print_metrics"),
202+
):
203+
from pipeline import run_task
204+
205+
run_task(
206+
repo_url="owner/repo",
207+
task_description="fix bug",
208+
github_token="ghp_test",
209+
aws_region="us-east-1",
210+
task_id="test-id",
211+
)
212+
213+
assert os.environ["GIT_AUTHOR_NAME"] == "bgagent"
214+
assert os.environ["GIT_AUTHOR_EMAIL"] == "bgagent@noreply.github.com"
215+
assert os.environ["GIT_COMMITTER_NAME"] == "bgagent"
216+
assert os.environ["GIT_COMMITTER_EMAIL"] == "bgagent@noreply.github.com"
217+
145218

146219
class TestRepoLessPipeline:
147220
"""#248 Phase 3: a repo-less workflow runs the agent with no clone/build/PR."""

cdk/src/constructs/jira-integration.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import * as dynamodb from 'aws-cdk-lib/aws-dynamodb';
2525
import * as iam from 'aws-cdk-lib/aws-iam';
2626
import { Runtime, Architecture } from 'aws-cdk-lib/aws-lambda';
2727
import * as lambda from 'aws-cdk-lib/aws-lambda-nodejs';
28+
import * as s3 from 'aws-cdk-lib/aws-s3';
2829
import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager';
2930
import { NagSuppressions } from 'cdk-nag';
3031
import { Construct } from 'constructs';
@@ -35,8 +36,17 @@ import { JiraWorkspaceRegistryTable } from './jira-workspace-registry-table';
3536
/** Default task-record retention used for TTL computation (days). */
3637
const DEFAULT_TASK_RETENTION_DAYS = 90;
3738

38-
/** Webhook-processor Lambda timeout (seconds). */
39-
const WEBHOOK_PROCESSOR_TIMEOUT_SECONDS = 30;
39+
/**
40+
* Webhook-processor Lambda timeout (seconds). The processor is invoked
41+
* asynchronously (not behind the API Gateway 30s deadline), so it can run
42+
* longer than the receiver. #577 added serial authenticated download +
43+
* Bedrock screening of up to 10 Jira attachments; the per-attachment fetch
44+
* timeout alone (10s) can sum past 60s across a full batch, and a mid-loop
45+
* kill would orphan objects and force an idempotent retry. 300s covers the
46+
* worst-case serial batch with headroom. (A future optimization could
47+
* parallelize the download/screen loop and lower this again.)
48+
*/
49+
const WEBHOOK_PROCESSOR_TIMEOUT_SECONDS = 300;
4050

4151
/**
4252
* Marker key embedded in the auto-generated stack-wide webhook-secret
@@ -87,6 +97,14 @@ export interface JiraIntegrationProps {
8797
/** Bedrock Guardrail version for input screening. */
8898
readonly guardrailVersion?: string;
8999

100+
/**
101+
* S3 bucket for task attachment storage. Required for the webhook processor
102+
* to fetch, screen, and store Jira `media` file attachments at task-admission
103+
* time (issue #577). When omitted, issues carrying supported file attachments
104+
* are rejected with a Jira comment rather than silently dropping them.
105+
*/
106+
readonly attachmentsBucket?: s3.IBucket;
107+
90108
/** Task retention in days for TTL computation. */
91109
readonly taskRetentionDays?: number;
92110

@@ -206,6 +224,9 @@ export class JiraIntegration extends Construct {
206224
createTaskEnv.GUARDRAIL_ID = props.guardrailId;
207225
createTaskEnv.GUARDRAIL_VERSION = props.guardrailVersion;
208226
}
227+
if (props.attachmentsBucket) {
228+
createTaskEnv.ATTACHMENTS_BUCKET_NAME = props.attachmentsBucket.bucketName;
229+
}
209230

210231
// --- Cognito Authorizer (for /jira/link) ---
211232
const cognitoAuthorizer = new apigw.CognitoUserPoolsAuthorizer(this, 'JiraCognitoAuthorizer', {
@@ -280,6 +301,13 @@ export class JiraIntegration extends Construct {
280301
],
281302
}));
282303
}
304+
// The processor downloads Jira `media` attachments, screens them, and
305+
// writes the cleaned bytes to the attachments bucket before creating the
306+
// task (#577). ReadWrite mirrors the confirm-uploads path (Put + Get for
307+
// multipart/versioned writes).
308+
if (props.attachmentsBucket) {
309+
props.attachmentsBucket.grantReadWrite(webhookProcessorFn);
310+
}
283311

284312
// --- Webhook receiver (verifies HMAC, dedups, invokes processor) ---
285313
const webhookFn = new lambda.NodejsFunction(this, 'WebhookFn', {

cdk/src/constructs/task-api.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,18 @@ export class TaskApi extends Construct {
340340
textTransformations: [{ priority: 0, type: 'NONE' }],
341341
},
342342
},
343+
{
344+
// Jira issue payloads include the full issue field set.
345+
// Attachment metadata can push them over 8 KB. The
346+
// receiver HMAC-verifies the raw body and the priority-4
347+
// rule below still rate-limits this route.
348+
byteMatchStatement: {
349+
fieldToMatch: { uriPath: {} },
350+
positionalConstraint: 'EXACTLY',
351+
searchString: '/v1/jira/webhook',
352+
textTransformations: [{ priority: 0, type: 'NONE' }],
353+
},
354+
},
343355
{
344356
// GitHub deployment_status webhook (preview-deploy
345357
// screenshot pipeline). The full payload (workflow run
@@ -390,6 +402,18 @@ export class TaskApi extends Construct {
390402
},
391403
},
392404
},
405+
{
406+
notStatement: {
407+
statement: {
408+
byteMatchStatement: {
409+
fieldToMatch: { uriPath: {} },
410+
positionalConstraint: 'EXACTLY',
411+
searchString: '/v1/jira/webhook',
412+
textTransformations: [{ priority: 0, type: 'NONE' }],
413+
},
414+
},
415+
},
416+
},
393417
{
394418
notStatement: {
395419
statement: {

0 commit comments

Comments
 (0)