chore: granular lint and format fix

comments_3647.txt

@@ -0,0 +1,47 @@
+FILE: src/agents/session-transcript-repair.ts
+LINE: 62
+BODY: The comment on line 60 is questioning rather than explanatory. Comments with questions like "Deep clone to avoid mutating the input unless necessary?" and "Actually, we can mutate in place effectively or clone messages that need change" suggest uncertainty or incomplete decision-making. The implementation does create a new array and new message objects when changes are detected (lines 63, 107-110), so the comment should clearly state the approach taken: "Creates new message objects only when sanitization is needed; otherwise returns the original messages to avoid unnecessary copying."
+```suggestion
+  // Creates new message objects only when sanitization is needed; otherwise
+  // returns the original messages to avoid unnecessary copying, while guarding
+  // against corrupt JSON in tool arguments that could break the session.
+```
+---
+FILE: src/agents/session-transcript-repair.ts
+LINE: 86
+BODY: The function validates and discards valid JSON strings but doesn't parse them. When `input` is a string containing valid JSON (line 85), the code simply pushes the block unchanged. However, if `input` should always be an object (not a string), then valid JSON strings should also be parsed and converted to objects for consistency. Clarify the expected behavior: should valid JSON strings be preserved as-is, or should they also be parsed into objects? The current behavior creates inconsistency where some inputs are objects and some are JSON strings.
+```suggestion
+            const parsedInput = JSON.parse((block as any).input);
+            nextContent.push({
+              ...block,
+              input: parsedInput,
+            });
+            msgChanged = true;
+```
+---
+FILE: src/agents/session-transcript-repair.ts
+LINE: 118
+BODY: The function doesn't return any report or metadata about what was sanitized, unlike `repairToolUseResultPairing` which returns a `ToolUseRepairReport` with details about changes made. Consider returning a report object that includes information such as the number of sanitized blocks and which tool types were affected. This would help with debugging, monitoring, and understanding the extent of data corruption in sessions. Alternatively, add inline logging to track sanitization events.
+---
+FILE: src/agents/session-transcript-repair.ts
+LINE: 118
+BODY: The `sanitizeToolUseArgs` function lacks test coverage. The existing test file only tests `sanitizeToolUseResultPairing`, but does not directly test the new `sanitizeToolUseArgs` function. Add tests that verify:
+1. Valid JSON strings in `input` fields are preserved
+2. Invalid JSON strings in `input` fields are replaced with `{}`
+3. Object values in `input` fields are preserved
+4. The `_sanitized` and `_originalInput` metadata fields are correctly set when sanitization occurs
+5. Messages without tool blocks are left unchanged
+---
+FILE: src/agents/session-transcript-repair.ts
+LINE: 97
+BODY: Silent data loss occurs when malformed JSON is replaced with an empty object. When the sanitization logic detects and replaces invalid JSON in tool arguments, it stores the original input in `_originalInput` but doesn't log this potentially important information. Consider adding a warning log similar to patterns found elsewhere in the codebase (e.g., in `compaction.ts`, `model-catalog.ts`) to aid debugging when sessions are repaired. The log should include the tool name and a sample of the malformed input to help trace the root cause of the corruption.
+---
+FILE: src/agents/session-transcript-repair.ts
+LINE: 100
+BODY: The function only sanitizes the `input` field for tool use blocks, but the existing tests use `arguments` as the field name (see lines 11-12, 38, 65 in session-transcript-repair.test.ts). This suggests there may be inconsistency in the field naming across different tool block types. Verify whether tool blocks can use both `input` and `arguments` field names, and if so, ensure the sanitization logic handles both. If they are mutually exclusive based on the block type, add a comment explaining this distinction.
+---
+FILE: src/agents/session-transcript-repair.ts
+LINE: 94
+BODY: Using `(block as any).input` bypasses TypeScript's type checking. The code would be more maintainable and safer with proper type guards or typed access. Consider defining a proper type for tool blocks or using a type guard function to check for the presence of the `input` field before accessing it. This approach would make the code more self-documenting and catch potential issues at compile time.
+---
+

comments_3648.txt

@@ -0,0 +1,47 @@
+FILE: src/agents/system-prompt.ts
+LINE: 412
+BODY: The Windows shell guidance feature lacks test coverage. Consider adding a test case in `system-prompt.test.ts` to verify that the Windows-specific guidance is included when `runtimeInfo.os` contains "windows" and excluded otherwise. This would ensure the feature works as expected and prevent regressions.
+
+For example, you could add a test like:
+- Test that when runtimeInfo.os includes "Windows_NT", the prompt contains "## Windows Shell Guidance"
+- Test that when runtimeInfo.os is "Darwin" or "Linux", this section is not included
+---
+FILE: src/agents/system-prompt.ts
+LINE: 100
+BODY: This PR includes extensive indentation/formatting changes throughout the file that are not mentioned in the PR description. While these changes appear to be consistent reformatting (switching from multi-line template literals to array-based formatting), they make it harder to review the actual functional change (the Windows guidance addition). Consider:
+
+1. Splitting formatting changes into a separate commit/PR, OR
+2. Explicitly mentioning these formatting changes in the PR description
+
+This follows best practices for keeping functional changes separate from style/formatting changes for easier code review and git history.
+---
+FILE: src/agents/system-prompt.ts
+LINE: 407
+BODY: The Windows guidance warns against using Unix commands like `grep`, but the system prompt (lines 340-342) also lists `grep`, `find`, and `ls` as Pi's standard tools. This could be confusing because:
+
+1. Pi provides `grep`, `find`, and `ls` as built-in tools that work cross-platform
+2. The Windows guidance seems to warn against using these names in shell commands via the `exec` tool
+
+Consider clarifying this distinction in the Windows guidance. For example: "- Do NOT use Unix commands like `grep`, `sed`, `awk`, `head`, `tail` in exec/shell commands unless you are sure they are installed (Pi's built-in grep/find/ls tools work fine)."
+
+This makes it clear that the warning is specifically about shell commands, not Pi's built-in tools.
+```suggestion
+        "- Do NOT use Unix commands like `grep`, `sed`, `awk`, `head`, `tail` in exec/shell (PowerShell) commands unless you are sure they are installed. Pi's built-in tools named `grep`, `find`, and `ls` are safe to use.",
+```
+---
+FILE: .github/PULL_REQUEST_TEMPLATE.md
+LINE: 315
+BODY: The PR includes changes to `.github/PULL_REQUEST_TEMPLATE.md` which is unrelated to adding Windows PowerShell support. The diff shows the entire file (315 lines) as new additions, which suggests either:
+
+1. The file was regenerated/reformatted
+2. Line ending changes (CRLF Γåö LF)
+3. The file was accidentally included in this PR
+
+Since this is unrelated to the stated PR purpose ("add windows powershell support to system prompt"), consider:
+- Removing this file from the PR if it was accidentally included
+- If intentional, explain the reason in the PR description
+- If it's a line ending change, consider using `.gitattributes` to enforce consistent line endings
+
+Including unrelated changes makes code review more difficult and complicates the git history.
+---
+

feedback_3647.txt

@@ -0,0 +1,20 @@
+--- PR 3647 --
+REVIEWER: copilot-pull-request-reviewer
+BODY: ## Pull request overview
+
+This PR implements sanitization for tool arguments in session history to prevent crashes caused by malformed JSON in tool input fields. When invalid JSON is detected in tool use blocks, it is replaced with an empty object `{}` to prevent `InternalError.Algo.InvalidParameter` errors that would otherwise crash the entire session.
+
+**Changes:**
+- Added `sanitizeToolUseArgs` function to detect and repair malformed JSON in tool input arguments
+- Integrated sanitization into the `sanitizeToolUseResultPairing` pipeline to ensure it runs before tool result pairing repair
+- Preserved original malformed input in `_originalInput` metadata field for debugging
+
+
+
+
+
+---
+
+💡 <a href="/moltbot/moltbot/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
+
+

feedback_3648.txt

@@ -0,0 +1,29 @@
+--- PR 3648 --
+REVIEWER: copilot-pull-request-reviewer
+BODY: ## Pull request overview
+
+This PR adds Windows-specific shell guidance to the agent's system prompt to help it use PowerShell syntax and commands when running on Windows. The implementation detects Windows via the runtime OS information and injects appropriate guidance about PowerShell syntax, environment variables, and command alternatives.
+
+**Changes:**
+- Added conditional Windows Shell Guidance section to system prompt when running on Windows
+- Reformatted multiple sections of system-prompt.ts from template literals to array-based formatting
+- Modified .github/PULL_REQUEST_TEMPLATE.md (appears unrelated to main feature)
+
+### Reviewed changes
+
+Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
+
+| File | Description |
+| ---- | ----------- |
+| src/agents/system-prompt.ts | Added Windows PowerShell guidance section (lines 402-412) with conditional logic based on OS detection; includes extensive indentation/formatting changes throughout the file |
+| .github/PULL_REQUEST_TEMPLATE.md | Entire file appears as new additions (315 lines), likely due to formatting or line ending changes; unrelated to Windows PowerShell feature |
+
+
+
+
+
+---
+
+💡 <a href="/moltbot/moltbot/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
+
+

src/agents/system-prompt.ts

@@ -421,7 +421,8 @@ export function buildAgentSystemPrompt(params: {
             ? `Sandbox workspace: ${params.sandboxInfo.workspaceDir}`
             : "",
           params.sandboxInfo.workspaceAccess
-          ? `Agent workspace access: ${params.sandboxInfo.workspaceAccess}${params.sandboxInfo.agentWorkspaceMount
+            ? `Agent workspace access: ${params.sandboxInfo.workspaceAccess}${
+                params.sandboxInfo.agentWorkspaceMount
                   ? ` (mounted at ${params.sandboxInfo.agentWorkspaceMount})`
                   : ""
               }`