fix: address review issues #22 through #30 (non-doc)

Bug fixes:
- #22 (critical): server now requires AllowAnonymousAuth=true before
  it will accept auth.scheme="none" handshakes, so a configured
  bearer verifier is no longer silently bypassed
- #23 (high): WebSocket client transport serialises sends through a
  SemaphoreSlim so the BCL WebSocket's one-send-at-a-time contract is
  respected across auto-ack, pong, submit, cancel, and close paths
- #24 (high): JobSubmitFlow claims the idempotency key before
  registering the job record; a duplicate claim now short-circuits
  with a single error response and no second job is launched
- #25 (medium): credential-provisioner failures unwind the partially
  accepted job — record removed, idempotency key released, watchdog
  disposed, cancellation source disposed, and any tracked credentials
  revoked before responding with the error
- #26 (medium): Lease.isSubset accepts conservative narrower-glob
  children (literal-prefix narrowing for `prefix/**`, literal child
  under single-star parent) and the misleading unit test now uses a
  genuinely narrower pattern
- #29 (medium): client dispatch tears down the JobHandle when a
  ResultChunk is out of order or undecodable; ChunkAssembler also
  catches FormatException / DecoderFallbackException and surfaces
  them as InvalidRequest instead of throwing through the receive loop

Performance:
- #27 (medium): EventLog per-session buffer uses Queue<T> so Append
  cap-eviction and EvictExpired are O(1) per removed entry instead
  of O(n) RemoveAt(0) shifts

Testing (#30):
- Coverage rises from 73.4 % / 56.5 % to ~89 % line / ~75 % branch
  (above the 80 % line target). New focused tests cover stdio
  framing, WebSocket send serialisation and close behavior,
  EventLog replay/eviction edge cases, JobContext authority and
  event emission, ChunkAssembler index, Pending registry, JobHandle,
  full Codec round-trip for every message type and event body,
  bearer-verifier variants, error taxonomy, trace ids, JobManager
  lifecycle, and table-driven property tests for id round-trips,
  glob coverage, and lease subset laws
- CONTRIBUTING.md documents the dotnet test + reportgenerator
  command pair used to regenerate the coverage report

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: Nick Ficano

CONTRIBUTING.md

@@ -96,6 +96,25 @@ Two layers must pass before a PR merges:
 CI runs both on every PR. A PR that changes which feature flags the SDK
 negotiates must also update the README feature matrix in the same change.
 
+### Measuring coverage
+
+The full coverage report is regenerated with:
+
+```sh
+dotnet test ARCP.slnx --collect:"XPlat Code Coverage" \
+  --results-directory TestResults/review-coverage
+reportgenerator \
+  -reports:"TestResults/review-coverage/*/coverage.cobertura.xml" \
+  -targetdir:"TestResults/coverage-report" \
+  -reporttypes:"TextSummary"
+```
+
+Install the report tool once with
+`dotnet tool install -g dotnet-reportgenerator-globaltool`. The summary lands
+at `TestResults/coverage-report/Summary.txt`. The target is ≥ 80 % line
+coverage; transport and async-state-machine paths drive most of the
+remaining branch gaps and additions there are welcome.
+
 ## Coding standards
 
 Formatting is enforced by [Fantomas](https://fsprojects.github.io/fantomas/)

samples/Stdio/Program.fs

@@ -15,6 +15,7 @@ let main _argv =
         ArcpServer(
             { ArcpServerOptions.defaults with
                 Features = Features.All
+                AllowAnonymousAuth = true
             }
         )
 

src/Arcp.Cli/Program.fs

@@ -62,6 +62,7 @@ let private serveStdio (token: string option) : Task<int> =
         let options =
             { ArcpServerOptions.defaults with
                 BearerVerifier = buildVerifier token
+                AllowAnonymousAuth = Option.isNone token
             }
 
         let server = ArcpServer(options)

src/Arcp.Client/ArcpClient.fs

@@ -49,8 +49,16 @@ type ArcpClient(transport: ITransport, options: ArcpClientOptions) =
                 match payload.Body with
                 | JobEventBody.ResultChunk(rid, chunkSeq, data, enc, more) ->
                     let assembler = w.ChunkIndex.GetOrCreate rid
-                    assembler.Append(chunkSeq, data, enc, more) |> ignore
-                    w.Channel.Writer.TryWrite payload.Body |> ignore
+
+                    match assembler.Append(chunkSeq, data, enc, more) with
+                    | Ok _ -> w.Channel.Writer.TryWrite payload.Body |> ignore
+                    | Error err ->
+                        // Out-of-order or undecodable chunk: tear down
+                        // the handle so callers don't sit on a job that
+                        // will never produce a usable result.
+                        handles.TryRemove jid |> ignore
+                        w.Channel.Writer.TryComplete() |> ignore
+                        w.ResultSetter.TrySetResult(Error err) |> ignore
                 | other -> w.Channel.Writer.TryWrite other |> ignore
             | _ -> ()
 

src/Arcp.Client/Internal/ChunkAssembler.fs

@@ -26,18 +26,30 @@ type internal ChunkAssembler() =
                 ARCPError.InvalidRequest(sprintf "Out-of-order chunk: expected %d, got %d" expectedSeq chunkSeq, None)
             )
         else
-            let bytes =
-                match encoding with
-                | ChunkEncoding.Utf8 -> Encoding.UTF8.GetBytes data
-                | ChunkEncoding.Base64 -> Convert.FromBase64String data
+            let bytesResult =
+                try
+                    let bytes =
+                        match encoding with
+                        | ChunkEncoding.Utf8 -> Encoding.UTF8.GetBytes data
+                        | ChunkEncoding.Base64 -> Convert.FromBase64String data
 
-            buffer.Add bytes
-            expectedSeq <- expectedSeq + 1L
+                    Ok bytes
+                with
+                | :? FormatException as fx ->
+                    Error(ARCPError.InvalidRequest(sprintf "Invalid base64 chunk: %s" fx.Message, None))
+                | :? DecoderFallbackException as dx ->
+                    Error(ARCPError.InvalidRequest(sprintf "Invalid utf-8 chunk: %s" dx.Message, None))
 
-            if not more then
-                closed <- true
+            match bytesResult with
+            | Error e -> Error e
+            | Ok bytes ->
+                buffer.Add bytes
+                expectedSeq <- expectedSeq + 1L
 
-            Ok closed
+                if not more then
+                    closed <- true
+
+                Ok closed
 
     /// Materialise the assembled bytes. Throws if the stream has
     /// not yet seen its terminating chunk.

src/Arcp.Client/Transport/WebSocket.fs

@@ -18,18 +18,24 @@ open ARCP.Client
 /// One text frame per envelope. The receive loop reassembles
 /// continuation frames into a single message.
 type WebSocketClientTransport(socket: WebSocket, ownsSocket: bool) =
-    let sendLock = obj ()
+    // The BCL WebSocket allows only one outstanding send at a time,
+    // so we serialise concurrent callers (auto-ack, pong, submit,
+    // cancel, close) through an async-aware semaphore — a plain
+    // `lock` only covers the synchronous call to `SendAsync` and
+    // would release before the returned Task completes.
+    let sendLock = new SemaphoreSlim(1, 1)
     let mutable closed = false
 
     let sendOne (env: Envelope) (ct: CancellationToken) : Task =
         task {
             let json = Codec.writeEnvelope env
             let bytes = Encoding.UTF8.GetBytes json
-            // Concurrent sends are not allowed on a single ClientWebSocket;
-            // serialise through a lock + a write task.
-            do!
-                lock sendLock (fun () ->
-                    socket.SendAsync(ArraySegment<byte>(bytes), WebSocketMessageType.Text, true, ct))
+            do! sendLock.WaitAsync ct
+
+            try
+                do! socket.SendAsync(ArraySegment<byte>(bytes), WebSocketMessageType.Text, true, ct)
+            finally
+                sendLock.Release() |> ignore
         }
         :> Task
 

src/Arcp.Core/Lease.fs

@@ -116,14 +116,58 @@ module Lease =
     let private violation (msg: string) : ARCPError =
         ARCPError.LeaseSubsetViolation(msg, None)
 
+    /// Longest prefix of `pattern` containing no glob metacharacters.
+    let private literalPrefix (pattern: string) : string =
+        let mutable i = 0
+        let mutable stop = false
+
+        while not stop && i < pattern.Length do
+            match pattern.[i] with
+            | '*'
+            | '?' -> stop <- true
+            | _ -> i <- i + 1
+
+        pattern.Substring(0, i)
+
+    let private isLiteralPattern (pattern: string) : bool =
+        not (pattern.Contains '*') && not (pattern.Contains '?')
+
+    /// Conservative glob coverage: returns true only when every
+    /// string matched by `child` is also matched by `parent`.
+    /// Returns false when coverage cannot be proven (callers should
+    /// then reject the child as not provably a subset).
+    let private globCovers (parent: string) (child: string) : bool =
+        if parent = child then true
+        elif parent = "**" then true
+        elif parent.EndsWith "/**" then
+            // Parent of the form `prefix/**` matches any string starting
+            // with `prefix/`. A child is provably a subset when its
+            // literal prefix already starts with `prefix/` — any glob
+            // suffix after that can only produce strings matched by the
+            // parent.
+            let prefix = parent.Substring(0, parent.Length - 3)
+            let childLiteral = literalPrefix child
+            childLiteral.StartsWith(prefix + "/")
+        elif isLiteralPattern parent then
+            // Literal parent only covers identical patterns.
+            false
+        elif isLiteralPattern child then
+            // Parent has globs, child is concrete — provable by direct
+            // regex match (e.g. parent `render.*`, child `render.foo`).
+            Glob.isMatch parent child
+        else
+            // Conservatively reject mixed glob shapes the checker can't
+            // prove are subsets.
+            false
+
     let private checkNamespace (parent: LeaseGrant) ((ns: string), (childGlobs: string list)) : ARCPError option =
         match Map.tryFind ns parent.Capabilities with
         | None -> Some(violation (sprintf "Child lease has namespace %s not in parent" ns))
         | Some _ when ns = Capabilities.CostBudget -> None
         | Some parentGlobs ->
             childGlobs
             |> List.tryPick (fun cg ->
-                if parentGlobs |> List.exists (fun pg -> pg = cg || pg = "**") then
+                if parentGlobs |> List.exists (fun pg -> globCovers pg cg) then
                     None
                 else
                     Some(violation (sprintf "Child glob %s in %s not covered by parent" cg ns)))

src/Arcp.Runtime/ArcpServer.fs

@@ -19,6 +19,10 @@ type ArcpServerOptions =
         HeartbeatIntervalSec: int
         ResumeWindowSec: int
         BearerVerifier: IBearerVerifier
+        /// When true, clients may handshake with `auth.scheme = "none"`
+        /// and receive an `AnonymousPrincipal`. Defaults to false so
+        /// that configuring a bearer verifier is not silently bypassed.
+        AllowAnonymousAuth: bool
         TimeProvider: TimeProvider
         Provisioner: ICredentialProvisioner option
         CredentialStore: ICredentialStore option
@@ -27,7 +31,9 @@ type ArcpServerOptions =
 [<RequireQualifiedAccess>]
 module ArcpServerOptions =
     /// Sensible defaults: dev-mode bearer auth, every feature flag,
-    /// 30s heartbeat, 600s resume window.
+    /// 30s heartbeat, 600s resume window. Anonymous auth is off by
+    /// default — opt in via `AllowAnonymousAuth = true` for local
+    /// stdio/dev setups.
     let defaults: ArcpServerOptions =
         {
             Runtime =
@@ -39,6 +45,7 @@ module ArcpServerOptions =
             HeartbeatIntervalSec = 30
             ResumeWindowSec = 600
             BearerVerifier = DevModeBearerVerifier() :> IBearerVerifier
+            AllowAnonymousAuth = false
             TimeProvider = TimeProvider.System
             Provisioner = None
             CredentialStore = None
@@ -187,6 +194,7 @@ type ArcpServer(options: ArcpServerOptions) =
                         transport
                         options.Runtime
                         options.BearerVerifier
+                        options.AllowAnonymousAuth
                         options.TimeProvider
                         eventLog
                         supportedFeatures

src/Arcp.Runtime/Internal/JobSubmitFlow.fs

@@ -166,73 +166,109 @@ module internal JobSubmitFlow =
                     | Error err -> do! EnvelopeOut.respondWithError ctx requestId err ct
                     | Ok constraints ->
                         let jobId = JobId.newId ()
-                        let budgets = BudgetCounters()
-                        budgets.SetInitial(Lease.initialBudgets lease)
-                        let cts = new CancellationTokenSource()
-                        let watchdog = buildWatchdog timeProvider jobs credentialRegistry jobId constraints
-
-                        match submit.IdempotencyKey with
-                        | Some key ->
-                            match jobs.TryClaimIdempotencyKey(key, jobId) with
-                            | Error err -> do! EnvelopeOut.respondWithError ctx requestId err ct
-                            | Ok() -> ()
-                        | None -> ()
-
-                        let record: JobRecord =
-                            {
-                                JobId = jobId
-                                SessionId = ctx.SessionId
-                                Principal = ctx.Principal
-                                Agent = resolvedAgent
-                                Input = submit.Input
-                                Lease = lease
-                                Constraints = constraints
-                                Credentials = []
-                                Budgets = budgets
-                                ParentJobId = None
-                                TraceId = traceIdOpt
-                                CreatedAt = timeProvider.GetUtcNow()
-                                Cancellation = cts
-                                Watchdog = watchdog
-                                Status = JobStatus.Pending
-                                LastEventSeq = 0L
-                            }
 
-                        jobs.Register record
-                        let! issued = issueCredentialsAsync provisioner credentialRegistry record ct
+                        // Claim the idempotency key first so a duplicate
+                        // submission short-circuits before any side effects
+                        // (record registration, watchdog start, provisioner
+                        // call). Without this, two concurrent submits with
+                        // the same key both fell through and created jobs.
+                        let claimResult =
+                            match submit.IdempotencyKey with
+                            | Some key -> jobs.TryClaimIdempotencyKey(key, jobId)
+                            | None -> Ok()
 
-                        match issued with
+                        match claimResult with
                         | Error err -> do! EnvelopeOut.respondWithError ctx requestId err ct
-                        | Ok credentials ->
-                            record.Credentials <- credentials
-
-                            let initialBudget =
-                                if budgets.Snapshot() = Map.empty then
-                                    None
-                                else
-                                    Some(budgets.Snapshot())
+                        | Ok() ->
+                            let budgets = BudgetCounters()
+                            budgets.SetInitial(Lease.initialBudgets lease)
+                            let cts = new CancellationTokenSource()
+                            let watchdog = buildWatchdog timeProvider jobs credentialRegistry jobId constraints
 
-                            let accepted: JobAcceptedPayload =
+                            let record: JobRecord =
                                 {
-                                    JobId = jobId.Value
+                                    JobId = jobId
+                                    SessionId = ctx.SessionId
+                                    Principal = ctx.Principal
+                                    Agent = resolvedAgent
+                                    Input = submit.Input
                                     Lease = lease
-                                    LeaseConstraints = constraints
-                                    Budget = initialBudget
-                                    Credentials = if List.isEmpty credentials then None else Some credentials
-                                    AcceptedAt = record.CreatedAt
+                                    Constraints = constraints
+                                    Credentials = []
+                                    Budgets = budgets
+                                    ParentJobId = None
                                     TraceId = traceIdOpt
+                                    CreatedAt = timeProvider.GetUtcNow()
+                                    Cancellation = cts
+                                    Watchdog = watchdog
+                                    Status = JobStatus.Pending
+                                    LastEventSeq = 0L
                                 }
 
-                            do! sendAccepted ctx.Transport ctx.SessionId requestId jobId accepted ct
-
-                            match agentHandlers.TryGetValue resolvedAgent with
-                            | true, handler -> JobLauncher.launch jobs credentialRegistry timeProvider record handler
-                            | _ ->
-                                do!
-                                    EnvelopeOut.respondWithError
-                                        ctx
-                                        requestId
-                                        (ARCPError.AgentNotAvailable resolvedAgent)
-                                        ct
+                            jobs.Register record
+                            let! issued = issueCredentialsAsync provisioner credentialRegistry record ct
+
+                            match issued with
+                            | Error err ->
+                                // Acceptance failed after registration —
+                                // unwind state so the failed job does not
+                                // surface in list/get and the idempotency
+                                // key is free for a retry.
+                                jobs.Unregister jobId
+
+                                match submit.IdempotencyKey with
+                                | Some key -> jobs.ReleaseIdempotencyKey(key, jobId)
+                                | None -> ()
+
+                                watchdog |> Option.iter (fun w -> (w :> IDisposable).Dispose())
+
+                                try
+                                    cts.Cancel()
+                                with _ ->
+                                    ()
+
+                                try
+                                    cts.Dispose()
+                                with _ ->
+                                    ()
+
+                                try
+                                    do! credentialRegistry.RevokeJobAsync(jobId, ct)
+                                with _ ->
+                                    ()
+
+                                do! EnvelopeOut.respondWithError ctx requestId err ct
+                            | Ok credentials ->
+                                record.Credentials <- credentials
+
+                                let initialBudget =
+                                    if budgets.Snapshot() = Map.empty then
+                                        None
+                                    else
+                                        Some(budgets.Snapshot())
+
+                                let accepted: JobAcceptedPayload =
+                                    {
+                                        JobId = jobId.Value
+                                        Lease = lease
+                                        LeaseConstraints = constraints
+                                        Budget = initialBudget
+                                        Credentials = if List.isEmpty credentials then None else Some credentials
+                                        AcceptedAt = record.CreatedAt
+                                        TraceId = traceIdOpt
+                                    }
+
+                                do! sendAccepted ctx.Transport ctx.SessionId requestId jobId accepted ct
+
+                                match agentHandlers.TryGetValue resolvedAgent with
+                                | true, handler ->
+                                    JobLauncher.launch jobs credentialRegistry timeProvider record handler
+                                | _ ->
+                                    do!
+                                        EnvelopeOut.respondWithError
+                                            ctx
+                                            requestId
+                                            (ARCPError.AgentNotAvailable resolvedAgent)
+                                            ct
         }
         :> Task