When the ACP process is started via a restricted runner (r != nil), c.cmd is nil, so Close() never terminates the underlying process; it only calls wait(), which may block until the process exits on its own. Store and invoke a cancellation/kill mechanism for both execution paths (e.g., create a derived context with cancel inside NewConnection, store cancel on Connection, call it in Close(), then call wait()), so restricted-runner processes are reliably stopped.

The filter claims to pass through “valid JSON-RPC messages”, but it currently passes any non-empty line whose first non-space character is '{', even if the line is not valid JSON. That can still break the downstream JSON-RPC decoder. Consider using json.Valid(trimmed) (or attempting a cheap unmarshal into map[string]any) before allowing the line through; otherwise, update the docstring to match the weaker guarantee.

Creating ~/.mittorc with mode 0644 makes the config world-readable on many systems, which is risky once users add credentials (e.g., auth passwords). Use a more restrictive default like 0600 (and consider preserving existing perms on overwrite).

This test case claims to cover a trailing-slash path, but workspaceRoot is identical to the non-trailing-slash case. Update it to actually include a trailing slash (e.g., .../myproject/) so it tests the intended behavior.

Mermaid extension configuration is duplicated in both WithHighlighting and NewConverter. This increases the risk of the two snippets drifting (or enabling Mermaid unexpectedly when only highlighting is desired). Consider centralizing Mermaid enabling in one place (typically NewConverter), and keep WithHighlighting focused on highlighting.

ArchiveRetentionPeriod has a declared set of valid values, but GetArchiveRetentionPeriod() returns arbitrary configured strings without validation. Consider adding a ValidateArchiveRetentionPeriod() (similar to ValidateRestrictedRunner()) during settings/config load so invalid values fail fast with a clear error.

New validation logic for restricted_runner is introduced here, but there’s no corresponding unit test shown for accepted values (empty/default + each valid type) and rejection of an invalid type. Adding table-driven tests in the internal/config package would help prevent regressions as runner types evolve.

Building file:// URLs via string concatenation can produce invalid URIs for paths containing spaces or other reserved characters, and can be platform-sensitive (notably Windows paths). Consider constructing the URL with net/url (including proper escaping) before passing it into ResourceLinkBlock.

The function comment says access logging is “disabled by default for CLI usage”, but the implementation always starts from web.DefaultAccessLogConfig(). If that default ever includes a non-empty path, CLI runs could unexpectedly enable access logging. Consider initializing with web.AccessLogConfig{} (disabled), and only applying defaults when the CLI flag or config explicitly enables the feature.

The function comment says access logging is “disabled by default for CLI usage”, but the implementation always starts from web.DefaultAccessLogConfig(). If that default ever includes a non-empty path, CLI runs could unexpectedly enable access logging. Consider initializing with web.AccessLogConfig{} (disabled), and only applying defaults when the CLI flag or config explicitly enables the feature.