fix(cron): guard nil job and tenant-consistent readback in cron create [B24:2794] (#1438)

Creating a cron job over a connection with no tenant in context
(gateway-token / CLI / master-scope UI session) panicked with a
nil-pointer dereference.

Root cause — tenant asymmetry between insert and readback:
- AddJob INSERT uses tenantIDForInsert(ctx), which falls back to
  MasterTenantID when the context has no tenant (row inserted OK).
- The readback (scanJob / GetJob) rejects a nil-tenant context with
  "tenant_id required", so GetJob returned (nil, false) and AddJob
  swallowed it as (nil, nil).
- handleCreate then dereferenced job.ID (UpdateJob patch) → panic.

Fix (defense-in-depth):
- handleCreate (internal/gateway/methods/cron.go): add a nil-job guard
  before any job.ID use; return ErrInternal instead of panicking. Apply
  the post-create stateless/command patch under a tenant-consistent
  context (WithTenantID(ctx, job.TenantID)) so lockCronJobForMutation
  finds the row for nil-tenant callers.
- AddJob (store/pg + store/sqlitestore cron_crud.go): read back under
  WithTenantID(ctx, tenantIDForInsert(ctx)) so insert and readback share
  the same tenant; return an explicit error if readback fails instead of
  swallowing it.

Tests:
- internal/store/sqlitestore/cron_crud_test.go: AddJob with a nil-tenant
  context returns a non-nil job + nil error (real SQLite DB).
- internal/gateway/methods/cron_test.go: handleCreate does not panic and
  skips UpdateJob when the store returns (nil, nil).

Co-authored-by: DangTinh311 <dangtinh31193@gmail.com>

internal/gateway/methods/cron.go

@@ -107,6 +107,13 @@ func (m *CronMethods) handleCreate(ctx context.Context, client *gateway.Client,
 		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, err.Error()))
 		return
 	}
+	// Anti-panic guard: a store may return (nil, nil) if the insert succeeded but
+	// the tenant-scoped readback failed (e.g. tenant asymmetry). Never dereference
+	// a nil job below.
+	if job == nil {
+		client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInternal, i18n.T(locale, i18n.MsgInternalError, "cron job created but could not be loaded")))
+		return
+	}
 
 	// Apply extra fields not in AddJob signature via an immediate patch.
 	// Default stateless=true for new crons (saves tokens); override with explicit false.
@@ -122,7 +129,13 @@ func (m *CronMethods) handleCreate(ctx context.Context, client *gateway.Client,
 		if isCommand {
 			patch.Command = params.Command
 		}
-		if updated, pErr := m.service.UpdateJob(ctx, job.ID, patch); pErr == nil {
+		// Patch under a tenant-consistent context: the job was inserted under
+		// job.TenantID (which falls back to MasterTenantID for nil-tenant /
+		// master-scope connections). Using the raw ctx here would make
+		// lockCronJobForMutation miss the row for nil-tenant callers, silently
+		// dropping the stateless/command patch.
+		updateCtx := store.WithTenantID(ctx, job.TenantID)
+		if updated, pErr := m.service.UpdateJob(updateCtx, job.ID, patch); pErr == nil {
 			job = updated
 		}
 	}

internal/gateway/methods/cron_test.go

@@ -16,6 +16,7 @@ import (
 type stubCronStore struct {
 	jobs      map[string]*store.CronJob
 	addErr    error
+	addNilJob bool // simulate insert-ok-but-readback-failed: AddJob returns (nil, nil)
 	removeErr error
 	updateErr error
 	enableErr error
@@ -38,6 +39,9 @@ func (s *stubCronStore) AddJob(_ context.Context, name string, schedule store.Cr
 	if s.addErr != nil {
 		return nil, s.addErr
 	}
+	if s.addNilJob {
+		return nil, nil
+	}
 	job := &store.CronJob{
 		ID:       "new-job-id",
 		Name:     name,
@@ -219,6 +223,27 @@ func TestCronCreate_ValidParams_CreatesJob(t *testing.T) {
 	}
 }
 
+// Regression (B24:2794): when the store's AddJob returns (nil, nil) — the
+// insert succeeded but the tenant-scoped readback failed — handleCreate must
+// NOT dereference the nil job (previously panicked at job.ID). It must return
+// an error response and never call UpdateJob with a nil job.
+func TestCronCreate_NilJobFromStore_NoPanic_ReturnsError(t *testing.T) {
+	svc := newStubCronStore()
+	svc.addNilJob = true
+	m := buildCronMethods(t, svc)
+	client := nullClient()
+	req := cronReqFrame(t, protocol.MethodCronCreate, map[string]any{
+		"name":     "nil-job",
+		"message":  "do the thing",
+		"schedule": map[string]any{"kind": "every", "everyMs": 60000},
+	})
+	m.handleCreate(context.Background(), client, req)
+	// No panic = nil-guard hit before job.ID dereference.
+	if svc.updateCnt != 0 {
+		t.Fatalf("UpdateJob called %d times after nil job, want 0", svc.updateCnt)
+	}
+}
+
 // ---- Tests: handleDelete ----
 
 func TestCronDelete_MissingJobID_ReturnsInvalidRequest(t *testing.T) {

internal/store/pg/cron_crud.go

@@ -81,7 +81,17 @@ func (s *PGCronStore) AddJob(ctx context.Context, name string, schedule store.Cr
 
 	s.cacheLoaded = false // invalidate cache
 
-	job, _ := s.GetJob(ctx, id.String())
+	// Read back with a tenant-consistent context. The INSERT above uses
+	// tenantIDForInsert(ctx), which falls back to MasterTenantID when the
+	// caller has no tenant in context (gateway-token / CLI / master-scope UI).
+	// scanJob, however, rejects a nil-tenant context with "tenant_id required".
+	// Reading back under the same tenant the row was inserted with keeps the
+	// insert and readback symmetric, so we never swallow the created job.
+	readCtx := store.WithTenantID(ctx, tenantIDForInsert(ctx))
+	job, ok := s.GetJob(readCtx, id.String())
+	if !ok || job == nil {
+		return nil, fmt.Errorf("cron job %s created but readback failed", id)
+	}
 	return job, nil
 }
 

internal/store/sqlitestore/cron_crud.go

@@ -83,7 +83,17 @@ func (s *SQLiteCronStore) AddJob(ctx context.Context, name string, schedule stor
 	}
 
 	s.InvalidateCache()
-	job, _ := s.GetJob(ctx, id.String())
+
+	// Read back with a tenant-consistent context. The INSERT above uses
+	// tenantIDForInsert(ctx), which falls back to MasterTenantID when the
+	// caller has no tenant in context. scanJob rejects a nil-tenant context
+	// with "tenant_id required", so reading back under the same tenant the row
+	// was inserted with keeps insert and readback symmetric (matches pg store).
+	readCtx := store.WithTenantID(ctx, tenantIDForInsert(ctx))
+	job, ok := s.GetJob(readCtx, id.String())
+	if !ok || job == nil {
+		return nil, fmt.Errorf("cron job %s created but readback failed", id)
+	}
 	return job, nil
 }
 

internal/store/sqlitestore/cron_crud_test.go

@@ -300,6 +300,39 @@ func TestSQLiteCronStore_EnableJob_IgnoresMalformedPayload(t *testing.T) {
 	}
 }
 
+// TestSQLiteCronStore_AddJobNilTenantContext_ReturnsJob is a regression test for
+// the cron-create nil-pointer bug (B24:2794). A connection without a tenant in
+// context (gateway-token / CLI / master-scope UI session) inserts the row under
+// MasterTenantID via the insert-tenant fallback, but the readback previously
+// used the raw nil-tenant context and hit scanJob's "tenant_id required" guard.
+// AddJob therefore returned (nil, nil), which the handler dereferenced → panic.
+// The fix reads back under a tenant-consistent context, so AddJob must now
+// return a non-nil job with nil error.
+func TestSQLiteCronStore_AddJobNilTenantContext_ReturnsJob(t *testing.T) {
+	cronStore, _, _ := newTestSQLiteCronStore(t)
+	// Bare context: NO tenant, NO cross-tenant flag — exactly the shape a
+	// gateway-token / master-scope connection produces.
+	ctx := context.Background()
+	everyMS := int64(time.Minute / time.Millisecond)
+
+	job, err := cronStore.AddJob(ctx, "job-nil-tenant", store.CronSchedule{
+		Kind:    "every",
+		EveryMS: &everyMS,
+	}, "hello", false, "", "", "", "user-1")
+	if err != nil {
+		t.Fatalf("AddJob error: %v", err)
+	}
+	if job == nil {
+		t.Fatal("AddJob returned (nil, nil) for nil-tenant context — readback swallowed the created job")
+	}
+	if job.TenantID != store.MasterTenantID {
+		t.Fatalf("job tenant = %v, want MasterTenantID %v", job.TenantID, store.MasterTenantID)
+	}
+	if job.Name != "job-nil-tenant" {
+		t.Fatalf("job name = %q, want %q", job.Name, "job-nil-tenant")
+	}
+}
+
 func newTestSQLiteCronStore(t *testing.T) (*SQLiteCronStore, context.Context, *sql.DB) {
 	t.Helper()