From 80f86c08faa824d9ec62bdd75220cbf50b8cee20 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Fri, 21 Aug 2026 15:40:18 +0200 Subject: [PATCH] docs(ticket): File five work items Five findings from a stretch of tooling work, filed rather than fixed in place so each can be judged on its own. Two concern the cargo tools: unknown `options` keys are dropped silently, so a tool configured with an option the installed binary predates reports success about the wrong thing; and `cargo_check` hardcodes `--all-targets --all-features`, which is right for this workspace but expensive when the tool is pointed elsewhere. One is missing coverage. The fs walker supports a nested `.ignore` that re-includes a subtree its parent excluded, and PR #727 rewrote that path without a test to catch a regression. One is a deferred RFD phase: a hand-authored `external = true` access rule is dropped rather than prompting for approval, so the config-only path cannot work without an unrelated `--mount` first. The last records a problem rather than a solution. A workspace cannot inherit another's config, so anything shared has to be copied and left to drift. The direction is sketched and the open questions listed; it should become an RFD before anyone implements it. Signed-off-by: Jean Mertz --- ...s-silently-ignores-unknown-tool-options.md | 60 ++++++++++++ ...rove-external-access-rules-on-first-use.md | 91 +++++++++++++++++++ ...let-cargo_check-narrow-its-clippy-scope.md | 32 +++++++ ...ore-re-inclusion-in-the-fs-walker-tests.md | 45 +++++++++ ...571g2h-inherit-config-across-workspaces.md | 59 ++++++++++++ 5 files changed, 287 insertions(+) create mode 100644 docs/ticket/04kss8j-jp-tools-silently-ignores-unknown-tool-options.md create mode 100644 docs/ticket/04p8zjb-prompt-to-approve-external-access-rules-on-first-use.md create mode 100644 docs/ticket/0571cdm-let-cargo_check-narrow-its-clippy-scope.md create mode 100644 docs/ticket/0571cwr-cover-nested-ignore-re-inclusion-in-the-fs-walker-tests.md create mode 100644 docs/ticket/0571g2h-inherit-config-across-workspaces.md diff --git a/docs/ticket/04kss8j-jp-tools-silently-ignores-unknown-tool-options.md b/docs/ticket/04kss8j-jp-tools-silently-ignores-unknown-tool-options.md new file mode 100644 index 000000000..c82d6160e --- /dev/null +++ b/docs/ticket/04kss8j-jp-tools-silently-ignores-unknown-tool-options.md @@ -0,0 +1,60 @@ +# jp-tools silently ignores unknown tool options + +- **Status**: Todo +- **Kind**: Bug +- **Authors**: jp +- **Date**: 2026-08-18 + +`jp-tools` reads per-tool settings from a free-form `options` map and drops +anything it does not recognise. +`Tool::option_or` returns the default both when the key is missing and when the +value fails to deserialize, so an option the running binary does not implement +is indistinguishable from one that was never set. + +The failure that surfaced this: `cargo_check` and `cargo_test` were configured +with `options.root` pointing at a second cargo workspace, but the installed +binary predated that option. +Both ran against the default workspace and reported success. +The results were green, plausible, and about the wrong code. +The only thing that caught it was recognising the test count from an earlier +run. + +## Why this is worse than an ordinary silent default + +A verification tool's whole job is to answer "is this code correct?". +When it silently answers about *different* code, every conclusion drawn from it +is wrong, and nothing in the output hints at it. + +The skew is easy to hit: tool config lives in the repository and updates on +pull, while the binary updates only on `just install-tools`. + +## Existing precedent + +`web_fetch` already parses its settings into a typed `WebFetchOptions` +(`.config/jp/tools/src/web/fetch/options.rs`) rather than reaching into the map +directly. +Generalising that pattern is most of the work. + +## Directions to investigate + +- Give each tool or tool group a typed options struct, deserialized once at + dispatch, replacing ad-hoc `option_or` lookups. +- `#[serde(deny_unknown_fields)]`, so a typo or an option meant for a newer + binary is a loud error rather than a no-op. +- Decide the failure mode deliberately: reject the call, or run and report the + ignored keys. + Silence should not be one of the choices. +- Have tools state the effective setting they acted on (for the cargo tools, the + workspace root), so a misdirect is visible in the result itself. + This is the cheapest mitigation and is independent of the typed-options work. +- Consider whether tool configs should declare a minimum binary version, since + typed options alone still cannot distinguish "unknown" from "not yet + implemented" in a useful message. + +## Scope + +`jp_config` forwards unknown options untouched by design, which is correct: it +cannot know each tool's schema. +The gap is that the receiving end discards them just as quietly. +The fix belongs in `.config/jp/tools`, which is developer tooling rather than +shipped surface. diff --git a/docs/ticket/04p8zjb-prompt-to-approve-external-access-rules-on-first-use.md b/docs/ticket/04p8zjb-prompt-to-approve-external-access-rules-on-first-use.md new file mode 100644 index 000000000..39edae403 --- /dev/null +++ b/docs/ticket/04p8zjb-prompt-to-approve-external-access-rules-on-first-use.md @@ -0,0 +1,91 @@ +# Prompt to approve external access rules on first use + +- **Status**: Todo +- **Kind**: Feature +- **Authors**: jp +- **Date**: 2026-08-18 + +Finish the deferred half of the approval lifecycle in RFD D43 Phase 3: a +hand-authored `external = true` access rule should prompt for approval on first +use instead of being dropped. + +## Current behaviour + +`compile_tool_policy` consults the approval store for every external rule and +maps both outcomes to a refusal: + +```rust +ApprovalLookup::Approved => ApprovalDecision::Approved, +ApprovalLookup::Retargeted { .. } | ApprovalLookup::Unknown => ApprovalDecision::Rejected, +``` + +An unapproved rule is dropped with a warning, and `compile_policy` inserts a +deny-all sentinel so the tool stays default-deny rather than degrading to +unrestricted workspace access. +Nothing is silently granted, so this is safe — just unusable without a +bootstrap step. + +The only way to seed an approval today is `--mount`, which creates a symlink and +records the binding as a side effect. +So a config-declared external rule cannot work until the user has run a +`--mount` invocation for that same rule path. + +## Why it matters + +It inverts the intended authoring model. +The config is meant to declare intent and JP is meant to ask about the risky +part once. +Instead the CLI flag is load-bearing, and the config-only path silently does +nothing (bar a warning). + +The concrete case: a second project developed inside a JP workspace keeps its +own JP config in an ignored directory, and wants a rule granting one tool access +to a file outside the workspace. +Declaring it is natural; having to remember an unrelated `--mount` invocation +first is not. + +## What D43 already specifies + +The design is settled, so this is implementation rather than a new decision: + +- Approval is **target-only**. + The user approves that a workspace-relative rule path may resolve to a + specific canonical absolute target. + Capability edits to the same rule (adding write, say) do **not** re-prompt + while the target is unchanged; those are visible in git diff and `jp config + show`. + Silent retargeting is the threat trust-on-first-use exists to catch. +- The prompt is **host-side** and uses the terminal prompting UI, but is + deliberately **not** recorded as `InquiryRequest` / `InquiryResponse` events + in the conversation stream: the prompt text contains a canonical host path, + which must not enter shared conversation state. + The durable record is the user-local approval store only. +- Retargeting is a distinct case from unknown, and the store already + distinguishes them (`ApprovalLookup::Retargeted { previous }`), so the prompt + can say what the binding used to point at. + +## Scope + +- `crates/jp_cli/src/access/compile.rs` — `compile_tool_policy` needs an + approver that can prompt, rather than the current pure store lookup. + Note `compile_fs` already takes `approve: impl FnMut(&str, &Utf8Path) -> + ApprovalDecision`, so the seam exists. +- `crates/jp_cli/src/access/approvals.rs` — `record` and `save` already exist; + the prompt path needs to persist on approval. +- Decide the non-interactive behaviour: a prompt is impossible under + `--no-interactive` or when detached, and the answer should be the current one + (drop with a warning) rather than blocking or auto-approving. + +## Out of scope + +The other unimplemented D43 phases: OS-level sandboxing (Phase 4, RFD 075), +Windows junction fallback (Phase 6), `--no-mount` cleanup, and the broad-mount +tool-scope confirmation prompt. + +## Note on RFD status + +D43 is still a Draft, so `implements` is deliberately unset — that field is for +phases of an accepted RFD's plan, and setting it would put a draft in the In +Progress column. +Phases 1, 2 and 5 of D43 shipped in PR \#727 while it was a draft, so filing +this against the draft matches how the RFD has actually been built out. diff --git a/docs/ticket/0571cdm-let-cargo_check-narrow-its-clippy-scope.md b/docs/ticket/0571cdm-let-cargo_check-narrow-its-clippy-scope.md new file mode 100644 index 000000000..528d81b47 --- /dev/null +++ b/docs/ticket/0571cdm-let-cargo_check-narrow-its-clippy-scope.md @@ -0,0 +1,32 @@ +# Let cargo\_check narrow its clippy scope + +- **Status**: Todo +- **Kind**: Feature +- **Authors**: jp +- **Date**: 2026-08-19 + +`cargo_check` hardcodes `--all-targets --all-features` +(`.config/jp/tools/src/cargo/check.rs`). +That matches `just lint-ci` and is the right default for this workspace. + +It becomes expensive when the tool is pointed at a different cargo workspace via +`options.root`: a dependency-heavy project turns a routine check into a large +compile, which is the opposite of what the inner loop needs. + +## Suggested change + +A tool option to drop `--all-features`, `--all-targets`, or both. +Default unchanged. + +## Related + +`check.rs` and `format.rs` also run `comfort` against the same root, so a second +workspace inherits this workspace's doc-comment conventions. +That is defensible (one formatter, one style), but if it proves annoying it +wants its own opt-out rather than being bundled into the flag above. + +## Priority + +Speculative. +File-and-forget until someone actually feels the cost; do not add knobs +pre-emptively. diff --git a/docs/ticket/0571cwr-cover-nested-ignore-re-inclusion-in-the-fs-walker-tests.md b/docs/ticket/0571cwr-cover-nested-ignore-re-inclusion-in-the-fs-walker-tests.md new file mode 100644 index 000000000..0a4c0b00a --- /dev/null +++ b/docs/ticket/0571cwr-cover-nested-ignore-re-inclusion-in-the-fs-walker-tests.md @@ -0,0 +1,45 @@ +# Cover nested .ignore re-inclusion in the fs walker tests + +- **Status**: Todo +- **Kind**: Chore +- **Authors**: jp +- **Date**: 2026-08-19 + +The fs tools support a two-level ignore arrangement that nothing tests: a root +`.ignore` excludes a subtree, and a `.ignore` *inside* that subtree re-includes +it with `!**`. + +The result is that an unscoped walk from the workspace root does not descend +into the subtree, while a walk scoped to that subtree (or deeper) sees it in +full. +That asymmetry is useful — it keeps a large, irrelevant directory out of broad +searches without making it unreachable — but it is entirely emergent. + +## Why it works + +`WalkBuilder` in `.config/jp/tools/src/fs/list_files.rs` is built with +`standard_filters(false)`, `.ignore(true)`, `.parents(true)`. +Only `.ignore` files are consulted; `.gitignore` and `.git/info/exclude` are +not. +On a broad walk the root `.ignore` prunes before descent, so the nested file is +never read. +On a scoped walk the nested file is picked up through `parents(true)` and its +negation applies. + +## Why it needs a test + +PR \#727 rewrote this code path (`walk_spec` now "scopes the workspace walk with +a path filter rather than re-rooting") and the behaviour survived, but nothing +would have caught it if it hadn't. +The failure mode is silent: searches quietly stop seeing a directory, and the +tool reports success with fewer results. + +## Suggested test + +In `.config/jp/tools/src/fs/list_files_tests.rs`: build a temp tree with a root +`.ignore` excluding `sub/`, a `sub/.ignore` containing `!**`, and a file at +`sub/file.rs`. +Assert an unscoped listing omits it and a listing with prefix `sub` includes it. + +Worth a matching case in `grep_files_tests.rs`, which delegates to the same +walker for directory targets. diff --git a/docs/ticket/0571g2h-inherit-config-across-workspaces.md b/docs/ticket/0571g2h-inherit-config-across-workspaces.md new file mode 100644 index 000000000..73eb4df4d --- /dev/null +++ b/docs/ticket/0571g2h-inherit-config-across-workspaces.md @@ -0,0 +1,59 @@ +# Inherit config across workspaces + +- **Status**: Todo +- **Kind**: Feature +- **Authors**: jp +- **Date**: 2026-08-19 + +A workspace cannot reuse another workspace's configuration. +This ticket records the problem and a rough direction; the design is worth +arguing properly, so it should be promoted to an RFD before implementation. + +## Problem + +The moment a directory gains its own `.jp/`, `find_root` stops there and it is a +standalone workspace. +It inherits nothing from the workspace it sits inside, or from any other +workspace. + +So a second workspace needs its own copies of everything worth sharing: tool +configuration, personas, skills, knowledge sections, model definitions, render +style. +Copies drift, and the drift is silent — the second workspace simply behaves +like an older version of the first. + +This is separate from [RFD 035] (multi-root *load path* resolution, which is +about where `--cfg` looks for named entries within one workspace) and from [RFD +D38] (which is about anchoring a single `extends` path at a named config root). +Neither addresses one workspace inheriting another's config as a whole. + +## Rough direction + +An `extends`-like mechanism that crosses the workspace boundary, so a +workspace's `.jp/config.toml` can declare "start from that workspace's config, +then override". +Open questions that make this RFD-shaped rather than ticket-shaped: + +- What is the unit of inheritance? + Whole config, or named entries only? +- How is the parent identified — path, workspace id, or something stable across + machines? +- What happens when the parent is unavailable (moved, deleted, different + machine)? + Hard error, or degrade with a warning? +- Does inheritance compose transitively, and how are cycles handled? +- How does this interact with the four implicit sources and their precedence + ([RFD 079])? + +## Related + +- [RFD D38] — root-qualified extends paths. + Solves the narrow case of addressing one file in another root; this ticket is + the general case. +- [RFD 046] — nested workspace projection. + +## Next step + +Promote to an RFD. +The problem is real and recurring, but the answer is a design argument, not an +implementation.