Skip to content

Commit c9f1abf

Browse files
committed
feat: extend Job Handler
1 parent c6fb67c commit c9f1abf

25 files changed

Lines changed: 563 additions & 271 deletions

File tree

app/AGENTS.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ AI-powered code review bot that monitors GitHub PRs and GitLab MRs. When a user
55
## Architecture
66

77
- **Async-first** — built on aiohttp + asyncio; all I/O (HTTP, git, agent) is non-blocking.
8+
- **Two-layer config** — mutable `*Settings` models (`config/models.py`) parse YAML/env vars; `loader.py` transforms them into frozen `*Config` models (`config/config.py`, `config/kubernetes.py`) that application code consumes. See `docs/architecture.md` for details.
89
- **Protocol-based platforms** — GitHub and GitLab implement the same `Platform` / `ReviewerPlatform` protocols, making it easy to add new providers.
910
- **Per-PR job serialisation**`JobQueue` protocol with two implementations (`AsyncioJobQueue` for in-process, `RedisJobQueue` for Kubernetes) guarantees only one agent job runs per PR at a time, preventing race conditions on the same workspace.
1011
- **Multi-turn conversations**`ConversationStore` maps (platform, repo, PR, bot) to conversation IDs and message histories so conversations resume across comments.
@@ -85,7 +86,13 @@ The dispatcher in `agent/invoke.py` routes based on the agent config type (`CliA
8586
```
8687
nominal_code/
8788
├── main.py # Entry point: dispatches to webhook server, CLI, or CI
88-
├── config.py # Frozen dataclass config loaded from env vars / files
89+
├── config/
90+
│ ├── models.py # Mutable *Settings models (YAML/env input layer)
91+
│ ├── config.py # Frozen *Config models (application output layer)
92+
│ ├── loader.py # Settings → Config transformation with validation
93+
│ ├── policies.py # FilteringPolicy and RoutingPolicy (frozen)
94+
│ ├── agent.py # AgentConfig, CliAgentConfig, ApiAgentConfig
95+
│ └── kubernetes.py # KubernetesConfig (frozen)
8996
├── models.py # Shared enums (EventType, BotType, FileStatus) and dataclasses (ReviewFinding, AgentReview, ChangedFile)
9097
├── commands/ # Entry points: CLI review, CI mode, webhook server, K8s job runner
9198
│ ├── webhook/ # Webhook server (server.py), helpers, mention extraction, K8s job runner (job.py)

app/nominal_code/commands/webhook/job.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,9 @@ async def run_job_main() -> int:
5656

5757
return 1
5858

59-
redis = config.webhook.redis if config.webhook is not None else None
6059
conversation_store: ConversationStore = build_conversation_store(
61-
redis_url=redis.url if redis is not None else "",
62-
redis_key_ttl_seconds=redis.key_ttl_seconds if redis is not None else 86400,
60+
redis_url=_env.str("REDIS_URL", ""),
61+
redis_key_ttl_seconds=int(_env.str("REDIS_KEY_TTL_SECONDS", "86400")),
6362
)
6463

6564
platform_name: PlatformName = PlatformName(job.event.platform)
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
from __future__ import annotations
2+
3+
from dataclasses import dataclass
4+
5+
6+
@dataclass(frozen=True)
7+
class DispatchResult:
8+
"""
9+
Framework-agnostic result from webhook dispatch functions.
10+
11+
Replaces direct ``web.Response`` returns so that dispatch logic is
12+
decoupled from any specific HTTP framework.
13+
14+
Attributes:
15+
status (str): Semantic status string (e.g. ``"accepted"``,
16+
``"ignored"``, ``"no_mention"``, ``"unauthorized"``).
17+
http_status (int): Suggested HTTP status code for the caller
18+
to use when building the actual response.
19+
"""
20+
21+
status: str
22+
http_status: int = 200

app/nominal_code/commands/webhook/server.py

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,10 @@
1010
from aiohttp import web
1111

1212
from nominal_code.commands.webhook.helpers import acknowledge_event, extract_mention
13+
from nominal_code.commands.webhook.result import DispatchResult
1314
from nominal_code.config import Config, load_config
15+
from nominal_code.config.config import WebhookConfig
1416
from nominal_code.config.policies import FilteringPolicy, RoutingPolicy
15-
from nominal_code.config.settings import WebhookConfig
1617
from nominal_code.jobs.payload import JobPayload
1718
from nominal_code.jobs.runner import build_runner
1819
from nominal_code.models import COMMENT_EVENT_TYPES, BotType
@@ -24,6 +25,7 @@
2425
PullRequestEvent,
2526
ReviewerPlatform,
2627
)
28+
2729
if TYPE_CHECKING:
2830
from nominal_code.jobs.runner.base import JobRunner
2931

@@ -217,7 +219,7 @@ async def dispatch_lifecycle_event(
217219
runner: JobRunner,
218220
namespace: str = "",
219221
extra_env: dict[str, str] | None = None,
220-
) -> web.Response:
222+
) -> DispatchResult:
221223
"""
222224
Dispatch a lifecycle event to the reviewer bot.
223225
@@ -235,17 +237,17 @@ async def dispatch_lifecycle_event(
235237
extra_env (dict[str, str] | None): Additional env vars for the job container.
236238
237239
Returns:
238-
web.Response: The HTTP response.
240+
DispatchResult: The dispatch result.
239241
"""
240242

241243
if not routing.reviewer_bot_username:
242-
return web.json_response({"status": "ignored"})
244+
return DispatchResult(status="ignored")
243245

244246
if not isinstance(event, LifecycleEvent):
245-
return web.json_response({"status": "ignored"})
247+
return DispatchResult(status="ignored")
246248

247249
if not isinstance(platform, ReviewerPlatform):
248-
return web.json_response({"status": "ignored"})
250+
return DispatchResult(status="ignored")
249251

250252
await acknowledge_event(
251253
event=event,
@@ -263,7 +265,7 @@ async def dispatch_lifecycle_event(
263265

264266
await runner.enqueue(job)
265267

266-
return web.json_response({"status": "accepted"})
268+
return DispatchResult(status="accepted")
267269

268270

269271
async def dispatch_comment_event(
@@ -274,7 +276,7 @@ async def dispatch_comment_event(
274276
runner: JobRunner,
275277
namespace: str = "",
276278
extra_env: dict[str, str] | None = None,
277-
) -> web.Response:
279+
) -> DispatchResult:
278280
"""
279281
Dispatch a comment event to the appropriate bot.
280282
@@ -293,14 +295,14 @@ async def dispatch_comment_event(
293295
extra_env (dict[str, str] | None): Additional env vars for the job container.
294296
295297
Returns:
296-
web.Response: The HTTP response.
298+
DispatchResult: The dispatch result.
297299
"""
298300

299301
if event.event_type not in COMMENT_EVENT_TYPES:
300-
return web.json_response({"status": "ignored"})
302+
return DispatchResult(status="ignored")
301303

302304
if not isinstance(event, CommentEvent):
303-
return web.json_response({"status": "ignored"})
305+
return DispatchResult(status="ignored")
304306

305307
comment_event: CommentEvent = event
306308

@@ -328,7 +330,7 @@ async def dispatch_comment_event(
328330
mention_prompt = reviewer_prompt
329331

330332
else:
331-
return web.json_response({"status": "no_mention"})
333+
return DispatchResult(status="no_mention")
332334

333335
proceed: bool = await acknowledge_event(
334336
event=comment_event,
@@ -338,7 +340,7 @@ async def dispatch_comment_event(
338340
)
339341

340342
if not proceed:
341-
return web.json_response({"status": "unauthorized"})
343+
return DispatchResult(status="unauthorized", http_status=403)
342344

343345
mentioned_event: CommentEvent = replace(
344346
comment_event,
@@ -354,7 +356,7 @@ async def dispatch_comment_event(
354356

355357
await runner.enqueue(job)
356358

357-
return web.json_response({"status": "accepted"})
359+
return DispatchResult(status="accepted")
358360

359361

360362
async def _handle_health(request: web.Request) -> web.Response:
@@ -421,15 +423,15 @@ async def _handle_webhook(
421423

422424
body: bytes = await request.read()
423425

424-
if not platform.verify_webhook(request=request, body=body):
426+
if not platform.verify_webhook(headers=request.headers, body=body):
425427
logger.warning("Invalid webhook signature for %s", platform_name)
426428

427429
return web.Response(status=401, text="Invalid signature")
428430

429431
await platform.authenticate(webhook_body=body)
430432

431433
event: CommentEvent | LifecycleEvent | None = platform.parse_event(
432-
request=request,
434+
headers=request.headers,
433435
body=body,
434436
)
435437

@@ -442,20 +444,25 @@ async def _handle_webhook(
442444
return web.json_response({"status": filter_reason})
443445

444446
if event.event_type in routing.reviewer_triggers:
445-
return await dispatch_lifecycle_event(
447+
result: DispatchResult = await dispatch_lifecycle_event(
448+
event=event,
449+
filtering=filtering,
450+
routing=routing,
451+
platform=platform,
452+
runner=runner,
453+
)
454+
else:
455+
result = await dispatch_comment_event(
446456
event=event,
447457
filtering=filtering,
448458
routing=routing,
449459
platform=platform,
450460
runner=runner,
451461
)
452462

453-
return await dispatch_comment_event(
454-
event=event,
455-
filtering=filtering,
456-
routing=routing,
457-
platform=platform,
458-
runner=runner,
463+
return web.json_response(
464+
{"status": result.status},
465+
status=result.http_status,
459466
)
460467

461468
except Exception:

app/nominal_code/config/__init__.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,7 @@
77
ProviderConfig,
88
resolve_provider_config,
99
)
10-
from nominal_code.config.kubernetes import KubernetesConfig
11-
from nominal_code.config.loader import (
12-
load_config,
13-
load_config_for_ci,
14-
load_config_for_cli,
15-
)
16-
from nominal_code.config.policies import FilteringPolicy, RoutingPolicy
17-
from nominal_code.config.settings import (
10+
from nominal_code.config.config import (
1811
Config,
1912
PromptsConfig,
2013
RedisConfig,
@@ -23,6 +16,13 @@
2316
WorkerConfig,
2417
WorkspaceConfig,
2518
)
19+
from nominal_code.config.kubernetes import KubernetesConfig
20+
from nominal_code.config.loader import (
21+
load_config,
22+
load_config_for_ci,
23+
load_config_for_cli,
24+
)
25+
from nominal_code.config.policies import FilteringPolicy, RoutingPolicy
2626

2727
__all__ = [
2828
"AgentConfig",
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
from __future__ import annotations
22

33
import logging
4-
import tempfile
54
from pathlib import Path
65

76
from pydantic import BaseModel, ConfigDict
@@ -10,6 +9,7 @@
109
from nominal_code.config.kubernetes import KubernetesConfig
1110
from nominal_code.config.policies import FilteringPolicy, RoutingPolicy
1211
from nominal_code.models import EventType, ProviderName
12+
from nominal_code.workspace.git import DEFAULT_BASE_DIR
1313

1414
logger: logging.Logger = logging.getLogger(__name__)
1515

@@ -76,7 +76,7 @@ class WorkspaceConfig(BaseModel):
7676

7777
model_config = ConfigDict(frozen=True)
7878

79-
base_dir: Path = Path(tempfile.gettempdir()) / "nominal-code"
79+
base_dir: Path = DEFAULT_BASE_DIR
8080

8181

8282
class RedisConfig(BaseModel):

app/nominal_code/config/kubernetes.py

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,6 @@ class KubernetesConfig(BaseModel):
2020
resource_requests_memory (str): Memory request (e.g. ``"512Mi"``).
2121
resource_limits_cpu (str): CPU limit.
2222
resource_limits_memory (str): Memory limit.
23-
pod_security_enabled (bool): Enable pod security hardening
24-
(``securityContext``, ``automountServiceAccountToken``).
25-
run_as_user (int): UID to run the container as when security is enabled.
26-
read_only_root_filesystem (bool): Mount the root filesystem as
27-
read-only when security is enabled.
28-
automount_service_account_token (bool): Whether to mount the service
29-
account token in job pods.
3023
"""
3124

3225
model_config = ConfigDict(frozen=True)
@@ -43,7 +36,3 @@ class KubernetesConfig(BaseModel):
4336
resource_requests_memory: str = ""
4437
resource_limits_cpu: str = ""
4538
resource_limits_memory: str = ""
46-
pod_security_enabled: bool = True
47-
run_as_user: int = 1000
48-
read_only_root_filesystem: bool = True
49-
automount_service_account_token: bool = False

app/nominal_code/config/loader.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@
99
ProviderConfig,
1010
resolve_agent_config,
1111
)
12-
from nominal_code.config.kubernetes import KubernetesConfig
13-
from nominal_code.config.models import AppSettings
14-
from nominal_code.config.policies import FilteringPolicy, RoutingPolicy
15-
from nominal_code.config.settings import (
12+
from nominal_code.config.config import (
1613
Config,
1714
PromptsConfig,
1815
RedisConfig,
@@ -25,6 +22,9 @@
2522
parse_reviewer_triggers,
2623
parse_title_tags,
2724
)
25+
from nominal_code.config.kubernetes import KubernetesConfig
26+
from nominal_code.config.models import AppSettings
27+
from nominal_code.config.policies import FilteringPolicy, RoutingPolicy
2828
from nominal_code.models import EventType, ProviderName
2929

3030
logger: logging.Logger = logging.getLogger(__name__)
@@ -319,7 +319,9 @@ def load_config_for_ci(
319319
)
320320

321321

322-
def _resolve_kubernetes(settings: AppSettings) -> KubernetesConfig | None:
322+
def _resolve_kubernetes(
323+
settings: AppSettings,
324+
) -> KubernetesConfig | None:
323325
"""
324326
Resolve KubernetesConfig from settings when a K8s image is configured.
325327

0 commit comments

Comments
 (0)