Skip to content

feat(telegram): add demo session refresh CronJob - #334

Merged
mashkovd merged 2 commits into
mainfrom
feat/telegram-demo-session-refresh-cron
May 30, 2026
Merged

mashkovd merged 2 commits into
mainfrom
feat/telegram-demo-session-refresh-cron

Conversation

@mashkovd

Copy link
Copy Markdown
Member

What

Adds a daily CronJob (labs-mctl-telegram-demo-session-refresh) via the base-service extraObjects hook that keeps the ChatGPT App Directory demo/reviewer Telegram session row perpetually fresh.

Why

On 2026-05-30 the reviewer login at tg.mctl.ai dead-ended with "Sign-in interrupted — The demo account has no active Telegram session."

Root cause (confirmed via DB + audit chain, not a reviewer disconnect): the newest telegram_accounts row for the demo account (8745115872 / users.id 6462) had expires_at = connected_at + 24h (artifact of a manual re-seed), while every other row had the normal +90d. The 24h TTL lapsed and the automatic absolute-TTL sweeper (SweepAbsoluteSessions / CheckSessionValid) revoked the only row (~30 min after expiry, no audit entry). With no non-revoked row, CheckSessionValid returns ErrNoActiveSession and demo_login.go renders the error page.

The session was restored manually (un-revoke + 90d TTL; verified live with a real list_dialogs). This CronJob prevents recurrence during the review window.

How

  • schedule: 17 3 * * * (daily, off-peak UTC).
  • Un-revokes the newest demo session row, pushes expires_at/last_used_at out 90d, forces send_enabled=false.
  • Reuses postgres:17-alpine + the existing labs-mctl-telegram-db-creds secret (same pattern as the chart's dbInitJob).
  • Targets the demo row via users.telegram_login_id = '8745115872' (robust to new row ids).

Scope / safety

  • image.tag unchanged (0.40.0) — auxiliary infra, not a mctl-telegram release; the 0.40.0 freeze is respected.
  • Does not re-login Telegram-side; if the MTProto session is signed out at Telegram, a manual mctl-telegram-login re-seed is still required.
  • Temporary — remove with DEMO_REVIEWER_ENABLED once the durable fix ships post-freeze (make the DEMO_REVIEWER_TG_ID identity immune to TTL sweeps, mirroring the evaluateSendGate reviewer override).

Validation

  • helm template renders the CronJob cleanly; app Deployment/image unchanged.

The demo/reviewer Telegram session row is subject to the normal session
TTLs (absolute 90d, idle 30d) and revoke-on-disconnect. On 2026-05-30 a
short 24h expires_at lapsed and the absolute-TTL sweeper revoked the only
row, leaving CheckSessionValid with no active session -> reviewers hit
"Sign-in interrupted: no active Telegram session" at tg.mctl.ai.

Add a daily CronJob (via base-service extraObjects) that un-revokes the
newest demo session row and pushes expires_at/last_used_at out 90d while
forcing send_enabled=false, so the reviewer login stays usable for the
whole App Directory review window.

Temporary: remove with DEMO_REVIEWER_ENABLED once the durable fix ships
post-freeze (make the DEMO_REVIEWER_TG_ID identity immune to TTL sweeps,
mirroring the evaluateSendGate reviewer override). image.tag unchanged
(0.40.0) -- this is auxiliary infra, not a release.
@claude

claude Bot commented May 30, 2026 •

Copy link
Copy Markdown

Claude finished @mashkovd's task in 2m 18s —— View job


Review: demo session refresh CronJob

  • Read PR diff and changed values.yaml
  • Verify extraObjects hook + DATABASE_URL secret reference resolve correctly
  • Check ResourceQuota / LimitRange admission impact
  • Post findings

Findings

Correctness — verified OK

  • extraObjects renders via base-service/templates/extra-objects.yaml (tpl (toYaml $obj) $); the args contain no {{ }} so there is no template-vs-shell conflict.
  • secretKeyRef → labs-mctl-telegram-db-creds / DATABASE_URL is valid: the key is produced by externalsecret-db.yaml, and dbSecret.secretName matches.
  • The labs LimitRange sets default/defaultRequest, so the resource-less pod still admits under the namespace ResourceQuota — the CronJob will actually run.
  • CronJob hygiene is good: concurrencyPolicy: Forbid, startingDeadlineSeconds, activeDeadlineSeconds, backoffLimit, history limits. SQL targets the demo row by telegram_login_id (robust to new ids) and forces send_enabled=false. image.tag is untouched, so the 0.40.0 freeze is respected.

P3 (non-blocking)

  • ghcr-credentials imagePullSecret is unnecessary for the public Docker Hub postgres:17-alpine image.
  • No explicit resources / securityContext on the container (relies on LimitRange defaults; consistent with dbInitJob).

The TEMPORARY / REMOVE comment block clearly documents this as a review-window workaround with a follow-up plan, which is the right call.

Verdict

P1: 0 · P2: 0 · P3: 2 — No P1/P2 findings, good to merge.

Comment on lines +207 to +208
imagePullSecrets:
- name: ghcr-credentials

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 — postgres:17-alpine is pulled from public Docker Hub, so the ghcr-credentials imagePullSecret is unnecessary here (the chart's dbInitJob references it because it can run app images from GHCR). Harmless, but you can drop it to avoid implying a GHCR dependency.

Comment on lines +210 to +211
- name: refresh
image: postgres:17-alpine

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 — The container sets no resources or securityContext. The labs LimitRange provides default/defaultRequest, so the pod will still admit under the namespace ResourceQuota (no blocker), and this mirrors the existing dbInitJob. Optional hardening for a long-lived CronJob: add small explicit requests/limits and a securityContext (runAsNonRoot: true, readOnlyRootFilesystem: true, allowPrivilegeEscalation: false).

postgres:17-alpine is a public Docker Hub image; the ghcr-credentials
secret is only relevant to GHCR app images (as in dbInitJob). Addresses
the P3 review nit on PR #334.
@claude

claude Bot commented May 30, 2026 •

Copy link
Copy Markdown

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


Re-review: demo session refresh CronJob (after sync)

  • Read updated diff (b15ae5a..HEAD)
  • Verify the ghcr imagePullSecret was dropped (prior P3)
  • Re-check correctness / security / operational safety
  • Post findings

What changed since last review

The latest commit (6da6b6a) drops the ghcr-credentials imagePullSecret and adds a comment explaining why (postgres:17-alpine is a public Docker Hub image). This resolves the first P3 from the prior review.

Findings — still clean

  • extraObjects rendering — base-service/templates/extra-objects.yaml does tpl (toYaml $obj) $. The CronJob args SQL contains no {{ }} braces, so the tpl pass leaves the $DATABASE_URL shell var and interval '90 days' literals untouched. No template/shell conflict.
  • Secret reference — externalsecret-db.yaml emits a secret with key DATABASE_URL (default name <fullname>-db-creds), matching the CronJob's secretKeyRef → labs-mctl-telegram-db-creds / DATABASE_URL.
  • Operational safety — concurrencyPolicy: Forbid, startingDeadlineSeconds, activeDeadlineSeconds: 120, backoffLimit: 2, history limits all present. SQL uses ON_ERROR_STOP=1, targets the demo row by telegram_login_id (robust to new ids), and forces send_enabled=false. image.tag untouched → 0.40.0 freeze respected.
  • Scope — the TEMPORARY / REMOVE block clearly documents this as a review-window workaround with a concrete follow-up plan.

Remaining (non-blocking, P3)

  • Still no explicit resources / securityContext on the container. It admits fine via the labs LimitRange defaults and mirrors the existing dbInitJob, so this is optional hardening only.

Verdict

P1: 0 · P2: 0 · P3: 1 — No P1/P2 findings, good to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant