Skip to content

Commit 59f2f01

Browse files
nficanocursoragent
andcommitted
fix: distinct lifecycle error codes and stricter required_features (#105 #111 #113)
- handleCancel/handleInterrupt return NOT_FOUND for an unknown job and reserve FAILED_PRECONDITION for an existing-but-terminal job. - handleResume surfaces a buffer miss as a correlated nack (DATA_LOSS) instead of an out-of-context tool.error. - HandshakeNegotiator rejects a non-string entry in required_features rather than silently skipping it. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 1e2424c commit 59f2f01

4 files changed

Lines changed: 45 additions & 16 deletions

File tree

src/Internal/Runtime/HandshakeNegotiator.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,10 @@ private function checkCapabilities(Capabilities $requested): ?string
202202
if (\is_array($required)) {
203203
$advertisedFeatures = $this->runtime->advertisedCapabilitiesForSession()->features;
204204
foreach ($required as $feature) {
205-
if (\is_string($feature) && !\in_array($feature, $advertisedFeatures, true)) {
205+
if (!\is_string($feature)) {
206+
return 'required_features entry must be string';
207+
}
208+
if (!\in_array($feature, $advertisedFeatures, true)) {
206209
return 'feature unsupported: ' . $feature;
207210
}
208211
}

src/Internal/Runtime/LifecycleHandler.php

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
use Arcp\Messages\Control\Ping;
2424
use Arcp\Messages\Control\Pong;
2525
use Arcp\Messages\Control\Resume;
26-
use Arcp\Messages\Execution\ToolError;
2726
use Arcp\Messages\Human\HumanInputRequest;
2827
use Arcp\Messages\Human\HumanInputResponse;
2928
use Arcp\Messages\Permissions\LeaseExtended;
@@ -79,7 +78,11 @@ public function handleCancel(Session $session, Envelope $env, Cancel $msg): void
7978
return;
8079
}
8180
$job = $this->runtime->jobs->tryGet(new JobId($msg->targetId));
82-
if (!$job instanceof Job || $job->state->isTerminal()) {
81+
if (!$job instanceof Job) {
82+
$this->nack($session, $env, 'NOT_FOUND', 'job not found');
83+
return;
84+
}
85+
if ($job->state->isTerminal()) {
8386
$this->nack($session, $env, 'FAILED_PRECONDITION', 'job already terminal');
8487
return;
8588
}
@@ -95,8 +98,12 @@ public function handleCancel(Session $session, Envelope $env, Cancel $msg): void
9598
public function handleInterrupt(Session $session, Envelope $env, Interrupt $msg): void
9699
{
97100
$job = $this->runtime->jobs->tryGet(new JobId($msg->targetId));
98-
if (!$job instanceof Job || $job->state->isTerminal()) {
99-
$this->nack($session, $env, 'FAILED_PRECONDITION', 'job not interruptible');
101+
if (!$job instanceof Job) {
102+
$this->nack($session, $env, 'NOT_FOUND', 'job not found');
103+
return;
104+
}
105+
if ($job->state->isTerminal()) {
106+
$this->nack($session, $env, 'FAILED_PRECONDITION', 'job already terminal');
100107
return;
101108
}
102109
// Only the owning session may interrupt a job (cf. handleCancel).
@@ -159,11 +166,9 @@ public function handleResume(Session $session, Envelope $env, Resume $msg): void
159166
$session->transport->send($past);
160167
}
161168
} catch (InvalidArgumentException) {
162-
$this->runtime->emit($session, new ToolError(
163-
new ErrorPayload('DATA_LOSS', 'after_message_id retention expired'),
164-
), [
165-
'correlation_id' => $env->id,
166-
]);
169+
// Resume is a lifecycle operation, not a tool invocation; surface
170+
// the failure as a correlated nack like every other lifecycle path.
171+
$this->nack($session, $env, 'DATA_LOSS', 'after_message_id retention expired');
167172
return;
168173
}
169174
$this->runtime->emit($session, new Ack('resumed'), ['correlation_id' => $env->id]);

tests/Unit/Internal/Runtime/HandshakeNegotiatorTest.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,28 @@ public function invoke(array $arguments, JobContext $ctx, ?Cancellation $cancell
151151
$futureB->await();
152152
}
153153

154+
public function testNonStringRequiredFeatureIsRejected(): void
155+
{
156+
$runtime = new ARCPRuntime();
157+
[$serverT, $clientT] = MemoryTransport::pair();
158+
$serverFuture = $runtime->serveAsync($serverT);
159+
160+
$client = new ARCPClient($clientT);
161+
try {
162+
$client->open(
163+
Auth::none(),
164+
new PeerInfo('cli', '0.1'),
165+
new Capabilities(anonymous: true, extra: ['required_features' => [123]]),
166+
);
167+
self::fail('expected UnimplementedException for non-string required feature');
168+
} catch (UnimplementedException $e) {
169+
self::assertStringContainsString('required_features entry must be string', $e->getMessage());
170+
} finally {
171+
$client->close();
172+
$serverFuture->await();
173+
}
174+
}
175+
154176
public function testMtlsAuthRouterReturnsUnimplemented(): void
155177
{
156178
// AuthRouter does not register mtls scheme; mtls is reserved -> UnimplementedException.

tests/Unit/Internal/Runtime/LifecycleHandlerTest.php

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
use Arcp\Messages\Control\Interrupt;
2424
use Arcp\Messages\Control\Nack;
2525
use Arcp\Messages\Control\Resume;
26-
use Arcp\Messages\Execution\ToolError;
2726
use Arcp\Messages\Execution\ToolInvoke;
2827
use Arcp\Messages\Human\HumanChoiceRequest;
2928
use Arcp\Messages\Human\HumanChoiceResponse;
@@ -73,7 +72,7 @@ public function testCancelNonJobTargetIsNackedAsUnimplemented(): void
7372
$serverFuture->await();
7473
}
7574

76-
public function testCancelUnknownJobReturnsFailedPrecondition(): void
75+
public function testCancelUnknownJobReturnsNotFound(): void
7776
{
7877
[, $client, $serverFuture] = $this->pair();
7978
$msgId = MessageId::random();
@@ -86,7 +85,7 @@ public function testCancelUnknownJobReturnsFailedPrecondition(): void
8685
$client->session->transport->send($env);
8786
$response = $client->pending->awaitResponse($msgId, 5.0);
8887
self::assertInstanceOf(Nack::class, $response);
89-
self::assertSame('FAILED_PRECONDITION', $response->error->code);
88+
self::assertSame('NOT_FOUND', $response->error->code);
9089

9190
$client->close();
9291
$serverFuture->await();
@@ -157,7 +156,7 @@ public function testInterruptOnUnknownJobIsNacked(): void
157156
$client->session->transport->send($env);
158157
$response = $client->pending->awaitResponse($msgId, 5.0);
159158
self::assertInstanceOf(Nack::class, $response);
160-
self::assertSame('FAILED_PRECONDITION', $response->error->code);
159+
self::assertSame('NOT_FOUND', $response->error->code);
161160

162161
$client->close();
163162
$serverFuture->await();
@@ -260,7 +259,7 @@ public function testResumeWithCheckpointIsNackedAsUnimplemented(): void
260259
$serverFuture->await();
261260
}
262261

263-
public function testResumeWithUnknownAfterMessageIdYieldsDataLossToolError(): void
262+
public function testResumeWithUnknownAfterMessageIdYieldsDataLossNack(): void
264263
{
265264
[, $client, $serverFuture] = $this->pair();
266265

@@ -273,7 +272,7 @@ public function testResumeWithUnknownAfterMessageIdYieldsDataLossToolError(): vo
273272
);
274273
$client->session->transport->send($env);
275274
$response = $client->pending->awaitResponse($msgId, 5.0);
276-
self::assertInstanceOf(ToolError::class, $response);
275+
self::assertInstanceOf(Nack::class, $response);
277276
self::assertSame('DATA_LOSS', $response->error->code);
278277

279278
$client->close();

0 commit comments

Comments
 (0)