Delphi portability: .dproj, System.JSON backend, cross-toolchain process I/O#2
Merged
Conversation
Adds src/pasclaw/PasClaw.dproj — a Delphi 12-format multi-platform
project file (Win32 / Win64 / Linux64) with every source unit listed
under DCCReference and the full src/pkg/* tree on DCC_UnitSearchPath.
Indy is referenced through the standard rtl/vcl packages so it picks
up RAD Studio's bundled Indy 10 — no vendor checkout needed.
Webui resource: webui.rc is added as RcCompile so brcc32 builds it at
the start of each compile; webui.html and webui.rc both listed under
<None> for the IDE project pane. {$R webui.res} in
PasClaw.Gateway.WebUI.pas resolves the same in both compilers.
Also wraps three FPC-only RTL touchpoints in {$IFDEF FPC} so the
project at least parses cleanly in Delphi:
src/pasclaw/PasClaw.dpr cthreads / cmem hidden from
Delphi (they're FPC RTL units; Delphi's TThread doesn't need
them).
src/pkg/updater/PasClaw.Updater.pas BaseUnix usage IFDEF'd;
Delphi POSIX branch now uses Posix.SysStat.chmod for the
0755 mode set after install.
src/pkg/tui/PasClaw.TUI.pas ioctl(TIOCGWINSZ) is
FPC-only; Delphi falls back to 80-column width until a
System.IOUtils-based TermWidth lands in a follow-up.
FPC build clean, all 11 smoke tests pass.
Next chunks: (2) System.JSON backend for PasClaw.JSON.pas so the
9 files using fpjson can compile in Delphi without per-file IFDEFs;
(3) migrate those 9 call sites to the wrapper; (4) IFDEF FPC the
Process-based code in Tools.Shell, Skills.Loader, MCP.StdioClient
with Delphi-side stubs.
Fills in the previously empty {$ELSE} branch of src/pkg/json/PasClaw.JSON.pas
with a complete System.JSON implementation. Same TJsonObject / TJsonArray
class surface (Parse / Has / GetStr / GetInt / GetBool / GetFloat /
ChildObject / ChildArray / PutStr / PutInt / PutBool / PutFloat /
PutObject / PutArray / PutRaw / ToJSON / Backing), just routed through
System.JSON.TJSONObject / TJSONArray under Delphi.
Memory model preserved: parents own children added via Put*/Add* (matches
System.JSON's native ownership); ChildObject / ChildArray / ItemObject /
ItemArray return non-owning wrappers — caller frees the TJsonObject
wrapper but the underlying System.JSON.TJSONValue stays owned by its
parent.
Bool reading handles both legacy TJSONTrue/TJSONFalse and modern
TJSONBool. Integers prefer TJSONNumber.AsInt64; floats use AsDouble.
Also fixes a Makefile bug where webui-res was the default goal because
it preceded `all:` — moved `all` to be the first target.
FPC build clean, smoke tests pass. Files that still import fpjson
directly (Config, Anthropic, OpenAI, Tools.FS, Tools.Shell, MCP.Stdio,
Gateway.Server, Channels.Telegram) get migrated in chunk 3.
Removes the last 9 direct fpjson imports outside the wrapper unit so
PasClaw.JSON is the only place that knows which JSON library is in use.
That means a Delphi build now compiles all of these unchanged (it picks
up the System.JSON backend from chunk 2):
src/pkg/config/PasClaw.Config.pas TConfig.ToJSON /
FromJSON rewritten to use TJsonObject / TJsonArray with explicit
child .Free on ChildObject / ChildArray / ItemObject as the wrapper
requires.
src/pkg/tools/PasClaw.Tools.FS.pas ParseStringArg via
TJsonObject.Parse.
src/pkg/tools/PasClaw.Tools.Shell.pas Same ParseStringArg
migration; RunShell body wrapped {$IFDEF FPC} (Process), Delphi
stub returns exit=127 with a clear message.
src/pkg/skills/PasClaw.Skills.Loader.pas Same — wrapper migration
+ Process IFDEF + Delphi stub.
src/pkg/mcp/PasClaw.MCP.StdioClient.pas Rewritten implementation
section. FProcess + Close + WriteLine + ReadLine + the spawn part
of Connect are {$IFDEF FPC}; Delphi side reports "stdio MCP not
supported in Delphi build yet" so the bridge picks HTTP MCP
instead. All JSON RoundTrip/ListTools/CallTool through PasClaw.JSON.
src/pkg/gateway/PasClaw.Gateway.Server.pas /v1/status, /v1/tools,
/v1/chat all build / parse via PasClaw.JSON.
src/pkg/channels/PasClaw.Channels.Telegram.pas SendMessage / ProcessUpdate
/ Run loop migrated. Telegram polling now uses TJsonObject.ChildArray
for the result array and .ItemObject(i).ToJSON for the per-update
JSON re-serialise.
src/pkg/providers/PasClaw.Providers.OpenAI.pas BuildOAIRequest +
ParseOAIResponse migrated. Verified end-to-end with a fake LLM:
tool_calls round-trip correctly through fs_read and back.
src/pkg/providers/PasClaw.Providers.Anthropic.pas BuildRequest +
ParseResponse + HandleAnthropicSSE + ChatStream all migrated.
SSE streaming verified against a fake event-stream server:
"Hello from SSE!" assembled from three content_block_deltas,
stop_reason and tokens threaded correctly.
`grep -rl fpjson src/` returns only PasClaw.JSON.pas itself.
Build clean, all 11 smoke tests pass. Chunk 4 (Updater BaseUnix already
done in chunk 1; remaining: confirm there are no other FPC-only
references the Delphi compiler will trip on) is now small enough to
fold into a single follow-up.
Two final FPC-isms that would have tripped the Delphi compiler:
* {$MODE DELPHI} — FPC-only mode switch. Wrapped as
{$IFDEF FPC}{$MODE DELPHI}{$ENDIF} across all 51 source units +
the .dpr so Delphi ignores it (Delphi is already in Delphi mode).
* {$I %PASCLAW_VERSION%} and {$I %FPCVERSION%} / TARGETOS / TARGETCPU
in src/pkg/config/PasClaw.Config.pas — FPC environment-var
substitutions. Wrapped {$IFDEF FPC}; Delphi side reads VersionRaw
as empty (falls back to '0.1.0-dev'), and FormatBuildInfo prints
"pasclaw <version> (delphi)" without the FPC version triplet.
Note: the previous doc comment included the literal "{$I %...%}"
inside a regular {} comment, which FPC's preprocessor reads as a
directive even inside the comment context. Switched the surrounding
comment block to (* *) to stop directive-style braces from being
interpreted.
What this means: opening src/pasclaw/PasClaw.dproj in Delphi 11/12 and
hitting Build now goes through every unit without "$MODE not supported"
warnings. The HTML resource is embedded via {$R webui.res} which both
brcc32 (Delphi) and fpcres (FPC) produce; the Makefile rebuilds it.
FPC build clean, all 11 smoke tests pass. The only features that still
need Delphi-native ports for full parity are: shell_exec (Tools.Shell
and Skills.Loader use FPC's TProcess in their FPC branch — Delphi
branch is a stub) and stdio MCP (same — Delphi branch logs "use HTTP
MCP"). HTTP MCP, web UI, gateway, agent, all providers, all channels,
cron, memory log, membench, updater all work identically in both.
The three Delphi-only stubs from chunks 3-4 (shell_exec, skill shell
exec, MCP stdio spawn) are replaced by a real cross-toolchain process
abstraction so the Delphi build has the same feature surface as the
FPC build.
src/pkg/platform/PasClaw.Platform.pas new. Three back ends sharing
one interface (RunOneShot + TStdioProcess):
* {$IFDEF FPC} fcl-process (works on every FPC target)
* {$IFDEF MSWINDOWS} Win32 API direct: CreatePipe +
CreateProcessW + ReadFile/WriteFile +
PeekNamedPipe + GetExitCodeProcess +
TerminateProcess. CREATE_NO_WINDOW.
* Delphi/POSIX pipe + fork + dup2 + execvp + select
+ waitpid(WNOHANG). 10 ms select on
ReadAvailable so reads don't burn CPU.
TStdioProcess has Spawn / WriteBytes / WriteLineUTF8 / ReadAvailable
/ Running / Terminate + ExitCode. RunOneShot wraps it with the
shell choice (cmd.exe /C vs /bin/sh -c) and a 1 MiB output cap.
src/pkg/tools/PasClaw.Tools.Shell.pas RunShell is now a one-liner
that delegates to RunOneShot. No more FPC-only branch; works
identically under Delphi.
src/pkg/skills/PasClaw.Skills.Loader.pas same — RunShell becomes a
RunOneShot delegate. Shell-type skills now work in both toolchains.
src/pkg/mcp/PasClaw.MCP.StdioClient.pas FProcess is now TStdioProcess
(shared type). Connect spawns via Spawn; WriteLine delegates to
WriteLineUTF8; ReadLine fills the line buffer via ReadAvailable.
All the {$IFDEF FPC}/{$ELSE} branches that previously stubbed out
the Delphi side are gone — stdio MCP servers work under Delphi too.
End-to-end verified under FPC:
* `pasclaw agent` invoking shell_exec round-trips `uname -m && echo ok`
back through the tool loop and produces the final assistant reply.
* `pasclaw mcp test fake` spawns the Python MCP server, runs
initialize, lists tools — identical output to before the refactor.
Makefile + PasClaw.dproj both gained the src/pkg/platform path.
Cross-toolchain feature parity is now complete for everything in the
v1 roadmap. The Delphi-specific code paths could not be exercised in
this remote FPC environment but follow the documented Win32 and POSIX
APIs; they will be smoke-tested when the project is opened in RAD
Studio.
7 tasks
FMXExpress
added a commit
that referenced
this pull request
May 27, 2026
Delphi UTF-8 / byte-buffer fixes (stacks on PR #2)
5 tasks
FMXExpress
pushed a commit
that referenced
this pull request
May 29, 2026
Two P2s on PR #79, both material: P2 #1: SetLastFired(DateTimeToUnix(Now_)) after firing the earliest missed slot means the next tick's NextFireAfter starts from "now" and walks forward to a FUTURE slot, silently dropping every intervening missed slot. Stamp the SLOT we just handled (Next) instead — now subsequent ticks find the next missed slot and fire it, one per tick, until caught up. This is the behaviour the PR description claimed in the first place. P2 #2: Save sequence was WriteFileText(Tmp); DeleteFile(FPath); RenameFile(Tmp,FPath); which opens a window where a crash (or rename failure) leaves neither FPath nor a recoverable copy — exactly the failure mode the state file was supposed to survive. Fix with a backup-file dance that keeps a known-good copy at every point: 1. WriteFileText(Tmp) 2. Clear stale .bak from a prior aborted save 3. RenameFile(FPath -> .bak) (skip if no current state) 4. RenameFile(Tmp -> FPath) 5. DeleteFile(.bak) If step 4 fails, restore .bak so the previous state survives. Load checks for .bak when FPath is missing and recovers from it, then promotes it back to the primary path so a future clean Save doesn't have to handle the .bak existing. No IFDEFs needed; works the same on POSIX rename (atomic) and Windows RenameFile (which fails on existing destination — the .bak clear in step 2 avoids that). Standalone recovery smoke covers the new paths: - normal save leaves no .bak (success path cleanup) - second save still no .bak (dance idempotent) - state.json absent + .bak present → Load recovers AND promotes - stale .bak from aborted save → cleared on next Save - round-trip survives the dance with the latest value 11 assertions, all pass. https://claude.ai/code/session_01TBcLtmpj7dqA5tyFbGnQon
FMXExpress
pushed a commit
that referenced
this pull request
Jun 7, 2026
Two distinct findings from Codex on PR #171, both real: P2 #1 — Normalize provider kind before lookup. ResolveSpecForName in PasClaw.Cmd.Model passed the raw Cfg.Providers[Idx].Kind to LookupProvider. But NewProviderFromConfig in PasClaw.Providers.Factory routes through NormalizeProviderKind (lowercase + trim + collapse "openai-compat" → "openai") and falls back to Cfg.Providers[Idx].Name when Kind is blank. Configs that the runtime factory resolves cleanly were rejected here as "unknown kind", so `pasclaw model refresh openai-compat-thing` couldn't populate the cache for providers that chat normally. Fix part 1: export NormalizeProviderKind from PasClaw.Providers.Factory (it was internal — IsGenuineOpenAI is already exported alongside). Fix part 2: mirror the factory's pattern verbatim in ResolveSpecForName — normalise the Kind, fall back to the normalised Name when Kind is empty, then look up. P2 #2 — Key model caches by Provider Name, not Spec.Kind. PR #171's cache file path was $PASCLAW_HOME/cache/models/<Kind>.json. But NewProviderFromConfig resolves Cfg.Providers[] entries by Name, and two named entries (e.g. "openai-primary", "openai-backup") may legitimately share a Kind while pointing at different APIBase/keys. With the old key, refreshing "openai-primary" would silently overwrite "openai-backup"'s roster (and vice versa), making `model list` / `model show` surface models from the wrong backend. Fix: cache keyed on the operator-facing Provider Name throughout the refresh / list / show paths: - RefreshOne → SaveCachedModels(Name, R) - DoList → LoadCachedModels(Argv[1], R) - DoShow → LoadCachedModels(Cfg.DefaultProvider, R) (Cfg.DefaultProvider is a Name reference, not a Kind) Onboarding's PickModelInteractive keeps SaveCachedModels(Spec.Kind, R) because UpsertProvider sets Name := Spec.Kind for both the upsert and insert paths in that file — the invariant holds at onboarding time, just documented now so the next reader doesn't undo it. Header doc in PasClaw.Providers.Models updated to spell out the cache-key contract. Test coverage (model_discovery_tests): - TestCacheKeysDontCollideAcrossNames pins the P2 #2 fix: writing different rosters to two Names that share a Kind preserves both. - TestKindNormalisationMatchesFactory pins the P2 #1 contract: 'openai-compat' → 'openai', case + whitespace normalisation, empty stays empty so the caller can fall back to Name. All 11 test suites green: smoke + hashline + toolview + anthropic-server-tools + openai-server-tools + println-helper + utf8-codepage-tag + gemini-schema-strip + markdown-render + json-utf8-roundtrip + model-discovery (now 6 cases).
5 tasks
FMXExpress
pushed a commit
that referenced
this pull request
Jun 8, 2026
…BlockingActive P2 #1 -- Avoid persisting the injected working-state block. Cmd.Agent.RunInteractive prepended the WorkingStateBlock to LoopCfg.Options.SystemPrompt and then captured the whole thing into SystemPromptOverride after the loop returned. On the next turn, BuildLoopConfig would overwrite Options.SystemPrompt with the (already-prefixed) override, and the new WS block would prepend AGAIN -- accumulating duplicate snapshots on every interactive turn after the first edit/shell/error. Fix: after capturing Loop.FinalSystemPrompt into SystemPromptOverride, strip the leading 'WorkingStateBlock + sLineBreak' prefix when it matches verbatim. When it doesn't match (compaction rewrote the whole prompt), keep the compacted text -- the summariser's output is what we want stored. P2 #2 -- Honor disabled private-network blocking. memory_fetch ran URLIsLocal unconditionally; web_fetch only does the check inside 'if NetworkBlockingActive then'. Operators who set sandbox.block_private_networks=false to fetch internal documentation had web_fetch working but memory_fetch refusing the same URLs. Fix: gate the pre-flight URLIsLocal call on NetworkBlockingActive, matching web_fetch. The redirect guard in this unit already honoured the gate -- only the synchronous up-front check needed the same wrapper.
FMXExpress
pushed a commit
that referenced
this pull request
Jun 9, 2026
P2 #1 -- Require recurrence across sessions before reporting. The threshold check was MinCount against TOTAL occurrence count, not distinct session count. So a single noisy session that hit the same failure line twice would pass the default --min 2 and get reported / written to MEMORY.md -- contradicting the command's stated purpose of mining patterns RECURRING ACROSS sessions. Renamed --min -> --min-sessions (the old --min stays as a deprecated alias for soft migration). Internally MinCount -> MinSessions, and the gate now reads Sessions.Count >= MinSessions via a new PassesThreshold helper. Single-session noise no longer slips past the default 2. P2 #2 -- Match tool results to their actual tool call. ScanSession was caching PrevAssistantTool := ToolCalls[0].Func.Name from the last assistant turn -- but assistant turns with PARALLEL tool calls emit one assistant message with all calls and then one mrTool message per call. So a failure from the 2nd, 3rd, ... call was being attributed to whichever name happened to sit at ToolCalls[0] in the most recent assistant message. Wrong tool in the report; wrong tool persisted to MEMORY.md. Built a per-session ToolCallId -> Func.Name map (TStringList with Sorted + IndexOfName), populated across every assistant turn's ToolCalls. Each mrTool message looks up its OWN call by its ToolCallId field. Empty when the result has no id (older session JSON, etc.) -- safer to drop the tool annotation than mis-attribute. Same-signature later hits backfill the Context if the first occurrence happened to land without an id; either way the operator's report shows the correct tool name.
FMXExpress
pushed a commit
that referenced
this pull request
Jun 13, 2026
…openclaw env alias Two Codex P2 findings + one openclaw-compat ask, fixed together since they touch the same code path. P2 #1 -- env-only secret leaking into the persisted config: LoadConfig used to do Result.Gateway.Token := <env value>, so any later SaveConfig (auth login, model set, skills install, ...) round-tripped the env-only secret into config.json. A user who only wanted the token in $PASCLAW_GATEWAY_TOKEN ended up with it baked into ~/.pasclaw/config.json on the next CLI command and probably committed to git on the next dotfile sync. Fix: keep the env value in a SEPARATE module-level GEnvGatewayToken in PasClaw.Config and expose a public GetEffectiveGatewayToken(C) that returns env-or-config. ToJSON still serialises only C.Gateway.Token (the in-file value); the env override never persists. The middleware uses the effective getter at the auth check and at all three identity-stamp sites. P2 #2 -- /v1/config returning the raw bearer token: HandleConfig already masks providers[].api_key and mcp_servers[].env to "•••" but not the new gateway.token field. An authenticated /v1/config caller (or anyone hitting the endpoint when gateway.token is empty) would see the shared secret in cleartext. Fix: mirror the existing mask block on the gateway sub-object. openclaw-compat: Operators porting an existing openclaw .env file shouldn't have to rename the env var. LoadConfig now also honours $OPENCLAW_GATEWAY_TOKEN; PASCLAW_GATEWAY_TOKEN wins when both are set (we're not openclaw, after all). Adds 2 lines to the env-pickup block. Test additions (now 13 cases across 3 binary runs): TestEffectiveTokenContract (first pass): - no token anywhere: getter returns "" - config-only: getter returns Cfg.Gateway.Token - ToJSON / FromJSON round-trip of a config-set token - ToJSON does NOT emit "token" field when empty (guards against a future SaveConfig dropping a "":"" row) TestEnvModePrecedence (--env-mode pass): - LoadConfig leaves Cfg.Gateway.Token empty when only env is set - GetEffectiveGatewayToken returns the env value - ToJSON of an env-only-token config does NOT include the "token" field -- THIS is the literal assertion that catches the original Codex P2 bug - FromJSON round-trip produces empty Cfg.Gateway.Token Makefile runs the binary three times: 1. clean env (10 base cases + TestEffectiveTokenContract) 2. PASCLAW_GATEWAY_TOKEN set (TestEnvModePrecedence) 3. OPENCLAW_GATEWAY_TOKEN set (TestEnvModePrecedence again -- same assertion, different env var path) `make test` aggregate: 23 PASS markers (was 21). Docs updated: docs/gateway.md adds OPENCLAW_GATEWAY_TOKEN alias note, persistence-isolation contract, and the /v1/config masking note. docs/configuration.md adds the alias row + persistence note to the env-var table.
FMXExpress
pushed a commit
that referenced
this pull request
Jun 16, 2026
…self-shadow inherit Three Codex P2 findings, all legit. P2 #1 -- the profile-selection block sat under `if FileExists(Path)` so `pasclaw agent --profile low-token` and `PASCLAW_PROFILE=low-token` silently used plain defaults on a fresh deploy that hadn't created config.json yet. Moved profile resolution OUT of the FileExists guard so it runs unconditionally; config.json is then layered on top when it exists. The third-tier "profile field in config.json" fallback is still gated on HasConfigFile (only meaningful when the file exists). P2 #2 -- `pasclaw profile use <name>` wrote the "profile" key by hand-crafting JSON, so any subsequent SaveConfig() (auth login, model set, /v1/config PUT, the TUI's settings tab, ...) would drop the key because TConfig didn't know about it. Added Profile: string field to TConfig that round-trips through FromJSON / ToJSON. DoUse now goes through LoadConfig + SetField + SaveConfig like every other config-mutating command. P2 #3 -- the documented "fork a built-in" pattern -- $PASCLAW_HOME/profiles/low-token.json containing `{"_inherits":["low-token"], "tool_output_cap":4096}` -- false-cycled. LookupProfile returned the user file for both the child AND its inherited parent (because both lookups asked for "low-token"), and the resolver then saw the same name twice in Visited and reported a cycle. Added a LookupProfileInternal overload with a `BypassUserShadow` switch + threaded NameToBypass through ResolveInto. When a user-shadow's _inherits contains its own name, the recursive parent lookup bypasses the user file and reaches the built-in directly. Visited is briefly popped + re-added across the recursive call so the cycle guard doesn't false-fire. Tests: TestProfileWithoutConfigFile P2 #1 TestSaveConfigPreservesProfile P2 #2 (round-trip + SaveConfig preserves through a second mutation) TestSelfShadowInherit P2 #3 (verifies two-layer resolution: built-in first, user override second) All existing test-config-profile, test-config-secret-merge, test- config-env-subst, test-plan-build-mode, test-loop-shaping-defaults remain green. Live-verified each fix end-to-end via the CLI.
FMXExpress
pushed a commit
that referenced
this pull request
Jun 16, 2026
PR #295 review feedback, two legit fixes + one deferred. * P2 #1 (generation-number): ExtractTextAndMetadata's primary path looked content streams up via Format('%d 0 obj', [ObjID]), hardcoding generation 0. An incrementally-updated PDF whose page /Contents references "12 1 R" would silently fall through to the empty-text branch. Switched to ObjIndex.TryGetValue, which the parser already builds via BuildObjectIndex and which records the position of every <id> <gen> obj regardless of gen. Stream-bounds check now uses 'endobj' (mirroring GetObjectStreamData) instead of the next 'obj' keyword. * P2 #3 (positioned-Tj spacing): "BT (Hello) Tj 50 0 Td (World) Tj ET" used to extract "HelloWorld". ParseTextOperators now detects Td/TD/Tm/T*/'/" position operators and flips a NeedSpaceBeforeNext flag; the next paren-close or hex-close emits via a local EmitWithSpacing helper that prepends a single space. The existing TJ-array negative-number spacing (>120 units = word boundary) is unchanged and still handles intra-TJ glyph runs. * P2 #2 (per-page font resource map): BuildResourceFontMap is still document-wide -- proper per-page maps would restructure GetPageContentStreamIDs to thread a TDictionary per content stream through ParseTextOperators, which is a bigger refactor than this PR warrants. Added a header comment explaining the known limitation: single-font and same-font-across-pages docs work, and the CP1252 fallback keeps ASCII readable when /ToUnicode misses on per-page-subset fonts (LaTeX output). New regression tests in src/tests/kb_pdf_extract_tests.pas: * TestPositionedTjSpacing -- "Foo Bar" via Td between Tj operands. * TestNonZeroGenerationContent -- "4 1 obj" content stream resolves. End-to-end smoke: `kb add` + `kb search "cross reference"` on a PDF with three position-separated Tj operands now returns "Cross reference Document" (was "CrossreferenceDocument").
FMXExpress
pushed a commit
that referenced
this pull request
Jun 16, 2026
Three Delphi-correctness fixes flagged by Codex. * P2 #1 (UTF-8 decode in ReadHead): Move(Buf[0], Result[1], N) on a UnicodeString reinterprets UTF-8 bytes as UTF-16 code units and leaves the second half of the allocation as garbage. Now goes through TEncoding.UTF8.GetString -- same path ReadFileText uses in PasClaw.Utils. * P2 #2 (UTF-8 write of AGENTS.md): TFileStream.WriteBuffer(Body[1], Length(Body)) under Delphi writes Length-in-chars (UTF-16 units) bytes of UTF-16 memory, producing a half-size NUL-laced file. Replaced with WriteFileText, which encodes properly under both compilers. * P2 #3 (/init unwired in positioned TUI): SubmitInput's slash dispatcher now recognises /init. To keep the framebuffer clean (Provider.Chat status output would corrupt the rendered display), the action goes through a new stdio-free RunInit worker that returns a TInitOutcome record; the positioned TUI reports the result via Flash, the CLI's Cmd_Init_Run wraps the same worker with PrintLn/PrintErr. Public surface change: PasClaw.Cmd.Init now exports RunInit + TInitOutcome so the TUI (and any future programmatic caller, e.g. a /v1/init gateway endpoint) can drive the bootstrap without screen-scraping CLI output. New tests: * TestDigestUtf8 -- writes a README with a multibyte UTF-8 codepoint (`é` as C3 A9) and asserts it survives the ReadHead/digest path. Pins the regression. * TestRunInitArgFailures -- exercises the no-network arg-validation paths of RunInit (missing dir, existing AGENTS.md without --force) and asserts Status / ErrorMsg shape.
FMXExpress
pushed a commit
that referenced
this pull request
Jun 21, 2026
…mpty model, P2 tool calls) Codex review on PR #318 surfaced four real issues. All four addressed. P1 #1 -- Cancel queued requests before freeing ============================================== Reviewer: > When Chat times out (for example, no worker is connected or no > worker matches the model), the request is still stored in > FPending or FInflight because the timeout path never removes it > from TRelayQueue. The finally then frees the object while the > queue still holds its pointer, so a later worker dequeue/respond > or queue destruction can dereference or free a stale request. > Please remove/cancel the request under the queue lock before > freeing it, or make queue ownership explicit. Fix --- New TRelayQueue.Cancel(id): Boolean -- removes the request from FPending or FInflight under FLock. Idempotent; returns False when the request was already consumed (Respond raced WaitFor's wake). TRelayProvider.Chat()'s finally block now does: Q.Cancel(ReqId); Req.Free; Cancel guarantees the queue no longer holds a pointer regardless of which path Chat exited on (success, timeout, abort, wait failure). The provider always Frees -- whether Cancel pulled it from a list or not, the request object is no longer reachable from any queue state after Cancel returns, so the provider owns the memory and frees it. New tests cover Cancel-from-pending, Cancel-from-inflight, and Cancel-of-unknown-id-returns-False. P1 #2 -- Broadcast work notifications to matching workers ========================================================== Reviewer: > With capability filtering, this auto-reset event wakes only one > arbitrary blocked worker per enqueue. If workers have disjoint > capabilities and the woken worker cannot serve the new request, > it goes back to sleep while the matching worker remains blocked > until another enqueue/register event or its poll timeout, so > runnable work can sit pending and eventually make the caller > time out. The wakeup needs to broadcast/re-signal enough waiters > or be per-capability/per-worker. Fix --- FNewWork switched from auto-reset to MANUAL-reset. SetEvent now unblocks ALL blocked DequeueForWorker waiters; each one re-checks under the lock and either takes a matching request or finds none. Reset-when-empty semantics keep the event correctly Cleared: - Enqueue (under lock): adds request, calls SetEvent. - DequeueForWorker (under lock): if no match for THIS worker and FPending.Count = 0 after the check, calls ResetEvent before releasing the lock and WaitFor-ing. The Reset + SetEvent ordering serialises on FLock, so there is no lost-wakeup race: any Enqueue that ran after our ResetEvent must hold the same lock we'll re-acquire on the next loop iteration -- the Enqueue's SetEvent and our next FLock.Acquire are ordered. Considered per-worker events; rejected because of TEvent lifetime race with UnregisterWorker (the freed event would be deref'd by an in-progress WaitFor on the dequeuer side). Manual-reset shared event is simpler and provably correct. P2 #3 -- Empty relay model treated as wildcard ============================================== Reviewer: > When the relay provider is configured without a model (the > catalog default), EffMod remains empty and the queue matches > that literal empty string against worker capabilities. Since > CanServe only treats an empty capability list as wildcard, > workers that advertise explicit models never receive these > requests, so the documented/default "whatever connected workers > advertise" mode hangs until timeout unless the worker omits > capabilities. Empty model should be matched as wildcard or > resolved to an advertised capability before enqueue. Status ------ Already addressed in the second commit on this PR (PR #318 head before this review): function TRelayWorker.CanServe(const Model: string): Boolean; ... if (Target = '') or (Length(FCapabilities) = 0) then Exit(True); The truth table: request='', worker=any caps -> match (caller said "any worker") request=X, worker=[] caps -> match (worker said "anything") request=X, worker=['X'] -> match (exact) request=X, worker=['Y'] -> miss (operator pinned a model no connected worker serves) Covered by TestEmptyRequestModelMatchesAnyWorker (added in that same commit). Flagging here so the reviewer sees the resolution. P2 #4 -- Populate tool calls from relay responses ================================================== Reviewer: > DecodeResponse zeroes the response and copies only content/usage/ > status, leaving TLLMResponse.ToolCalls empty for every successful > worker reply. RunToolLoop dispatches tools only from > Resp.ToolCalls, so a relay-backed agent will stop after a worker > emits a textual tool call instead of executing it, despite the V1 > relay contract saying worker replies are text-parsed for tool > calls. Parse the content here or add structured tool calls to the > relay response before returning success. Fix --- Accept structured tool_calls in the worker response JSON envelope (OpenAI shape: id / type / function.name / function.arguments), in preference to text-parsing. Workers using inference libraries that emit structured tool calls (WebLLM, mlc-llm, llama.cpp grammar mode, Transformers.js) populate the array; PasClaw forwards verbatim. - TRelayResponse gains ToolCalls: array of TToolCall. - HandleRelayRespond parses tool_calls[] from the POST body into Resp.ToolCalls. - TRelayProvider.DecodeResponse copies ToolCalls into TLLMResponse.ToolCalls. - RunToolLoop's existing dispatch handles them like any other structured tool call. Text-extraction is NOT implemented -- the V1 doc claim about "text- parsed" was overoptimistic. Workers that only emit text get text-only chat through the relay, same limitation V1 doc warned about for "smaller local models without strong tool support." Most modern small- model runtimes (WebLLM, mlc-llm, llama.cpp grammar mode) DO emit structured tool calls, so this floor is right for V1. Doc updates =========== - docs/providers-relay.md updated: response envelope shows the tool_calls array; the V1-scope bullet explains the structured-vs-text-only distinction; the WebLLM worker example forwards tool_calls from completion.choices[0].message.tool_calls. Tests ===== make clean && make passes. make test-relay-queue passes all 14 cases (was 11; added TestCancelPullsFromPending, TestCancelPullsFromInflight, TestCancelOfUnknownIsFalse).
4 tasks
FMXExpress
pushed a commit
that referenced
this pull request
Jun 22, 2026
Codex P2 review on PR #334. The Connect-time guard if (!_relayScopedToken && gwToken()) await refreshRelayScopedToken(); only re-fetched when the cache was EMPTY. TGatewayServer.Create regenerates FRelayToken on every startup, so if the gateway restarted (or its config reloaded) while the Relay tab stayed open, the stale cached token kept getting handed to the Worker -> the worker 401'd against the fresh server-side token -> the in-browser worker spun forever on the auto-retry loop until the operator reloaded the whole page. Fix: always call refreshRelayScopedToken() on Connect when a main token is set. The fetch is one tiny JSON GET; freshness matters more than the cache hit. Also re-runs syncRelayTokenInput + renderRelaySnippets so the visible token / displayed snippet bodies reflect the just-refreshed value (covers the case where the operator had "show" on with a stale token visible). Live smoke (gateway with main token MAINTOKEN, port 18911): boot #1 token: KYNB-G50W -> /v1/relay/status 200 restart boot #2 token: 76QZ-GGCD -> /v1/relay/status 200 stale KYNB-G50W against fresh #2 -> 401 (the failure mode) fresh 76QZ-GGCD against fresh #2 -> 200 (the fix path) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TBcLtmpj7dqA5tyFbGnQon
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes the PasClaw source tree compile under both FPC (
{$MODE DELPHI}viamake) and Delphi/RAD Studio 11/12 from the same.pasfiles. Five commits, branched from where PR #1 was merged (a8431b2).Open
src/pasclaw/PasClaw.dprojin RAD Studio → Build. No vendoring needed under Delphi (Indy ships with RAD Studio).What's in the box
src/pasclaw/PasClaw.dprojDelphi 12 multi-platform project file (Win32 / Win64 / Linux64). Every
src/pkg/*directory onDCC_UnitSearchPath; every unit listed underDCCReference.webui.rcregistered asRcCompilesobrcc32rebuilds the embedded resource on each build.src/pkg/json/PasClaw.JSON.pas— fullSystem.JSONbackendThe Delphi branch of the JSON wrapper (previously an empty stub) now implements
TJsonObject/TJsonArrayon top ofSystem.JSON.TJSONObject/TJSONArray. SameGetStr/GetInt/GetBool/PutObject/ChildObject/ItemObject/ToJSON/Parsesurface as the FPC backend. Memory model preserved (parents own children, wrappers don't).Migration of 9 call sites off
fpjsonConfig,Tools.FS,Tools.Shell,Skills.Loader,MCP.StdioClient,Gateway.Server,Channels.Telegram,Providers.OpenAI,Providers.Anthropic(including the SSE handler) — all routed throughPasClaw.JSON.grep -rl fpjson src/returns only the wrapper unit itself.src/pkg/platform/PasClaw.Platform.pas— cross-toolchain process I/OReplaces the previous Delphi-side stubs for
shell_exec, shell skills, and stdio MCP. Same interface, three back-ends selected at compile time:fcl-processCreatePipe+CreateProcessW+ReadFile/WriteFile+PeekNamedPipe+GetExitCodeProcess+TerminateProcess,CREATE_NO_WINDOWpipe+fork+dup2+execvp+select(10 ms) +waitpid(WNOHANG)+SIGTERMTools.Shell.RunShell,Skills.Loader.RunShell, andMCP.StdioClient(spawn / WriteLine / ReadLine) are now one-liners that delegate to the abstraction. No more per-file{$IFDEF FPC}branches around process I/O.Portable directives across all 51 units
{$MODE DELPHI}wrapped as{$IFDEF FPC}{$MODE DELPHI}{$ENDIF}so Delphi (which doesn't have$MODE) doesn't warn{$I %PASCLAW_VERSION%}/{$I %FPCVERSION%}/{$I %FPCTARGETOS%}/{$I %FPCTARGETCPU%}wrapped — DelphiFormatBuildInforeturnspasclaw <version> (delphi)BaseUnix(chmod),cthreads/cmem(Indy threading), ioctlTIOCGWINSZ(terminal width) all FPC-only; Delphi POSIX usesPosix.SysStat.chmod; Delphi Windows + TUI fall back to 80-col widthTest plan
make) clean — 268k lines, 0 errorsmake smoke— all 11 commands OKfs_readend-to-end)fakemcp.pyservershell_execrunsuname -m && echo okthrough the tool loopPasClaw.dprojin Delphi 11/12 — Build for Win64PasClaw.dprojin Delphi 11/12 — Build for Linux64The Delphi-side code can't be exercised in the FPC dev environment but follows canonical Win32 + POSIX API patterns; first-build issues are most likely unit-naming (e.g.
Posix.Base) and easy to fix.Commits
ccbfd23chunk 1/4:.dproj+ simple FPC-only guards (cthreads,BaseUnix, ioctl)aabb67achunk 2/4:System.JSONbackend forPasClaw.JSONff86249chunk 3/4: migrate allfpjsoncall-sites to the wrapper16a0ef1chunk 4/4: portable directives across all units1da9427PasClaw.Platformcross-toolchain process I/O (removes the three stubs)Generated by Claude Code