fix: safe credential rotation ordering and retried revocation (#90 #91 #159)

- JobCredentialControls::rotateCredential() adds the new credential before
  removing the old one, so a store add() failure can no longer leave the job
  with neither credential.
- Rotation revocation now retries a transient failure before logging a
  permanent error (§9.8.2), mirroring CredentialLifecycle.
- Both revoke paths apply a brief backoff between attempts instead of two
  identical back-to-back calls.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: nficano

src/Internal/Runtime/CredentialLifecycle.php

@@ -202,16 +202,22 @@ private function revokeCredential(
                 $provisioner->revoke($credentialId);
                 return true;
             } catch (\Throwable $e) {
-                if ($attempt === 2) {
-                    $this->runtime->logger->error(
-                        'credential revocation failed; record retained for retry',
-                        [
-                            'credential_id' => $credentialId,
-                            'job_id' => (string) $job->id,
-                            'error' => $e->getMessage(),
-                        ],
-                    );
+                if ($attempt < 2) {
+                    // Brief backoff so the retry has a chance to clear a
+                    // transient upstream failure rather than failing twice
+                    // back-to-back.
+                    \Amp\delay(0.02 * $attempt);
+
+                    continue;
                 }
+                $this->runtime->logger->error(
+                    'credential revocation failed; record retained for retry',
+                    [
+                        'credential_id' => $credentialId,
+                        'job_id' => (string) $job->id,
+                        'error' => $e->getMessage(),
+                    ],
+                );
             }
         }
         return false;

src/Runtime/JobCredentialControls.php

@@ -27,8 +27,10 @@ public function rotateCredential(Credential $new, string $previousCredentialId):
         if (!$provisioner instanceof CredentialProvisioner) {
             throw new FailedPreconditionException('no credential provisioner configured');
         }
-        $this->runtime->credentials->remove($this->jobId, $previousCredentialId);
+        // Add the replacement before removing the old record so a store
+        // add() failure cannot leave the job with neither credential.
         $this->runtime->credentials->add($this->jobId, $new);
+        $this->runtime->credentials->remove($this->jobId, $previousCredentialId);
         $this->revokePreviousCredential($provisioner, $previousCredentialId);
         $this->runtime->emit($this->session, new EventEmit('status', [
             'phase' => 'credential_rotated',
@@ -44,13 +46,23 @@ private function revokePreviousCredential(
         CredentialProvisioner $provisioner,
         string $credentialId,
     ): void {
-        try {
-            $provisioner->revoke($credentialId);
-        } catch (\Throwable $e) {
-            $this->runtime->logger->warning(
-                'credential revocation failed during rotation',
-                ['credential_id' => $credentialId, 'error' => $e->getMessage()],
-            );
+        // §9.8.2: revocation is best-effort but SHOULD retry transient
+        // failures before giving up and logging a permanent failure.
+        for ($attempt = 1; $attempt <= 2; $attempt++) {
+            try {
+                $provisioner->revoke($credentialId);
+                return;
+            } catch (\Throwable $e) {
+                if ($attempt < 2) {
+                    \Amp\delay(0.02 * $attempt);
+
+                    continue;
+                }
+                $this->runtime->logger->error(
+                    'credential revocation failed during rotation',
+                    ['credential_id' => $credentialId, 'error' => $e->getMessage()],
+                );
+            }
         }
     }
 }