[mashkovd]

Summary

• Phase 4 of the dev-workflow control plane plan: new services/admins/mctl-agents-worker/ — a standalone Deployment (base-service chart, ingress.enabled: false) running the Temporal worker for DevLoopWorkflow (mctl-agents#105).
• Rides the same ghcr.io/mctlhq/mctl-agents image every Argo CWFT already uses, with command: ["python", "-m", "orchestrator.temporal.worker"] instead of the usual entrypoint — no separate build.
• probes.liveness/probes.readiness explicitly nulled: the base-service chart's own values.yaml defaults those to httpGet /healthz//readyz, which would crash-loop this pod against a port nothing listens on (this worker is a pure asyncio process, no HTTP server). Verified via helm template — first attempt (probes: {}) silently left the chart defaults in place since Helm deep-merges rather than replaces; had to null the two specific leaf keys.
• replicaCount: 0 on purpose: orchestrator/temporal/ only exists on mctl-agents#105 (unreleased) as of this PR. Deploying at today's pin (1.21.0) would crash-loop on ImportError: No module named orchestrator.temporal. Bump replicaCount to 1 and image.tag to the first release containing chore(mctl-agent): bump image to 1.7.0 (rollback resolver) #105 together, in one commit, once that release exists.
• MCTL_TOKEN sourced via a new ExternalSecret from the same Vault path (platform/mctl-agents, key mctl-token) the Argo CWFT pods already use for the same purpose.

Test plan

• python3 -c "import yaml; ..." on both new files — parses cleanly
• helm lint platform-gitops/helm-charts/base-service -f .../values.yaml — clean
• helm template ... -f .../values.yaml — renders correctly; confirmed no livenessProbe/readinessProbe block in the output
• Live, after this merges + ArgoCD syncs (~3 min): confirm the Application appears (replicas: 0, no pods — expected until the follow-up bump)
• Live, after the replicaCount+image.tag follow-up: confirm the worker pod reaches Running, connects to Temporal (temporal workflow list --namespace mctl-agents via admintools, or the Web UI), and a orchestrator.temporal.cli start <issue_url> run completes

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[claude]

Claude finished @mashkovd's task in 1m 32s —— View job

---

Code Review: mctl-agents-worker Deployment

Status: ✅ Approved

Summary

This PR scaffolds a new Temporal worker service (mctl-agents-worker) in the admins tenant using the existing base-service Helm chart. The configuration correctly handles the unique requirements of a non-HTTP asyncio worker process.

Analysis

Correctness:

• ✅ ApplicationSet auto-discovery: The service path platform-gitops/services/admins/mctl-agents-worker matches the ApplicationSet generator pattern (platform-gitops/services/*/*)
• ✅ Helm values structure: All required fields present and correctly formatted
• ✅ Probe nulling: probes.liveness: null and probes.readiness: null correctly prevent HTTP healthchecks on this non-HTTP process
• ✅ ExternalSecret format: externalSecret.data[].remoteKey and .property correctly map to the template's remoteRef.key and remoteRef.property
• ✅ Vault reference: Pulls MCTL_TOKEN from secret/platform/mctl-agents (same path used by Argo CWFT pods) via ExternalSecret, avoiding secrets in YAML

Operational Safety:

• ✅ Resource right-sizing: 50m/128Mi requests, 500m/256Mi limits appropriate for a lightweight polling worker (vs. full agent-worker pods that spawn CLI/clone repos)
• ✅ Probe comment clarity: Explains why probes are null (pure asyncio, no HTTP server) and the Helm deep-merge gotcha
• ✅ Replica count: replicaCount: 0 is safe and intentional — image tag 1.21.0 doesn't include orchestrator.temporal yet (feat(temporal): DevLoopWorkflow — issue -> investigate -> approve -> implement mctl-agents#105 unreleased). Comment correctly warns against splitting replicaCount+tag bumps across commits
• ✅ ServiceAccountToken: automountServiceAccountToken: false is correct — worker doesn't need API access
• ✅ No premature security hardening: Defers podSecurityContext/containerSecurityContext until worker is live (replicaCount > 0) to avoid guessing settings

Security:

• ✅ No plain-text secrets committed
• ✅ Proper ExternalSecret + Vault pattern followed
• ✅ No RBAC or access escalation risks
• ✅ Auto-deploy disabled (mctl.me/auto-deploy: false in catalog-info.yaml) — prevents accidental deployment before tag bump

Backstage Integration:

• ✅ catalog-info.yaml properly configured with team, lifecycle, links
• ✅ ArgoCD integration annotations correct
• ✅ Kubernetes discovery labels and namespace set properly

Test Coverage

PR author has verified:

• ✅ YAML syntax (python yaml parser)
• ✅ Helm lint clean
• ✅ Helm template rendering (confirmed nulled probes don't appear in output)
• ⏳ Live verification deferred to post-merge + follow-up PR (when image tag includes chore(mctl-agent): bump image to 1.7.0 (rollback resolver) #105)

Finding Summary

P1: 0 | P2: 0 | P3: 0

No blocking issues. Configuration follows codebase conventions, correctly uses ExternalSecret for secrets management, and includes appropriate comments explaining non-obvious decisions. Ready to merge.

---

[claude]

No P1/P2 findings (0 P3). Good to merge.