The reviewer identified two bugs in the original PR:
1. PATCH /api/config leaves session.dimensions stale: LoadConfig()
derives dimensions from the old dm_scope, and the merge carries
those stale dimensions forward. ApplyDmScope() then exits early
because dimensions is already populated, causing a mismatch between
dm_scope (new) and dimensions (old).
2. Legacy/default configs omit dm_scope in GET response: configs with
explicit dimensions but no dm_scope (including DefaultConfig) return
no dm_scope field, causing the frontend to fall back to its default
('per-channel-peer'), which may not match the actual dimensions.
Fix:
- Add DeriveDmScope() to reverse-map known dimensions arrays to
dm_scope when dm_scope is empty.
- Call it in LoadConfig(), PUT handler, PATCH handler, and
ResetToDefaults() for consistent normalization.
- In PATCH handler, clear stale dimensions from the merge result when
the patch contains session.dm_scope but not session.dimensions,
allowing ApplyDmScope() to re-derive from the new scope.
- Add comprehensive unit tests for DeriveDmScope() and scope
transition scenarios.
* fix(launcher): hide console flashes in all Windows child processes
PR #2654 only applied HideWindow to child processes in gateway.go (powershell, tasklist, ps). Several other files still use exec.Command directly, causing visible console windows on Windows.
- startup.go: reg query/add/delete for autostart registry
- version.go: picoclaw version subcommand
- runtime.go: rundll32 for browser launch
- onboard.go: picoclaw onboard subcommand
Add launcherExecCommand to the utils package (matching the api package pattern) and replace all bare exec.Command calls on Windows paths.
* refactor: consolidate launcherExecCommand into utils package
Export LauncherExecCommand and ApplyLauncherProcAttrs from the utils
package as the single source of truth. The api package now imports
and delegates to these exported functions, eliminating code duplication.
Addresses review feedback from imguoguo on PR #3061.
short_retrieval.go: Check Atoi error even though regex ensures numeric input. gateway.go: Log warning when gateway config JSON is malformed instead of silently using defaults.
RFC 2544 benchmark addresses (198.18.0.0/15) are not globally routable
but were missing from the isPrivateOrRestrictedIP blocklist, allowing
SSRF bypasses via literal IPv4.
Fixes#3077
The dm_scope field was stored in config but never translated into the
dimensions array that the routing layer actually consumes. This meant
changing the session isolation scope in the UI had no effect at runtime.
Add ApplyDmScope() to SessionConfig which maps the user-facing dm_scope
values (per-channel-peer, per-channel, per-peer, global) to the
corresponding dimension arrays. Call it in LoadConfig post-processing
and in both the PATCH and PUT API handlers.
Includes table-driven tests covering all dm_scope values and the
precedence rule (explicit dimensions > derived from dm_scope).
The frontend sends dm_scope as part of the session config, but the
backend SessionConfig struct lacked the corresponding field. Go's
encoding/json silently discards unknown fields, so the value was lost
on every PATCH request. Additionally, MarshalJSON only emitted the
session block when Dimensions or IdentityLinks were set, so even a
stored dm_scope would not appear in GET responses.
- Add DmScope string field with json tag 'dm_scope' to SessionConfig
- Update MarshalJSON condition to include session when DmScope is set
Replace raw log.Printf and fmt.Printf calls in pkg/state, pkg/agent, and pkg/tools with structured logger calls (WarnCF/InfoCF). This ensures warnings and info messages are routed through the configured logging infrastructure instead of raw stderr/stdout.
errutil.go: Change %v to %w in ClassifySendError and ClassifyNetError so callers can use errors.Is/errors.As on the underlying HTTP/network error.
isolated_command_transport.go: Change %v to %w in Close() and Write() error paths for the same reason.
GetStartupInfo returns map[string]any, and type-asserting tools/skills entries without checking ok is fragile. While the current implementation always stores the correct types, a future refactor could cause silent nil dereference. Add ok checks with explicit nil fallback.
When os.Getwd fails, wd is empty and builtinSkillsDir resolves to relative path, causing confusing downstream errors. Fall back to config.GetHome on error.
singleflight.Group.Do() returns any, which is type-asserted as bool
without an ok check at model_status.go:211. If a non-bool value is
returned (e.g. nil from shared/cache corruption), this panics.
Add ok check and return false (model probe failed) as a safe default.
Mauro