Trim Pi ambient skill prompt

[glittercowboy]

Summary

• pass --no-skills on normal Pi invocations to prevent configured/global skills from entering every cloud chat prompt
• keep explicit skill paths available by still forwarding --skill flags
• update session executor tests to lock in the intended args

Validation

• go test ./internal/pi ./internal/session
• go test ./...
• Live local Claude probe on claude-haiku-4-5-20251001: provider system prompt 123,917 chars -> 58,000 chars; input-side/cache-write about 52.4k tokens -> 26.8k tokens; response still succeeded with full provider tool profile

Summary by CodeRabbit

• Bug Fixes
  • Fixed skills configuration so the application consistently applies the "no skills" option and only includes individual skill selections when skills are enabled, ensuring command-line behavior matches user preference.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 31c1dd17-e7aa-48fb-b651-090d7abc38ab

📥 Commits

Reviewing files that changed from the base of the PR and between de74381 and 1c9a495.

📒 Files selected for processing (2)

• internal/pi/executor.go
• internal/session/actor_test.go

---

📝 Walkthrough

Walkthrough

processArgs for the PI executor was changed to always emit --no-skills and to add --skill flags only when skills are enabled. Tests were updated to reflect the new presence/absence expectations for --no-skills in generated CLI args.

Changes

Cohort / File(s)	Summary
Skill Flag Logic Refactor internal/pi/executor.go	Changed processArgs to unconditionally include --no-skills and to append --skill entries only when DisableSkills is false; removed previous mutually exclusive branches.
Test Assertions Update internal/session/actor_test.go	Adjusted tests to match new flag behavior: TestActorPiExecutorUsesPersistentSessionFile now expects --no-skills to be absent (fails if present, error message changed to "missing --no-skills"), while TestActorPiExecutorPassesReferencedClaudeSkillPaths now asserts that --no-skills is present when referenced skill paths are used.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Honor disabled skills tasks #101: Modifies DisableSkills/PI CLI flag emission in the same processArgs function — directly related to this change.
• Enable Pi skill discovery #77: Adjusts handling of --no-skills vs skill discovery in the PI executor — potentially conflicting with this PR.
• Pass Claude skills through Pi #79: Adds/changes emission of --skill flags from SkillPaths in processArgs — overlaps with skill-flag construction logic here.

Poem

🐰 I nibble flags beneath the moonlit code,
--no-skills first, a tiny, steady node.
Then if skills wake, I scatter --skill seeds,
Hopping through args to do small useful deeds.
Hooray for tidy flags and tested meads!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Trim Pi ambient skill prompt' is vague and does not clearly convey the main technical change of unconditionally adding --no-skills flags to prevent configured/global skills from entering cloud chat prompts.	Consider using a more specific title like 'Disable configured skills in Pi invocations' or 'Add --no-skills flag to prevent ambient skills in cloud prompts' to better reflect the primary change.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/normal-prompt-trim

---

_{Review rate limit: 7/10 reviews remaining, refill in 16 minutes and 16 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

internal/session/actor_test.go (1)

525-527: 💤 Low value

Remove duplicate assertion.

This check is redundant—lines 519-521 already verify --no-skills is present with the same condition and error message.

🧹 Suggested fix

 	if skillPaths := argValues(args, "--skill"); len(skillPaths) != 0 {
 		t.Fatalf("pi skill paths = %+v, want none", skillPaths)
 	}
-	if argIndex(args, "--no-skills") < 0 {
-		t.Fatalf("pi args missing --no-skills: %v", args)
-	}
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/session/actor_test.go` around lines 525 - 527, Remove the redundant
assertion that checks for "--no-skills" using argIndex(args, "--no-skills") < 0
in the test (duplicate of the earlier check around the same test function);
delete the later if-block (the t.Fatalf call) so only the original assertion
remains and the test no longer duplicates the same condition.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@internal/session/actor_test.go`:
- Around line 525-527: Remove the redundant assertion that checks for
"--no-skills" using argIndex(args, "--no-skills") < 0 in the test (duplicate of
the earlier check around the same test function); delete the later if-block (the
t.Fatalf call) so only the original assertion remains and the test no longer
duplicates the same condition.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f5e93c8a-dd09-4bc3-b9d1-bcb2fc94069f

📥 Commits

Reviewing files that changed from the base of the PR and between 67fd1d4 and de74381.

📒 Files selected for processing (2)

• internal/pi/executor.go
• internal/session/actor_test.go