From 6ec31b0bb9ee9a93033cde2a36f867b1719900fa Mon Sep 17 00:00:00 2001
From: christophervoelpel
 <123032885+christophervoelpel@users.noreply.github.com>
Date: Mon, 21 Sep 2026 18:26:35 +0000
Subject: [PATCH 1/7] Remove the duplicate GCS signing cache from orch.py

orch.py and util/gcs_wrapper.py contained two copies of the same GCS IAM signing-credential caching logic. PR #195 fixed only the gcs_wrapper.py copy, leaving the hot path in orch.py (upload URLs, single GETs, and batch signing) running the stale, un-locked copy with dead refresh-on-expiry logic.

Delete the duplicate caching logic in orch.py and delegate to the single public get_signing_context() in util/gcs_wrapper.py. In addition, fix util/gcs_wrapper.py to detect non-service-account ADC credentials from local development and raise a clear, actionable RuntimeError explaining that URL signing requires a service account identity (pointing to impersonation or service account key instructions in DEVELOPING.md) rather than raising an unhelpful AttributeError.

TAG=agy
CONV=ec862d59-a9a7-4a7c-9eae-b59095e8cd08
---
 orch.py                     | 51 ++++++---------------------
 test/test_frontdoor_data.py | 56 +++++++++++++++++++++++++-----
 test/test_gcs_wrapper.py    | 69 +++++++++++++++++++++++++++++++++++++
 util/gcs_wrapper.py         | 20 ++++++++---
 4 files changed, 143 insertions(+), 53 deletions(-)

diff --git a/orch.py b/orch.py
index 9b6123a..58ee33d 100644
--- a/orch.py
+++ b/orch.py
@@ -65,17 +65,15 @@
 from werkzeug.exceptions import RequestEntityTooLarge
 from flask_cors import CORS
 from google.api_core import exceptions as google_exceptions
-from google.auth import compute_engine
-from google.auth import default as google_auth_default
 from google.auth.transport import requests as google_auth_requests
 from google.cloud import firestore
 from google.cloud.firestore_v1 import field_path
-from google.cloud import storage
 from google.oauth2 import id_token as google_id_token
 import orchestrator
 import transcription
 from util import database as util_database
 from util import errors as util_errors
+from util import gcs_wrapper
 from util import model_allowlist
 from util import submission_validation
 from werkzeug.security import safe_join
@@ -692,9 +690,8 @@ def get_status_handler() -> flask_response:
 # ---------------------------------------------------------------------------
 # Mediated data plane (ROLE='app'): signed-URL minting for GCS plus CRUD on
 # the UI Firestore database, so the SPA can run without direct Firestore or
-# Storage access. Module-level lazy singletons keep the per-request cost at
-# ~1 IAM signBlob RPC per unique path (the per-call construction in
-# util.gcs_wrapper.get_signed_url costs ~3 RPCs).
+# Storage access. Storage client and IAM signing credentials delegate to the
+# cached signing context in util.gcs_wrapper.
 # ---------------------------------------------------------------------------
 
 _SIGNED_GET_TTL = datetime.timedelta(hours=24)
@@ -732,17 +729,7 @@ def _is_allowed_upload_content_type(content_type: str) -> bool:
       _ALLOWED_UPLOAD_TYPE_PREFIXES
   )
 
-_storage_client = None
 _ui_db = None
-_signing_credentials = None
-
-
-def _get_storage_client() -> storage.Client:
-  """Returns the lazily-built module-level Cloud Storage client."""
-  global _storage_client
-  if _storage_client is None:
-    _storage_client = storage.Client()
-  return _storage_client
 
 
 def _get_ui_db() -> firestore.Client | None:
@@ -759,31 +746,11 @@ def _get_ui_db() -> firestore.Client | None:
   return _ui_db
 
 
-def _get_signing_credentials() -> compute_engine.IDTokenCredentials:
-  """Returns cached IAM-signing credentials, rebuilding them on expiry.
-
-  Inlines the flask-context mechanics of util.gcs_wrapper.get_signed_url:
-  the metadata-server credentials carry no private key, so URL signing
-  goes through the IAM signBlob API via compute_engine.IDTokenCredentials
-  (requires roles/iam.serviceAccountTokenCreator on the runtime SA).
-  """
-  global _signing_credentials
-  if _signing_credentials is None or _signing_credentials.expired:
-    auth_request = google_auth_requests.Request()
-    source_credentials, _ = google_auth_default()
-    source_credentials.refresh(auth_request)
-    _signing_credentials = compute_engine.IDTokenCredentials(
-        auth_request,
-        '',
-        service_account_email=source_credentials.service_account_email,  # pyright: ignore[reportAttributeAccessIssue]
-    )
-  return _signing_credentials
-
-
 def _signed_url(
     blob, method: str, expiration: datetime.timedelta, content_type=None
 ) -> str:
-  """Generates a V4 signed URL for the blob with the cached credentials."""
+  """Generates a V4 signed URL for the blob with cached IAM credentials."""
+  _, credentials = gcs_wrapper.get_signing_context()
   kwargs = {}
   if content_type is not None:
     kwargs['content_type'] = content_type
@@ -791,7 +758,7 @@ def _signed_url(
       version='v4',
       expiration=expiration,
       method=method,
-      credentials=_get_signing_credentials(),
+      credentials=credentials,
       **kwargs,
   )
 
@@ -999,7 +966,8 @@ def upload_url_handler() -> flask_response:
   if not bucket_name:
     return _json_error('gcsBucket not configured', 500)
   object_path = f'{prefix}/{file_name}'
-  blob = _get_storage_client().bucket(bucket_name).blob(object_path)
+  storage_client, _ = gcs_wrapper.get_signing_context()
+  blob = storage_client.bucket(bucket_name).blob(object_path)
   exists = blob.exists()
   upload_url = None
   if not exists:
@@ -1061,7 +1029,8 @@ def sign_url_handler() -> flask_response:
     if not ttl_arg.isdecimal() or not 1 <= int(ttl_arg) <= 86400:
       return _json_error('ttl must be 1-86400 seconds', 400)
     ttl = datetime.timedelta(seconds=int(ttl_arg))
-  bucket = _get_storage_client().bucket(bucket_name)
+  storage_client, _ = gcs_wrapper.get_signing_context()
+  bucket = storage_client.bucket(bucket_name)
   urls = {
       path: _signed_url(bucket.blob(path), 'GET', ttl) for path in paths
   }
diff --git a/test/test_frontdoor_data.py b/test/test_frontdoor_data.py
index 7eff0bf..ce74d55 100644
--- a/test/test_frontdoor_data.py
+++ b/test/test_frontdoor_data.py
@@ -17,10 +17,9 @@
 Follows the fixture pattern of test/test_frontdoor.py: env vars are set
 via monkeypatch BEFORE (re)importing orch, and orchestrator's import-time
 side effects are neutralised by the shared orchestrator_module fixture.
-The GCS and UI-Firestore
-clients are replaced with in-memory fakes by presetting orch's lazy
-module-level singletons, and the cached signing-credentials factory is
-stubbed so no test performs network I/O.
+The UI-Firestore client is replaced with an in-memory fake by presetting
+orch's lazy module-level singleton, and GCS storage/signing context is
+stubbed on util.gcs_wrapper so no test performs network I/O.
 """
 
 import copy
@@ -37,6 +36,7 @@
 
 # Reused module-scoped fixture; pytest picks it up from this namespace.
 from test.test_frontdoor import orchestrator_module  # noqa: F401  pylint: disable=unused-import
+from util import gcs_wrapper
 from util import model_allowlist
 
 _REPO = pathlib.Path(__file__).resolve().parent.parent
@@ -376,8 +376,11 @@ def _load_app(monkeypatch, **env):
   fake_db = FakeUiDb()
   fake_storage = FakeStorageClient()
   monkeypatch.setattr(orch, '_ui_db', fake_db)
-  monkeypatch.setattr(orch, '_storage_client', fake_storage)
-  monkeypatch.setattr(orch, '_get_signing_credentials', lambda: None)
+  monkeypatch.setattr(
+      gcs_wrapper,
+      'get_signing_context',
+      lambda: (fake_storage, None),
+  )
   monkeypatch.setitem(orch.config, 'gcsBucket', _BUCKET)
   return orch, fake_db, fake_storage
 
@@ -2041,8 +2044,11 @@ def test_data_endpoints_fail_soft_without_firestore_db_ui(
   assert response.get_json() == {'error': 'FIRESTORE_DB_UI not configured'}
 
   # The storage endpoints do not need the UI database.
-  monkeypatch.setattr(orch, '_storage_client', FakeStorageClient())
-  monkeypatch.setattr(orch, '_get_signing_credentials', lambda: None)
+  monkeypatch.setattr(
+      gcs_wrapper,
+      'get_signing_context',
+      lambda: (FakeStorageClient(), None),
+  )
   monkeypatch.setitem(orch.config, 'gcsBucket', _BUCKET)
   assert client.get('/api/signUrl?path=remix-input/a.png').status_code == 200
 
@@ -2061,3 +2067,37 @@ def test_worker_role_has_no_data_routes(monkeypatch, orchestrator_module):
   assert client.get('/api/projects').status_code == 404
   assert client.post('/api/uploadUrl', json={}).status_code == 404
   assert client.get('/api/config').status_code == 404
+
+
+def test_orch_signed_url_routes_through_gcs_wrapper_without_second_cache(
+    monkeypatch, orchestrator_module
+):
+  """orch delegates signed-URL generation to util.gcs_wrapper with no second cache."""
+  del orchestrator_module
+  orch, _, fake_storage = _load_app(monkeypatch)
+
+  # Assert orch has no duplicate cache state or functions
+  assert not hasattr(orch, '_storage_client')
+  assert not hasattr(orch, '_signing_credentials')
+  assert not hasattr(orch, '_get_signing_credentials')
+
+  blob = fake_storage.bucket(_BUCKET).blob('remix-input/asset.png')
+  sentinel_creds = object()
+  called_with_creds = []
+
+  def fake_get_signing_context():
+    return fake_storage, sentinel_creds
+
+  def fake_generate_signed_url(credentials=None, **kwargs):
+    del kwargs
+    called_with_creds.append(credentials)
+    return 'https://signed.example/url'
+
+  monkeypatch.setattr(
+      gcs_wrapper, 'get_signing_context', fake_get_signing_context
+  )
+  monkeypatch.setattr(blob, 'generate_signed_url', fake_generate_signed_url)
+
+  url = orch._signed_url(blob, 'GET', datetime.timedelta(hours=1))
+  assert url == 'https://signed.example/url'
+  assert called_with_creds == [sentinel_creds]
diff --git a/test/test_gcs_wrapper.py b/test/test_gcs_wrapper.py
index ecd347b..078bf43 100644
--- a/test/test_gcs_wrapper.py
+++ b/test/test_gcs_wrapper.py
@@ -15,6 +15,7 @@
 """Tests for the Cloud Storage wrapper."""
 
 import datetime
+import threading
 from unittest import mock
 
 import pytest
@@ -146,3 +147,71 @@ def test_get_signed_url_flask_context_caches_client_and_credentials_until_expire
     assert url3 == 'https://storage.googleapis.com/signed'
     assert client_factory.call_count == 1
     assert mock_cred.refresh.call_count == 2
+
+
+def test_signing_context_built_once_across_concurrent_callers():
+  mock_client = mock.Mock()
+  mock_cred = mock.Mock()
+  mock_cred.service_account_email = 'sa@example.com'
+  mock_cred.expiry = datetime.datetime.now() + datetime.timedelta(hours=1)
+  mock_default = mock.Mock(return_value=(mock_cred, 'project-id'))
+
+  with mock.patch(
+      'util.gcs_wrapper.storage.Client', return_value=mock_client
+  ) as client_factory, mock.patch(
+      'util.gcs_wrapper.default', mock_default
+  ), mock.patch(
+      'util.gcs_wrapper.iam.Signer'
+  ) as signer_factory:
+    num_threads = 10
+    barrier = threading.Barrier(num_threads)
+    results = [None] * num_threads
+
+    def worker(idx):
+      barrier.wait()
+      results[idx] = gcs_wrapper.get_signing_context()
+
+    threads = [
+        threading.Thread(target=worker, args=(i,)) for i in range(num_threads)
+    ]
+    for t in threads:
+      t.start()
+    for t in threads:
+      t.join()
+
+    assert client_factory.call_count == 1
+    assert mock_default.call_count == 1
+    assert mock_cred.refresh.call_count == 1
+    assert signer_factory.call_count == 1
+
+    expected_client, expected_creds = results[0]
+    for client, creds in results:
+      assert client is expected_client
+      assert creds is expected_creds
+
+
+def test_non_service_account_adc_raises_actionable_error():
+  # Simulates an end-user OAuth credential from `gcloud auth application-default login`
+  # which has no service_account_email attribute.
+  user_cred = mock.NonCallableMock(spec=['token', 'refresh'])
+  assert not hasattr(user_cred, 'service_account_email')
+
+  mock_client = mock.Mock()
+  mock_default = mock.Mock(return_value=(user_cred, 'project-id'))
+
+  with mock.patch(
+      'util.gcs_wrapper.storage.Client', return_value=mock_client
+  ) as client_factory, mock.patch(
+      'util.gcs_wrapper.default', mock_default
+  ):
+    # Must raise a clear, actionable RuntimeError, not an unhelpful AttributeError.
+    with pytest.raises(RuntimeError) as exc_info:
+      gcs_wrapper.get_signed_url('bucket', 'file.mp4', flask_context=True)
+
+    err_msg = str(exc_info.value)
+    assert 'service_account_email' in err_msg
+    assert 'gcloud auth application-default login' in err_msg
+    assert '--impersonate-service-account' in err_msg
+    assert 'DEVELOPING.md' in err_msg
+    assert client_factory.call_count == 1
+    assert mock_default.call_count == 1
diff --git a/util/gcs_wrapper.py b/util/gcs_wrapper.py
index 2b99364..4300fcd 100644
--- a/util/gcs_wrapper.py
+++ b/util/gcs_wrapper.py
@@ -48,7 +48,8 @@
 _SIGNING_CONTEXT_LOCK = threading.Lock()
 
 
-def _get_cached_signing_context():
+def get_signing_context():
+  """Returns cached (storage_client, signing_credentials) for GCS URL signing."""
   global _CACHED_STORAGE_CLIENT, _CACHED_CLIENT_FACTORY
   global _CACHED_SIGNING_CREDENTIALS, _CACHED_AUTH_FACTORY
 
@@ -63,16 +64,27 @@ def _get_cached_signing_context():
     ):
       auth_request = transport_requests.Request()
       cred, _ = default()
+      sa_email = getattr(cred, 'service_account_email', None)
+      if not sa_email:
+        raise RuntimeError(
+            'GCS URL signing requires a service account identity, but the active '
+            'credentials have no service_account_email. User credentials from '
+            '"gcloud auth application-default login" cannot sign URLs; configure '
+            'service account impersonation ("gcloud auth application-default login '
+            '--impersonate-service-account=<SA_EMAIL>") or set '
+            'GOOGLE_APPLICATION_CREDENTIALS to a service account key file. '
+            'See DEVELOPING.md ("The local loop") for details.'
+        )
       cred.refresh(auth_request)  # pyright: ignore[reportAttributeAccessIssue]
       signer = iam.Signer(
           auth_request,
           cred,
-          cred.service_account_email,  # pyright: ignore[reportAttributeAccessIssue]
+          sa_email,
       )
       signing_creds = compute_engine.IDTokenCredentials(
           auth_request,
           '',
-          service_account_email=cred.service_account_email,  # pyright: ignore[reportAttributeAccessIssue]
+          service_account_email=sa_email,
           signer=signer,
       )
       if isinstance(getattr(cred, 'expiry', None), datetime.datetime):
@@ -96,7 +108,7 @@ def get_signed_url(
     A signed URL to the input file.
   """
   if flask_context:
-    storage_client, signing_credentials = _get_cached_signing_context()
+    storage_client, signing_credentials = get_signing_context()
     bucket = storage_client.bucket(gcs_bucket_name)
     blob = bucket.blob(blob_name)
     return blob.generate_signed_url(

From b50f5a2a27a07b77ba4ab87ba78723b186f02741 Mon Sep 17 00:00:00 2001
From: christophervoelpel
 <123032885+christophervoelpel@users.noreply.github.com>
Date: Mon, 21 Sep 2026 18:39:58 +0000
Subject: [PATCH 2/7] Treat an absent storyboard key as no change

SM-6: In orch.py, _write_project_doc and _write_editor_project_doc
previously coerced an absent storyboard key in a PATCH payload to [],
causing keep_ids to be empty and deleting every scene document in the
scenes subcollection.

Distinguish absent storyboard from an explicit empty list. When
'storyboard' is omitted from the PATCH payload, leave the scenes
subcollection completely untouched. An explicit 'storyboard': []
continues to delete all scenes. Existing create semantics are preserved.

Reachability:
Not reachable from the shipped UI: there is exactly one project PATCH call
site (ui/.../config.ts:1293) and it always sends a full cloned ProjectConfig
where storyboard is required (config.ts:295) and initialised to []
(config.ts:604). Every .patch( in ui/src, scripts and tools was audited.
It matters because a partial PATCH is the most natural third-party call to
make against a documented endpoint.

Stacked on #198.
---
 orch.py                     |  57 ++++++------
 test/test_frontdoor_data.py | 178 ++++++++++++++++++++++++++++++++++++
 2 files changed, 209 insertions(+), 26 deletions(-)

diff --git a/orch.py b/orch.py
index 58ee33d..504b0e5 100644
--- a/orch.py
+++ b/orch.py
@@ -1175,22 +1175,24 @@ def _write_project_doc(
   precondition, scene writes retain their existing set behavior, and the
   stale-scene prune scan is skipped: a brand-new project has no prior scenes
   to prune, and callers cap create at _MAX_CREATE_SCENES so this always fits
-  one atomic batch.
+  one atomic batch. When create is false, an omitted storyboard key leaves the
+  scenes subcollection untouched.
   """
-  scenes = payload.get('storyboard')
-  if not isinstance(scenes, list):
-    scenes = []
   root = dict(payload)
   root['storyboard'] = []
-  scenes_ref = doc_ref.collection(_SCENES_SUBCOLLECTION)
   ops = [('create' if create else 'set', doc_ref, root)]
-  for index, scene in enumerate(scenes):
-    ops.append(('set', scenes_ref.document(_scene_doc_id(index)), scene))
-  if not create:
-    keep_ids = {_scene_doc_id(i) for i in range(len(scenes))}
-    for snapshot in scenes_ref.stream():
-      if snapshot.id not in keep_ids:
-        ops.append(('delete', scenes_ref.document(snapshot.id), None))
+  if create or 'storyboard' in payload:
+    scenes = payload.get('storyboard')
+    if not isinstance(scenes, list):
+      scenes = []
+    scenes_ref = doc_ref.collection(_SCENES_SUBCOLLECTION)
+    for index, scene in enumerate(scenes):
+      ops.append(('set', scenes_ref.document(_scene_doc_id(index)), scene))
+    if not create:
+      keep_ids = {_scene_doc_id(i) for i in range(len(scenes))}
+      for snapshot in scenes_ref.stream():
+        if snapshot.id not in keep_ids:
+          ops.append(('delete', scenes_ref.document(snapshot.id), None))
   _commit_in_batches(ui_db, ops)
 
 
@@ -1205,29 +1207,32 @@ def _write_editor_project_doc(
   The root update is deliberately field-based: unlike a read-modify-write
   replacement, it cannot copy a stale inputConfig snapshot over a concurrent
   Setup save. Fields omitted by the editor payload retain the legacy full-save
-  replacement behavior through DELETE_FIELD updates.
+  replacement behavior through DELETE_FIELD updates, except storyboard which
+  leaves the scenes subcollection untouched when omitted.
   """
   root_updates = {
       field_path.FieldPath(key).to_api_repr(): firestore.DELETE_FIELD
       for key in stored
-      if key not in payload and key != 'inputConfig'
+      if key not in payload and key not in ('inputConfig', 'storyboard')
   }
   root_updates.update({
       field_path.FieldPath(key).to_api_repr(): value
       for key, value in payload.items()
   })
-  scenes = payload.get('storyboard')
-  if not isinstance(scenes, list):
-    scenes = []
-  root_updates[field_path.FieldPath('storyboard').to_api_repr()] = []
-  scenes_ref = doc_ref.collection(_SCENES_SUBCOLLECTION)
-  ops = [('update', doc_ref, root_updates)]
-  for index, scene in enumerate(scenes):
-    ops.append(('set', scenes_ref.document(_scene_doc_id(index)), scene))
-  keep_ids = {_scene_doc_id(i) for i in range(len(scenes))}
-  for snapshot in scenes_ref.stream():
-    if snapshot.id not in keep_ids:
-      ops.append(('delete', scenes_ref.document(snapshot.id), None))
+  ops = []
+  if 'storyboard' in payload:
+    scenes = payload.get('storyboard')
+    if not isinstance(scenes, list):
+      scenes = []
+    root_updates[field_path.FieldPath('storyboard').to_api_repr()] = []
+    scenes_ref = doc_ref.collection(_SCENES_SUBCOLLECTION)
+    for index, scene in enumerate(scenes):
+      ops.append(('set', scenes_ref.document(_scene_doc_id(index)), scene))
+    keep_ids = {_scene_doc_id(i) for i in range(len(scenes))}
+    for snapshot in scenes_ref.stream():
+      if snapshot.id not in keep_ids:
+        ops.append(('delete', scenes_ref.document(snapshot.id), None))
+  ops.insert(0, ('update', doc_ref, root_updates))
   _commit_in_batches(ui_db, ops)
 
 
diff --git a/test/test_frontdoor_data.py b/test/test_frontdoor_data.py
index ce74d55..db71f0c 100644
--- a/test/test_frontdoor_data.py
+++ b/test/test_frontdoor_data.py
@@ -31,6 +31,8 @@
 import threading
 import uuid
 
+import pytest
+
 from google.api_core import exceptions as google_exceptions
 from google.cloud import firestore
 
@@ -1662,6 +1664,182 @@ def test_project_patch_prunes_removed_scenes(monkeypatch, orchestrator_module):
   body = client.get('/api/projects/shrink').get_json()
   assert [s['d'] for s in body['storyboard']] == ['0']
 
+@pytest.mark.parametrize(
+    'path_template',
+    [
+        '/api/projects/{id}',
+        '/api/projects/{id}/editor',
+        '/api/projects/{id}?view=editor',
+    ],
+)
+def test_project_patch_omitting_storyboard_leaves_scenes_intact(
+    monkeypatch, orchestrator_module, path_template
+):
+  del orchestrator_module
+  orch, fake_db, _ = _load_app(monkeypatch)
+  client = orch.app.test_client()
+  project_id = f'keep-scenes-{uuid.uuid4().hex[:8]}'
+
+  assert (
+      client.post(
+          '/api/projects',
+          json={
+              'id': project_id,
+              'name': 'Original',
+              'storyboard': [{'d': '0'}, {'d': '1'}, {'d': '2'}],
+          },
+      ).status_code
+      == 200
+  )
+  assert len(_scenes_docs(fake_db, project_id)) == 3
+
+  # PATCH {"name": "Renamed"} must leave all 3 scenes intact (SM-6 fix).
+  url = path_template.format(id=project_id)
+  response = client.patch(url, json={'name': 'Renamed'})
+  assert response.status_code == 200
+
+  scenes = _scenes_docs(fake_db, project_id)
+  assert len(scenes) == 3
+  assert [scenes[f'{i:06d}']['d'] for i in range(3)] == ['0', '1', '2']
+
+  body = client.get(f'/api/projects/{project_id}').get_json()
+  assert body['name'] == 'Renamed'
+  assert [s['d'] for s in body['storyboard']] == ['0', '1', '2']
+
+
+@pytest.mark.parametrize(
+    'path_template',
+    [
+        '/api/projects/{id}',
+        '/api/projects/{id}/editor',
+        '/api/projects/{id}?view=editor',
+    ],
+)
+def test_project_patch_empty_payload_leaves_scenes_intact(
+    monkeypatch, orchestrator_module, path_template
+):
+  del orchestrator_module
+  orch, fake_db, _ = _load_app(monkeypatch)
+  client = orch.app.test_client()
+  project_id = f'empty-patch-{uuid.uuid4().hex[:8]}'
+
+  assert (
+      client.post(
+          '/api/projects',
+          json={
+              'id': project_id,
+              'name': 'Original',
+              'storyboard': [{'d': '0'}, {'d': '1'}, {'d': '2'}],
+          },
+      ).status_code
+      == 200
+  )
+  assert len(_scenes_docs(fake_db, project_id)) == 3
+
+  url = path_template.format(id=project_id)
+  response = client.patch(url, json={})
+  assert response.status_code == 200
+
+  scenes = _scenes_docs(fake_db, project_id)
+  assert len(scenes) == 3
+  body = client.get(f'/api/projects/{project_id}').get_json()
+  assert [s['d'] for s in body['storyboard']] == ['0', '1', '2']
+
+
+@pytest.mark.parametrize(
+    'path_template',
+    [
+        '/api/projects/{id}',
+        '/api/projects/{id}/editor',
+        '/api/projects/{id}?view=editor',
+    ],
+)
+def test_project_patch_explicit_empty_storyboard_deletes_all_scenes(
+    monkeypatch, orchestrator_module, path_template
+):
+  del orchestrator_module
+  orch, fake_db, _ = _load_app(monkeypatch)
+  client = orch.app.test_client()
+  project_id = f'clear-scenes-{uuid.uuid4().hex[:8]}'
+
+  assert (
+      client.post(
+          '/api/projects',
+          json={
+              'id': project_id,
+              'name': 'Original',
+              'storyboard': [{'d': '0'}, {'d': '1'}, {'d': '2'}],
+          },
+      ).status_code
+      == 200
+  )
+  assert len(_scenes_docs(fake_db, project_id)) == 3
+
+  url = path_template.format(id=project_id)
+  response = client.patch(url, json={'storyboard': []})
+  assert response.status_code == 200
+
+  assert len(_scenes_docs(fake_db, project_id)) == 0
+  body = client.get(f'/api/projects/{project_id}').get_json()
+  assert body['storyboard'] == []
+
+
+@pytest.mark.parametrize(
+    'path_template',
+    [
+        '/api/projects/{id}',
+        '/api/projects/{id}/editor',
+        '/api/projects/{id}?view=editor',
+    ],
+)
+def test_project_patch_shrinking_storyboard_deletes_trailing_scenes(
+    monkeypatch, orchestrator_module, path_template
+):
+  del orchestrator_module
+  orch, fake_db, _ = _load_app(monkeypatch)
+  client = orch.app.test_client()
+  project_id = f'shrink-5-to-3-{uuid.uuid4().hex[:8]}'
+
+  assert (
+      client.post(
+          '/api/projects',
+          json={
+              'id': project_id,
+              'name': 'Original',
+              'storyboard': [{'d': str(i)} for i in range(5)],
+          },
+      ).status_code
+      == 200
+  )
+  assert len(_scenes_docs(fake_db, project_id)) == 5
+
+  url = path_template.format(id=project_id)
+  response = client.patch(
+      url,
+      json={
+          'storyboard': [{'d': f'{i}-updated'} for i in range(3)],
+      },
+  )
+  assert response.status_code == 200
+
+  scenes = _scenes_docs(fake_db, project_id)
+  assert len(scenes) == 3
+  assert set(scenes.keys()) == {'000000', '000001', '000002'}
+  assert [scenes[f'{i:06d}']['d'] for i in range(3)] == [
+      '0-updated',
+      '1-updated',
+      '2-updated',
+  ]
+  assert '000003' not in scenes
+  assert '000004' not in scenes
+
+  body = client.get(f'/api/projects/{project_id}').get_json()
+  assert [s['d'] for s in body['storyboard']] == [
+      '0-updated',
+      '1-updated',
+      '2-updated',
+  ]
+
 
 def test_project_delete_clears_scenes_subcollection(
     monkeypatch, orchestrator_module

From 0b689829edf5bf5c65e6e9e728a5109c24dfd219 Mon Sep 17 00:00:00 2001
From: Christopher Voelpel
 <123032885+christophervoelpel@users.noreply.github.com>
Date: Mon, 21 Sep 2026 22:50:01 +0200
Subject: [PATCH 3/7] Clarify omitted root fields in project PATCH contract

---
 DEVELOPING.md               | 4 +++-
 orch.py                     | 4 +++-
 test/test_frontdoor_data.py | 9 +++++++++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/DEVELOPING.md b/DEVELOPING.md
index 147c24f..23075e2 100644
--- a/DEVELOPING.md
+++ b/DEVELOPING.md
@@ -251,7 +251,9 @@ Editor saves without inputs use `PATCH /api/projects/:id/editor`. Its root
 field updates leave stored `inputConfig` untouched, including a concurrent
 Setup update. Do not copy an earlier Setup snapshot into a replacement write.
 Full GET/PATCH remains the Setup and legacy detail contract; full PATCH keeps
-its replacement semantics. All editor candidates remain available for counts,
+its replacement semantics, except that an omitted `storyboard` leaves stored
+scenes unchanged. Other omitted root fields, including `inputConfig`, are removed.
+All editor candidates remain available for counts,
 selection, generation and composition.
 
 The dedicated editor PATCH path is also a rollback safety boundary: a
diff --git a/orch.py b/orch.py
index 504b0e5..ffeb6d0 100644
--- a/orch.py
+++ b/orch.py
@@ -1399,7 +1399,9 @@ def project_detail_handler(
 
   The default PATCH is the faithful port of the UI's whole-document autosave:
   a full set() with createdBy stripped from the payload (immutable; the stored
-  owner is preserved) and lastEdited refreshed server-side. PATCH
+  owner is preserved) and lastEdited refreshed server-side. An omitted
+  storyboard preserves scenes; other omitted mutable root fields are removed.
+  PATCH
   ?view=editor is the legacy form of the explicit editor exception. The
   dedicated PATCH /api/projects/<id>/editor route is the preferred form; both
   use field updates so the omitted Setup inputConfig cannot be overwritten by
diff --git a/test/test_frontdoor_data.py b/test/test_frontdoor_data.py
index db71f0c..0fa397f 100644
--- a/test/test_frontdoor_data.py
+++ b/test/test_frontdoor_data.py
@@ -1686,6 +1686,7 @@ def test_project_patch_omitting_storyboard_leaves_scenes_intact(
           json={
               'id': project_id,
               'name': 'Original',
+              'inputConfig': {'productDescription': 'Keep for editor saves'},
               'storyboard': [{'d': '0'}, {'d': '1'}, {'d': '2'}],
           },
       ).status_code
@@ -1706,6 +1707,14 @@ def test_project_patch_omitting_storyboard_leaves_scenes_intact(
   assert body['name'] == 'Renamed'
   assert [s['d'] for s in body['storyboard']] == ['0', '1', '2']
 
+  if path_template == '/api/projects/{id}':
+    # Full PATCH replaces root fields; only an omitted storyboard is preserved.
+    assert 'inputConfig' not in body
+  else:
+    assert body['inputConfig'] == {
+        'productDescription': 'Keep for editor saves'
+    }
+
 
 @pytest.mark.parametrize(
     'path_template',

From 01861539a3577f8e1be29af9cc5620c3c5acb757 Mon Sep 17 00:00:00 2001
From: Christopher Voelpel
 <123032885+christophervoelpel@users.noreply.github.com>
Date: Mon, 21 Sep 2026 22:52:41 +0200
Subject: [PATCH 4/7] Use refreshed service account identity for signed URLs

---
 test/test_gcs_wrapper.py | 51 +++++++++++++++++++++++++++++++++++++---
 util/gcs_wrapper.py      |  1 +
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/test/test_gcs_wrapper.py b/test/test_gcs_wrapper.py
index 078bf43..7785983 100644
--- a/test/test_gcs_wrapper.py
+++ b/test/test_gcs_wrapper.py
@@ -114,7 +114,10 @@ def test_get_signed_url_flask_context_caches_client_and_credentials_until_expire
 
   mock_cred = mock.Mock()
   mock_cred.service_account_email = 'sa@example.com'
-  future_expiry = datetime.datetime.now() + datetime.timedelta(hours=1)
+  future_expiry = (
+      datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None)
+      + datetime.timedelta(hours=1)
+  )
   mock_cred.expiry = future_expiry
   mock_default = mock.Mock(return_value=(mock_cred, 'project-id'))
 
@@ -141,7 +144,8 @@ def test_get_signed_url_flask_context_caches_client_and_credentials_until_expire
 
     # Now expire credentials via real expiry; refresh must be called a second time
     gcs_wrapper._CACHED_SIGNING_CREDENTIALS.expiry = (
-        datetime.datetime.now() - datetime.timedelta(minutes=5)
+        datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None)
+        - datetime.timedelta(minutes=5)
     )
     url3 = gcs_wrapper.get_signed_url('bucket', 'c.mp4', flask_context=True)
     assert url3 == 'https://storage.googleapis.com/signed'
@@ -149,11 +153,52 @@ def test_get_signed_url_flask_context_caches_client_and_credentials_until_expire
     assert mock_cred.refresh.call_count == 2
 
 
+def test_signing_context_uses_refreshed_service_account_email():
+  """Signer identities must use the email resolved by credential refresh."""
+  gcs_wrapper._CACHED_STORAGE_CLIENT = None
+  gcs_wrapper._CACHED_CLIENT_FACTORY = None
+  gcs_wrapper._CACHED_SIGNING_CREDENTIALS = None
+  gcs_wrapper._CACHED_AUTH_FACTORY = None
+
+  mock_client = mock.Mock()
+  mock_cred = mock.Mock()
+  mock_cred.service_account_email = 'default'
+  mock_cred.expiry = (
+      datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None)
+      + datetime.timedelta(hours=1)
+  )
+
+  def resolve_service_account(_):
+    mock_cred.service_account_email = 'resolved@example.com'
+
+  mock_cred.refresh.side_effect = resolve_service_account
+  mock_default = mock.Mock(return_value=(mock_cred, 'project-id'))
+
+  with mock.patch(
+      'util.gcs_wrapper.storage.Client', return_value=mock_client
+  ), mock.patch(
+      'util.gcs_wrapper.default', mock_default
+  ), mock.patch(
+      'util.gcs_wrapper.iam.Signer'
+  ) as signer_factory, mock.patch(
+      'util.gcs_wrapper.compute_engine.IDTokenCredentials'
+  ) as id_token_factory:
+    gcs_wrapper.get_signing_context()
+
+  assert signer_factory.call_args.args[2] == 'resolved@example.com'
+  assert id_token_factory.call_args.kwargs[
+      'service_account_email'
+  ] == 'resolved@example.com'
+
+
 def test_signing_context_built_once_across_concurrent_callers():
   mock_client = mock.Mock()
   mock_cred = mock.Mock()
   mock_cred.service_account_email = 'sa@example.com'
-  mock_cred.expiry = datetime.datetime.now() + datetime.timedelta(hours=1)
+  mock_cred.expiry = (
+      datetime.datetime.now(datetime.timezone.utc).replace(tzinfo=None)
+      + datetime.timedelta(hours=1)
+  )
   mock_default = mock.Mock(return_value=(mock_cred, 'project-id'))
 
   with mock.patch(
diff --git a/util/gcs_wrapper.py b/util/gcs_wrapper.py
index 4300fcd..ca0a42a 100644
--- a/util/gcs_wrapper.py
+++ b/util/gcs_wrapper.py
@@ -76,6 +76,7 @@ def get_signing_context():
             'See DEVELOPING.md ("The local loop") for details.'
         )
       cred.refresh(auth_request)  # pyright: ignore[reportAttributeAccessIssue]
+      sa_email = cred.service_account_email
       signer = iam.Signer(
           auth_request,
           cred,

From b240c852f5059ce97bcea51f945fa9001ec5e015 Mon Sep 17 00:00:00 2001
From: christophervoelpel
 <123032885+christophervoelpel@users.noreply.github.com>
Date: Tue, 22 Sep 2026 07:46:19 +0000
Subject: [PATCH 5/7] Add return type annotation, docstring, and DEVELOPING.md
 signing note

---
 DEVELOPING.md       |  2 +-
 util/gcs_wrapper.py | 27 ++++++++++++++++++---------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/DEVELOPING.md b/DEVELOPING.md
index 147c24f..7313891 100644
--- a/DEVELOPING.md
+++ b/DEVELOPING.md
@@ -112,7 +112,7 @@ links; raw HTML, images, and other block content are not displayed.
 
 ### The local loop (two terminals)
 
-The local backend talks to a real dev GCP project through your Application Default Credentials, so run `gcloud auth application-default login` once first.
+The local backend talks to a real dev GCP project through your Application Default Credentials. Because GCS URL signing (`/api/signUrl`, `/api/uploadUrl`) requires a service account identity, run `gcloud auth application-default login --impersonate-service-account=<SA_EMAIL>` (or set `GOOGLE_APPLICATION_CREDENTIALS` to a service account key file) once first.
 
 **Terminal 1 - local backend (the `/api` server):**
 
diff --git a/util/gcs_wrapper.py b/util/gcs_wrapper.py
index ca0a42a..7408f63 100644
--- a/util/gcs_wrapper.py
+++ b/util/gcs_wrapper.py
@@ -48,8 +48,16 @@
 _SIGNING_CONTEXT_LOCK = threading.Lock()
 
 
-def get_signing_context():
-  """Returns cached (storage_client, signing_credentials) for GCS URL signing."""
+def get_signing_context(
+) -> tuple[storage.Client, compute_engine.IDTokenCredentials]:
+  """Returns cached (storage_client, signing_credentials) for GCS URL signing.
+
+  Returns:
+    A tuple of (storage.Client, compute_engine.IDTokenCredentials).
+
+  Raises:
+    RuntimeError: If active credentials do not have a service account email.
+  """
   global _CACHED_STORAGE_CLIENT, _CACHED_CLIENT_FACTORY
   global _CACHED_SIGNING_CREDENTIALS, _CACHED_AUTH_FACTORY
 
@@ -67,13 +75,14 @@ def get_signing_context():
       sa_email = getattr(cred, 'service_account_email', None)
       if not sa_email:
         raise RuntimeError(
-            'GCS URL signing requires a service account identity, but the active '
-            'credentials have no service_account_email. User credentials from '
-            '"gcloud auth application-default login" cannot sign URLs; configure '
-            'service account impersonation ("gcloud auth application-default login '
-            '--impersonate-service-account=<SA_EMAIL>") or set '
-            'GOOGLE_APPLICATION_CREDENTIALS to a service account key file. '
-            'See DEVELOPING.md ("The local loop") for details.'
+            'GCS URL signing requires a service account identity, but the '
+            'active credentials have no service_account_email. User '
+            'credentials from "gcloud auth application-default login" cannot '
+            'sign URLs; configure service account impersonation ("gcloud auth '
+            'application-default login --impersonate-service-account='
+            '<SA_EMAIL>") or set GOOGLE_APPLICATION_CREDENTIALS to a service '
+            'account key file. See DEVELOPING.md ("The local loop") for '
+            'details.'
         )
       cred.refresh(auth_request)  # pyright: ignore[reportAttributeAccessIssue]
       sa_email = cred.service_account_email

From 63e67fa3acd86675f06dd954c677306b8f640b6f Mon Sep 17 00:00:00 2001
From: christophervoelpel
 <123032885+christophervoelpel@users.noreply.github.com>
Date: Tue, 22 Sep 2026 10:31:30 +0000
Subject: [PATCH 6/7] Reject unresolved default service account email after
 refresh

---
 test/test_gcs_wrapper.py | 16 ++++++++++++++++
 util/gcs_wrapper.py      |  7 ++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/test/test_gcs_wrapper.py b/test/test_gcs_wrapper.py
index 7785983..b6766b4 100644
--- a/test/test_gcs_wrapper.py
+++ b/test/test_gcs_wrapper.py
@@ -260,3 +260,19 @@ def test_non_service_account_adc_raises_actionable_error():
     assert 'DEVELOPING.md' in err_msg
     assert client_factory.call_count == 1
     assert mock_default.call_count == 1
+
+
+def test_unresolved_default_service_account_email_after_refresh_raises():
+  mock_client = mock.Mock()
+  mock_cred = mock.Mock()
+  mock_cred.service_account_email = 'default'
+  mock_default = mock.Mock(return_value=(mock_cred, 'project-id'))
+
+  with mock.patch(
+      'util.gcs_wrapper.storage.Client', return_value=mock_client
+  ), mock.patch('util.gcs_wrapper.default', mock_default):
+    with pytest.raises(
+        RuntimeError,
+        match="credential refresh produced 'default'",
+    ):
+      gcs_wrapper.get_signing_context()
diff --git a/util/gcs_wrapper.py b/util/gcs_wrapper.py
index 7408f63..f85abf6 100644
--- a/util/gcs_wrapper.py
+++ b/util/gcs_wrapper.py
@@ -85,7 +85,12 @@ def get_signing_context(
             'details.'
         )
       cred.refresh(auth_request)  # pyright: ignore[reportAttributeAccessIssue]
-      sa_email = cred.service_account_email
+      sa_email = getattr(cred, 'service_account_email', None)
+      if not sa_email or sa_email == 'default':
+        raise RuntimeError(
+            'GCS URL signing requires a resolved service account email, but '
+            f'credential refresh produced {sa_email!r}.'
+        )
       signer = iam.Signer(
           auth_request,
           cred,

From 80d0221e173db2a62d490a8d1aad602d6db74350 Mon Sep 17 00:00:00 2001
From: christophervoelpel
 <123032885+christophervoelpel@users.noreply.github.com>
Date: Thu, 24 Sep 2026 09:11:58 +0000
Subject: [PATCH 7/7] Only write or prune scenes for an explicit storyboard
 list

A PATCH carrying "storyboard": null (or any non-list value) was treated
like [] and deleted every scene. Both the full and editor write paths now
touch the scenes subcollection only when storyboard is a list, and the
editor path always pins the root storyboard placeholder to [] so a
non-list value can never be stored on the root document.

Tests assert the stored root placeholder directly (GET rebuilds storyboard
from the subcollection and would mask a regression) and cover null,
string and object storyboard values on all three PATCH routes.

Addresses review feedback from victor-paunescu on #203.
---
 orch.py                     | 24 ++++++++++----------
 test/test_frontdoor_data.py | 44 +++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/orch.py b/orch.py
index ffeb6d0..0522100 100644
--- a/orch.py
+++ b/orch.py
@@ -1175,16 +1175,15 @@ def _write_project_doc(
   precondition, scene writes retain their existing set behavior, and the
   stale-scene prune scan is skipped: a brand-new project has no prior scenes
   to prune, and callers cap create at _MAX_CREATE_SCENES so this always fits
-  one atomic batch. When create is false, an omitted storyboard key leaves the
-  scenes subcollection untouched.
+  one atomic batch. When create is false, an omitted or non-list storyboard
+  (e.g. null) leaves the scenes subcollection untouched; only an explicit list
+  (including []) writes and prunes scenes.
   """
   root = dict(payload)
   root['storyboard'] = []
   ops = [('create' if create else 'set', doc_ref, root)]
-  if create or 'storyboard' in payload:
-    scenes = payload.get('storyboard')
-    if not isinstance(scenes, list):
-      scenes = []
+  scenes = payload.get('storyboard')
+  if isinstance(scenes, list):
     scenes_ref = doc_ref.collection(_SCENES_SUBCOLLECTION)
     for index, scene in enumerate(scenes):
       ops.append(('set', scenes_ref.document(_scene_doc_id(index)), scene))
@@ -1207,8 +1206,9 @@ def _write_editor_project_doc(
   The root update is deliberately field-based: unlike a read-modify-write
   replacement, it cannot copy a stale inputConfig snapshot over a concurrent
   Setup save. Fields omitted by the editor payload retain the legacy full-save
-  replacement behavior through DELETE_FIELD updates, except storyboard which
-  leaves the scenes subcollection untouched when omitted.
+  replacement behavior through DELETE_FIELD updates, except storyboard: the
+  root always keeps the [] placeholder, and an omitted or non-list storyboard
+  (e.g. null) leaves the scenes subcollection untouched.
   """
   root_updates = {
       field_path.FieldPath(key).to_api_repr(): firestore.DELETE_FIELD
@@ -1219,12 +1219,10 @@ def _write_editor_project_doc(
       field_path.FieldPath(key).to_api_repr(): value
       for key, value in payload.items()
   })
+  root_updates[field_path.FieldPath('storyboard').to_api_repr()] = []
   ops = []
-  if 'storyboard' in payload:
-    scenes = payload.get('storyboard')
-    if not isinstance(scenes, list):
-      scenes = []
-    root_updates[field_path.FieldPath('storyboard').to_api_repr()] = []
+  scenes = payload.get('storyboard')
+  if isinstance(scenes, list):
     scenes_ref = doc_ref.collection(_SCENES_SUBCOLLECTION)
     for index, scene in enumerate(scenes):
       ops.append(('set', scenes_ref.document(_scene_doc_id(index)), scene))
diff --git a/test/test_frontdoor_data.py b/test/test_frontdoor_data.py
index 0fa397f..853604e 100644
--- a/test/test_frontdoor_data.py
+++ b/test/test_frontdoor_data.py
@@ -1699,6 +1699,9 @@ def test_project_patch_omitting_storyboard_leaves_scenes_intact(
   response = client.patch(url, json={'name': 'Renamed'})
   assert response.status_code == 200
 
+  # The GET below rebuilds storyboard from the subcollection, so check the
+  # stored root placeholder directly.
+  assert fake_db.collection('projects').docs[project_id]['storyboard'] == []
   scenes = _scenes_docs(fake_db, project_id)
   assert len(scenes) == 3
   assert [scenes[f'{i:06d}']['d'] for i in range(3)] == ['0', '1', '2']
@@ -1755,6 +1758,45 @@ def test_project_patch_empty_payload_leaves_scenes_intact(
   assert [s['d'] for s in body['storyboard']] == ['0', '1', '2']
 
 
+@pytest.mark.parametrize('storyboard', [None, 'not-a-list', {'d': '0'}])
+@pytest.mark.parametrize(
+    'path_template',
+    [
+        '/api/projects/{id}',
+        '/api/projects/{id}/editor',
+        '/api/projects/{id}?view=editor',
+    ],
+)
+def test_project_patch_non_list_storyboard_leaves_scenes_intact(
+    monkeypatch, orchestrator_module, path_template, storyboard
+):
+  del orchestrator_module
+  orch, fake_db, _ = _load_app(monkeypatch)
+  client = orch.app.test_client()
+  project_id = f'null-storyboard-{uuid.uuid4().hex[:8]}'
+
+  assert (
+      client.post(
+          '/api/projects',
+          json={
+              'id': project_id,
+              'name': 'Original',
+              'storyboard': [{'d': '0'}, {'d': '1'}, {'d': '2'}],
+          },
+      ).status_code
+      == 200
+  )
+
+  # Clients that serialize unset optional fields as null must not wipe scenes.
+  url = path_template.format(id=project_id)
+  response = client.patch(url, json={'name': 'n', 'storyboard': storyboard})
+  assert response.status_code == 200
+
+  assert fake_db.collection('projects').docs[project_id]['storyboard'] == []
+  scenes = _scenes_docs(fake_db, project_id)
+  assert [scenes[f'{i:06d}']['d'] for i in range(3)] == ['0', '1', '2']
+
+
 @pytest.mark.parametrize(
     'path_template',
     [
@@ -1831,6 +1873,8 @@ def test_project_patch_shrinking_storyboard_deletes_trailing_scenes(
   )
   assert response.status_code == 200
 
+  # Scenes live only in the subcollection, never inline on the root doc.
+  assert fake_db.collection('projects').docs[project_id]['storyboard'] == []
   scenes = _scenes_docs(fake_db, project_id)
   assert len(scenes) == 3
   assert set(scenes.keys()) == {'000000', '000001', '000002'}