feat: extend Job Handler

[gauthierdmn]

No description provided.

[nominalbot]

Done, no output.

[gauthierdmn]

@nominalbot review this PR

[gauthierdmn]

@nominalbot review this PR

[gauthierdmn]

@nominalbot review this PR

[nominalbot]

I completed my analysis but failed to produce a structured review. You can re-trigger the review by mentioning me again. If the issue persists, contact your administrator.

[gauthierdmn]

@nominalbot review this PR

[nominalbot]

This PR introduces significant improvements in security, configuration, and modularity. The two-layer configuration architecture, webhook server decomposition, and the new DispatchResult and JobHandler protocol are excellent for maintainability and extensibility. The security hardening measures are crucial. The documentation is also well-written and insightful.

Additional notes (not in diff):

• config/settings.py:1 — Consider adding a require_webhook() method to the Config class (as described in "Proposed Improvements feat: add core types, configuration, and utilities #1" in BRANCH_ANALYSIS.md) to eliminate the need for # type: ignore[assignment] in commands/webhook/server.py and other modules that rely on config.webhook being present in a webhook context. This would make the code cleaner and typesafe.
• commands/webhook/server.py:418 — Replace config.webhook with config.require_webhook() (once implemented) to improve type safety and adhere to the proposed improvement in BRANCH_ANALYSIS.md.
• handlers/review.py:296 — Remove the explicit build_sanitized_env() call from this file, as invoke_agent() / run_api_agent() can handle it internally as a secure default. This aligns with "Proposed Improvements feat: add platform abstraction with GitHub and GitLab support #2" in BRANCH_ANALYSIS.md to reduce redundant boilerplate.
• agent/api/tools.py:1 — Add an explicit check for file:// protocol at the top of _validate_clone_host() to further harden against potential bypasses of Git's protocol.file.allow=never config, as suggested in "Proposed Improvements feat: add agent runner, git workspace, and session management #3" in BRANCH_ANALYSIS.md.
• agent/prompts.py:1 — Implement the fix for tag injection in wrap_tag() by escaping closing tags (</tag-name>) in the content, as detailed in "Proposed Improvements feat: add comment handlers and workspace cleanup #4" in BRANCH_ANALYSIS.md. This is a critical security hardening.
• jobs/runner/base.py:65 — Replace SystemExit(1) with a ValueError (or a custom ConfigurationError) when Redis is missing for K8s mode, as outlined in "Proposed Improvements feat: add webhook server and entry point #5" in BRANCH_ANALYSIS.md. This allows for more robust error handling and testing.
• config/policies.py:1 — Add Pydantic @model_validators to RoutingPolicy to prevent worker_bot_username == reviewer_bot_username and ensure allowed_users is not empty when authorization is required in webhook mode. This is suggested in "Proposed Improvements feat: add cross-file review with diff-aware finding routing #6" in BRANCH_ANALYSIS.md for better validation.
• agent/sandbox.py:1 — Consolidate secret pattern detection by moving TOKEN_PATTERN from workspace/git.py into this file alongside SECRET_PATTERNS, and expose a redact_url() helper. This aligns with "Proposed Improvements feat: add CLI mode for one-off PR reviews #7" in BRANCH_ANALYSIS.md to centralize token redaction logic.
• workspace/git.py:1 — Update this module to import the consolidated redact_url() helper from agent/sandbox.py instead of defining TOKEN_PATTERN locally, as described in "Proposed Improvements feat: add CLI mode for one-off PR reviews #7" in BRANCH_ANALYSIS.md.
• tests/commands/webhook/test_server.py:1 — Add spec=Platform, spec=JobRunner, etc., to mock constructors in this file as suggested in "Proposed Improvements chore: misc cleanup #8" in BRANCH_ANALYSIS.md. This helps prevent silent failures when mock methods are called that don't exist on the real object, improving test robustness.
• docs/deployment/index.md:1 — Add an explicit warning about disk accumulation in non-K8s mode and suggest a cron job for manual cleanup (e.g., find /tmp/nominal-code -maxdepth 3 -mtime +7 -type d -exec rm -rf {} +). This is proposed in "Proposed Improvements chore: improve README.md #9" in BRANCH_ANALYSIS.md since workspace cleanup was removed.