fix(tasks): raise sandbox credential lock TTL to cover the full write

The 30s lease could expire mid-write: each managed write is a chain of
in-sandbox execs bounded by a 30s timeout, and the per-message gate can run two
chains back-to-back (apply the new token, then log out), so a slow sandbox can
hold the lock ~2 min. Raise the TTL to 5 min so the lease cannot expire
mid-write and let a concurrent refresh acquire and interleave. Document the
derivation and add a test covering a writer past the old 30s lease.

Author: VojtechBartos

products/tasks/backend/temporal/process_task/sandbox_credentials.py

@@ -166,9 +166,15 @@ def _actor_rebound_away_from_owner(run_id: str, state: dict | None, owner_id: in
     return bound_actor if bound_actor is not None and bound_actor != owner_id else None
 
 
-# Only the fast in-sandbox writes run under this lock (token resolution stays outside), so a short
-# TTL comfortably covers them; the wait stays well under the refresh activity's 2 min timeout.
-_CREDENTIAL_LOCK_TTL_SECONDS = 30
+# TTL derivation: the lock must outlive the whole critical section, or the lease can expire mid-write
+# and let a concurrent refresh acquire and interleave (a regression we hit before). Each managed
+# write is a chain of in-sandbox execs, each bounded by a 30s timeout — set_git_remote_token (1 exec)
+# and _write_sandbox_credential_file (a file write + a chmod exec). The per-message gate can run two
+# such chains back-to-back in one lock hold (apply the new token, then, if that fails, log out), so
+# the worst case is ~4 × 30s ≈ 2 min. A 5 min TTL clears that with margin; recompute if those exec
+# timeouts change. The wait stays under the refresh activity's 2 min timeout, so a contender that
+# can't acquire skips (fail-safe) rather than blocking the activity.
+_CREDENTIAL_LOCK_TTL_SECONDS = 5 * 60
 _CREDENTIAL_LOCK_WAIT_SECONDS = 15
 
 

products/tasks/backend/temporal/process_task/tests/test_sandbox_credentials.py

@@ -396,6 +396,37 @@ def test_fails_closed_without_applying_when_the_lock_is_contended(self):
             apply.assert_not_called()
             lock.release.assert_not_called()
 
+    def test_lock_is_leased_long_enough_to_outlive_a_writer_past_the_old_30s_lease(self):
+        # A credential write does a git-remote rewrite + env-file write + chmod, each an in-sandbox
+        # exec bounded by a 30s timeout, so it can run well past the old 30s lease. The redis lock
+        # must be leased for that whole worst case, or the lease expires mid-write and a concurrent
+        # refresh could acquire and interleave.
+        import contextlib
+
+        from products.tasks.backend.temporal.process_task.sandbox_credentials import (
+            _CREDENTIAL_LOCK_TTL_SECONDS,
+            _apply_owner_token_locked,
+        )
+
+        assert _CREDENTIAL_LOCK_TTL_SECONDS == 5 * 60
+        assert _CREDENTIAL_LOCK_TTL_SECONDS > 2 * 30  # clears a 30s git-remote + 30s chmod worst case
+
+        with contextlib.ExitStack() as stack:
+            get_client = stack.enter_context(patch(f"{MODULE}.get_client"))
+            lock = MagicMock()
+            lock.acquire.return_value = True
+            get_client.return_value.lock.return_value = lock
+            stack.enter_context(patch(f"{MODULE}.get_sandbox_github_identity_user", return_value=None))
+            stack.enter_context(patch(f"{MODULE}.apply_github_credentials_to_sandbox", return_value=True))
+            sandbox = MagicMock()
+            sandbox.id = "sb-1"
+
+            _apply_owner_token_locked(sandbox, "org/repo", "ghu_x", "run-1", {}, 7)
+
+            # The lock is leased for the full worst-case write, not the old 30s.
+            get_client.return_value.lock.assert_called_once()
+            assert get_client.return_value.lock.call_args.kwargs["timeout"] == _CREDENTIAL_LOCK_TTL_SECONDS
+
 
 class TestLoopOwnerRefreshGate:
     def _as_user_integration_run(self, stack):