Skip to content

Commit 5868c64

Browse files
Nick Ficanocursoragent
andcommitted
fix(runtime): agent_versions gating, chunk/progress/metric validation, revocation surfacing
- reject name@version without agent_versions; strip suffix from non-negotiated sessions (#79) - enforce chunk_seq monotonicity and single-stream completion (#84) - progress current>=0 (reject) and current<=total (clamp) (#85) - negative cost metrics rejected; negative non-cost metrics still emit (#86) - permanent credential revocation failures logged + exposed via RevocationFailures (#87) Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 055659b commit 5868c64

5 files changed

Lines changed: 117 additions & 7 deletions

File tree

src/Arcp.Runtime/Internal/CredentialRegistry.fs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ type internal CredentialRegistry(provisioner: ICredentialProvisioner, store: ICr
1313
let perJob = ConcurrentDictionary<string, ConcurrentDictionary<string, unit>>()
1414
let retryDelays = [ 200; 1000; 5000 ]
1515

16+
// §9.8.2/§14: credentials whose revocation permanently failed, kept
17+
// for operator inspection (`RevocationFailures`).
18+
let revocationFailures = ConcurrentDictionary<string, string>()
19+
1620
let remember (jobId: JobId) (credentialId: string) =
1721
let ids =
1822
perJob.GetOrAdd(jobId.Value, fun _ -> ConcurrentDictionary<string, unit>())
@@ -43,6 +47,8 @@ type internal CredentialRegistry(provisioner: ICredentialProvisioner, store: ICr
4347
attempt <- attempt + 1
4448

4549
if revoked then
50+
revocationFailures.TryRemove credentialId |> ignore
51+
4652
match jobIdOpt with
4753
| Some jobId ->
4854
do! store.RecordRevokedAsync(jobId, credentialId)
@@ -54,8 +60,26 @@ type internal CredentialRegistry(provisioner: ICredentialProvisioner, store: ICr
5460
if id = credentialId then
5561
do! store.RecordRevokedAsync(jobId, credentialId)
5662
forget jobId credentialId
63+
else
64+
// §9.8.2: permanent revocation failures MUST be logged; §14:
65+
// surfaced to operators. Record for `RevocationFailures`.
66+
let jobLabel =
67+
jobIdOpt |> Option.map (fun j -> j.Value) |> Option.defaultValue "<unknown>"
68+
69+
revocationFailures.[credentialId] <- jobLabel
70+
71+
eprintfn
72+
"[ARCP] WARN credential revocation failed permanently: credential_id=%s job_id=%s"
73+
credentialId
74+
jobLabel
5775
}
5876

77+
/// Credentials whose revocation permanently failed, as
78+
/// `(credentialId, jobId)` pairs. Operators can poll this to find
79+
/// dangling credentials (§9.8.2, §14).
80+
member _.RevocationFailures: (string * string) list =
81+
revocationFailures |> Seq.map (fun kv -> kv.Key, kv.Value) |> List.ofSeq
82+
5983
member _.Track(jobId: JobId, cred: Credential) : Task =
6084
task {
6185
remember jobId cred.Id

src/Arcp.Runtime/Internal/JobSubmitFlow.fs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,10 +225,30 @@ module internal JobSubmitFlow =
225225
do! sendAccepted ctx.Transport ctx.SessionId requestId (JobId.ofString existing) accepted ct
226226
| Error err -> do! EnvelopeOut.respondWithError ctx requestId err ct
227227
| _ ->
228+
let agentVersionsOn = ctx.NegotiatedFeatures.Contains Features.AgentVersions
229+
230+
if not agentVersionsOn && submit.Agent.Contains '@' then
231+
// §6.2/§7.5: cannot pin a version without negotiating
232+
// agent_versions.
233+
do!
234+
EnvelopeOut.respondWithError
235+
ctx
236+
requestId
237+
(ARCPError.InvalidRequest("agent_versions not negotiated; bare agent name required", None))
238+
ct
239+
else
240+
228241
match inventory.Resolve submit.Agent with
229242
| Error err -> do! EnvelopeOut.respondWithError ctx requestId err ct
230243
| Ok(name, version, _) ->
231-
let resolvedAgent = AgentRef.format name (Some version)
244+
// The handler is always keyed name@version; the agent
245+
// surfaced on the wire omits the version when the feature
246+
// wasn't negotiated (§6.2).
247+
let handlerKey = AgentRef.format name (Some version)
248+
249+
let resolvedAgent =
250+
if agentVersionsOn then handlerKey else name
251+
232252
let lease = submit.LeaseRequest |> Option.defaultValue Lease.empty
233253

234254
match validateConstraints timeProvider submit.LeaseConstraints with
@@ -344,7 +364,7 @@ module internal JobSubmitFlow =
344364

345365
do! sendAccepted ctx.Transport ctx.SessionId requestId jobId accepted ct
346366

347-
match agentHandlers.TryGetValue resolvedAgent with
367+
match agentHandlers.TryGetValue handlerKey with
348368
| true, handler ->
349369
JobLauncher.launch jobs credentialRegistry timeProvider record handler
350370
| _ ->

src/Arcp.Runtime/JobContext.fs

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ type JobContext
3232
streamResultBegin: unit -> ResultId,
3333
onCostMetric: string * decimal -> unit
3434
) =
35+
// Per-result_id chunk ordering state (§8.4): next expected chunk_seq
36+
// and whether the stream was closed by a `more=false` chunk.
37+
let chunkNext = System.Collections.Generic.Dictionary<string, int64>()
38+
let chunkClosed = System.Collections.Generic.HashSet<string>()
39+
let chunkLock = obj ()
40+
3541
member _.JobId: JobId = jobId
3642
member _.SessionId: SessionId = sessionId
3743
member _.ParentJobId: JobId option = parentJobId
@@ -63,10 +69,21 @@ type JobContext
6369
member _.RotateCredentialAsync(credentialId: string, newValue: string, ct: CancellationToken) : Task =
6470
rotateCredential (credentialId, newValue, ct)
6571

72+
/// Emit a `progress` event (§8.2.1). `current` MUST be non-negative
73+
/// (rejected with INVALID_REQUEST otherwise); when `total` is present
74+
/// `current` is clamped to `total` so the wire invariant holds.
6675
member _.EmitProgressAsync
6776
(current: decimal, total: decimal option, units: string option, message: string option, _ct: CancellationToken)
6877
: Task =
69-
emit (JobEventBody.Progress(current, total, units, message))
78+
if current < 0m then
79+
raise (ArcpException(ARCPError.InvalidRequest("progress.current must be non-negative", None)))
80+
81+
let clamped =
82+
match total with
83+
| Some t when current > t -> t
84+
| _ -> current
85+
86+
emit (JobEventBody.Progress(clamped, total, units, message))
7087

7188
/// Emit a `metric` event. Names starting with `cost.` and a
7289
/// budgeted `unit` decrement the matching budget counter
@@ -80,7 +97,12 @@ type JobContext
8097
_ct: CancellationToken
8198
) : Task =
8299
if value < 0m then
83-
Task.CompletedTask
100+
// §9.6: negative cost metrics are rejected; other negative
101+
// metrics are not governed by §9.6 and still flow through.
102+
if name.StartsWith("cost.") then
103+
raise (ArcpException(ARCPError.InvalidRequest("cost metric value must be non-negative", None)))
104+
else
105+
emit (JobEventBody.Metric(name, value, unit, dimensions))
84106
else
85107
// §9.6: `cost.budget.*` is budget telemetry (e.g.
86108
// `cost.budget.remaining`), not a charge — it must not
@@ -135,6 +157,36 @@ type JobContext
135157
more: bool,
136158
_ct: CancellationToken
137159
) : Task =
160+
// §8.4: chunk_seq is 0-based monotonic per result_id and chunks
161+
// MUST be emitted in order; nothing may follow a `more=false`
162+
// chunk. Enforce both before anything reaches the wire.
163+
lock chunkLock (fun () ->
164+
if chunkClosed.Contains resultId.Value then
165+
raise (
166+
ArcpException(
167+
ARCPError.InternalError(sprintf "result_id %s already completed; no further chunks allowed" resultId.Value)
168+
)
169+
)
170+
171+
let expected =
172+
match chunkNext.TryGetValue resultId.Value with
173+
| true, n -> n
174+
| _ -> 0L
175+
176+
if chunkSeq <> expected then
177+
raise (
178+
ArcpException(
179+
ARCPError.InternalError(
180+
sprintf "out-of-order chunk_seq %d (expected %d) for result_id %s" chunkSeq expected resultId.Value
181+
)
182+
)
183+
)
184+
185+
chunkNext.[resultId.Value] <- expected + 1L
186+
187+
if not more then
188+
chunkClosed.Add resultId.Value |> ignore)
189+
138190
let encoded =
139191
match encoding with
140192
| ChunkEncoding.Utf8 -> Encoding.UTF8.GetString(data.Span)

tests/Arcp.IntegrationTests/IdempotencyAndCancelTests.fs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,12 @@ let ``list_jobs filter by agent narrows the set`` () =
5757
let! _ = p.Client.SubmitAsync(mkRequest "b", CancellationToken.None)
5858
let! _ = p.Client.SubmitAsync(mkRequest "b", CancellationToken.None)
5959

60+
// agent_versions not negotiated -> agents are stored bare; the
61+
// bare-name filter matches (§79, §108).
6062
let filter: JobListFilter =
6163
{
6264
Status = None
63-
Agent = Some "a@default"
65+
Agent = Some "a"
6466
CreatedAfter = None
6567
}
6668

tests/Arcp.UnitTests/JobContextTests.fs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,24 @@ let ``EmitMetric with positive cost.* decrements budget tracking`` () =
9090
costs.[0] |> should equal ("USD", 0.25m)
9191

9292
[<Fact>]
93-
let ``EmitMetric with negative value is silently dropped`` () =
93+
let ``EmitMetric negative cost value is rejected`` () =
94+
// §86: negative cost.* metrics raise INVALID_REQUEST.
9495
let ctx, emitted, _, costs, _ = mkContext Lease.empty
95-
ctx.EmitMetricAsync("anything", -1m, None, None, CancellationToken.None).Wait()
96+
97+
(fun () -> ctx.EmitMetricAsync("cost.inference", -0.01m, Some "USD", None, CancellationToken.None) |> ignore)
98+
|> should throw typeof<ArcpException>
99+
96100
emitted.Count |> should equal 0
97101
costs.Count |> should equal 0
98102

103+
[<Fact>]
104+
let ``EmitMetric negative non-cost value still emits`` () =
105+
// §86: §9.6 only governs cost metrics; negative non-cost metrics flow.
106+
let ctx, emitted, _, costs, _ = mkContext Lease.empty
107+
ctx.EmitMetricAsync("latency.ms", -5m, Some "ms", None, CancellationToken.None).Wait()
108+
emitted.Count |> should equal 1
109+
costs.Count |> should equal 0
110+
99111
[<Fact>]
100112
let ``EmitMetric non-cost name does not touch budget`` () =
101113
let ctx, emitted, _, costs, _ = mkContext Lease.empty

0 commit comments

Comments
 (0)