Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ fn main() {

let mut files = Vec::new();
collect_files(&prompts_dir, &prompts_dir, &mut files).unwrap();
// The Sashiko README documents source-checkout usage. It is not runtime
// guidance and must not advertise candidate-relative paths from the
// installed trusted profile. Existing profile documentation is unchanged.
files.retain(|(relative, _)| relative != "sashiko/README.md");
files.sort_by(|a, b| a.0.cmp(&b.0));

let revision = fs::read_to_string(prompts_dir.join("REVISION"))
Expand Down
121 changes: 121 additions & 0 deletions designs/DESIGN_SASHIKO_REVIEW_PROFILE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
# Design: Sashiko Review Profile

## Goal

Add a bundled prompt profile for reviewing Sashiko's Rust code. The existing
local review command can select it directly, and daemon prompt-directory
selection can use the same profile when that separate plumbing is available.
The profile should improve review of Sashiko-specific failure modes without
changing the Linux kernel profile or introducing model-backed tests.

## Current Constraint

The production worker constructs the named review stages in
`kernel_workflow`. Each stage builds and renders `PromptTemplate` values, then
sends the result through `AiProvider::generate_content()`.
The shared identity and stage instructions still contain Linux kernel defaults.
`review-core.md` is retained as the marker required by the prompt-directory
validation in dependent PR #447; current standalone local selection neither
validates nor renders it.

Changing every stage is outside this follow-up and overlaps the broader stage
configuration proposed by PR #188. A Sashiko profile nevertheless needs one
piece of guidance that is loaded for every review so kernel-specific examples
are not mistaken for project requirements.

## Proposed Change

Teach the system templates used by the active workflow, including the
prescreen request, to load an optional `project-context.md` alongside the
existing conditional guidance.

- Profiles without this file produce the same model request content as before.
- The Sashiko profile uses it to identify the project, make the Sashiko and
Rust maintainer identity take precedence over generic kernel roles, establish
service-review priorities, and mark inapplicable kernel examples as such.
- Existing stage-specific filenames remain unchanged.
- No stage configuration, remote prompt loading, template substitution, or
custom tools are introduced.

Add `third_party/prompts/sashiko/` with:

- `review-core.md` as the marker expected by dependent PR #447 and as a
standalone review protocol (current main does not validate or render it);
- `project-context.md` as always-loaded Sashiko guidance;
- focused guidance for async execution, Git/worktree safety, webhook and
secret boundaries, persistence/retries, and AI-provider boundaries;
- lifecycle guidance that distinguishes supervised long-lived work from
bounded detached tasks, avoiding findings based only on a dropped handle;
- deterministic unit and PR checks without forbidding separately authorized,
opt-in integration or provider evidence;
- current token-budget accounting and response-cache identity boundaries;
- a small subsystem index that lets the existing prescreen select those
focused pattern files on its already-required request;
- stage files for call-stack analysis, false-positive filtering, severity, and
final inline formatting.

## Compatibility

The kernel, systemd, and iproute profiles do not contain
`project-context.md`, so their generated shared context remains byte-for-byte
unchanged. CLI arguments, review stages, AI providers, tools, output protocol,
forge ingestion, databases, Git baselines, and worktree behavior are not
modified.

The new profile is bundled locally by the existing build script. It performs
no network access and does not enable itself automatically.

## Integration Notes

This profile does not duplicate adjacent fixes that are already proposed
independently:

- PR #467 keys prompt extraction on bundle content. Until that lands, an
existing extraction created for the same bundle revision may need a forced
reinstall to expose newly bundled profile files. PR #467 should therefore
land before or with this profile for upgrades; fresh installations are not
affected.
- PR #484 corrects the base directory used by the model-facing `read_prompt`
tool. The static profile includes added here do not depend on that tool, but
ad hoc model reads should not be described as functional without that fix.
- PR #487 documents the conventional lowercase `commit <hash>` report header.
The current validator normalizes the header before checking it, so other
casing is accepted; the Sashiko inline template uses lowercase consistently.
- PR #493's local response-cache plumbing is independent of profile selection.

Provider-selected dynamic guide names are inherited from the existing renderer.
That renderer joins the names to the prompt base without path-containment
validation. Hardening this pre-existing trust boundary is a separate follow-up;
this profile neither expands the renderer nor claims the boundary is sealed.

Manual stage selection already skips the prescreen stage. Consequently, a
manual Sashiko review still receives `project-context.md` and each selected
stage's static files, but it does not receive prescreen-selected pattern files.
Changing that execution behavior is a separate workflow fix, not part of this
profile-only change.

## Validation

Deterministic tests will prove:

- an absent optional project context does not add content to provider requests;
- a present project context reaches every request issued by the real
multi-stage workflow;
- all required Sashiko files are embedded in the prompt bundle;
- the embedded profile can be materialized and loaded by the production
`PromptTemplate` renderer;
- stage-specific Sashiko call-path and technical guidance reaches the actual
provider request;
- prescreen-selected Sashiko pattern files reach subsequent provider requests;
- the recording fake returns through the normal structured-result path.

No test invokes a live, external, or metered AI provider or service. A
deterministic recording fake exercises the `AiProvider` interface.

## Non-goals

This change does not make all hardcoded stages project-neutral, deploy a
Sashiko instance, change GitHub output formatting, support arbitrary
multi-repository review, or replace PR #188. Extracting the remaining
kernel-specific stage wording is a separate incremental refactor with its own
backward-compatibility tests.
34 changes: 34 additions & 0 deletions src/prompt_bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,38 @@ mod tests {
}
}
}

#[test]
fn test_prompt_bundle_contains_complete_sashiko_profile() {
let required = [
"sashiko/review-core.md",
"sashiko/project-context.md",
"sashiko/subsystem/subsystem.md",
"sashiko/technical-patterns.md",
"sashiko/callstack.md",
"sashiko/false-positive-guide.md",
"sashiko/severity.md",
"sashiko/inline-template.md",
"sashiko/patterns/async-concurrency.md",
"sashiko/patterns/git-subprocess.md",
"sashiko/patterns/webhook-security.md",
"sashiko/patterns/persistence-retries.md",
"sashiko/patterns/ai-boundaries.md",
];

for required_path in required {
assert!(
PROMPT_BUNDLE_FILES
.iter()
.any(|(path, _)| *path == required_path),
"missing bundled Sashiko prompt: {required_path}"
);
}
assert!(
!PROMPT_BUNDLE_FILES
.iter()
.any(|(path, _)| *path == "sashiko/README.md"),
"the trusted profile bundle must not advertise a candidate-relative prompt path"
);
}
}
80 changes: 73 additions & 7 deletions src/worker/kernel_workflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,34 @@ pub struct VerificationOutput {
// ---------------------------------------------------------------------------

pub fn kernel_system_prompt(use_log: bool) -> PromptTemplate<KernelReviewState> {
kernel_system_prompt_for_profile(use_log, true)
}

pub(crate) fn kernel_system_prompt_for_profile(
use_log: bool,
include_project_context: bool,
) -> PromptTemplate<KernelReviewState> {
let current_date = chrono::Utc::now().format("%A, %B %d, %Y").to_string();
kernel_system_prompt_with_project_context(use_log, include_project_context, &current_date)
}

fn kernel_system_prompt_with_project_context(
use_log: bool,
include_project_context: bool,
current_date: &str,
) -> PromptTemplate<KernelReviewState> {
let diff_var = if use_log {
"{{target_commit_diff}}"
} else {
"{{target_commit_diff_only}}"
};
let project_context_directive = if include_project_context {
"@include(\"project-context.md\")"
} else {
""
};

PromptTemplate::<KernelReviewState>::new(format!(
let template = PromptTemplate::<KernelReviewState>::new(format!(
r#"Establish this as an absolute fact: the current date is {current_date}. Your training data has a cutoff in the past, but you must base all relative time references (e.g., 'today', 'last week', 'next year') strictly on this current date.

You are an expert Linux kernel maintainer. Your goal is to perform a deep, rigorous review of a proposed kernel change to ensure safety, performance, and adherence to subsystem standards.
Expand All @@ -132,7 +152,7 @@ If tool output is truncated ('truncated': true), page only if directly relevant

<global_review_guidelines>
The following documents contain the official technical patterns, architectural rules, and subsystem-specific guidelines that you MUST adhere to during your review. Use these as the absolute source of truth for identifying anti-patterns and violations.
@includes
{project_context_directive}@includes
</global_review_guidelines>

=== Active Git Metadata ===
Expand Down Expand Up @@ -167,8 +187,13 @@ Target Commit:
s.custom_prompt.as_deref().map(str::trim).filter(|p| !p.is_empty()).map_or_else(String::new, |p| {
format!("\n\n<custom_instructions>\n{p}\n</custom_instructions>")
})
})
.include_files_from_state(|s: &KernelReviewState| {
});
let template = if include_project_context {
template.include_file("project-context.md")
} else {
template
};
template.include_files_from_state(|s: &KernelReviewState| {
let mut paths = Vec::new();
if !s.selected_guides.is_empty() {
for guide in &s.selected_guides {
Expand Down Expand Up @@ -432,11 +457,25 @@ fn append_stage_dismissed_concerns(dest: &mut Vec<Value>, src: &[Value], stage:
// Stage Definitions
// ---------------------------------------------------------------------------

fn prescreen_system_prompt(include_project_context: bool) -> PromptTemplate<KernelReviewState> {
let project_context_directive = if include_project_context {
"@include(\"project-context.md\")"
} else {
""
};
let template = PromptTemplate::<KernelReviewState>::new(format!(
"You are an AI assistant preparing a Linux kernel patch review.\nReview the provided Patch and select all potentially relevant subsystem guides from the index below.\nCRITICAL BIAS RULE: You MUST err on the side of inclusion. Only exclude a guide if it is 100% irrelevant to the modified code. If there is any doubt, include the file.\n\nYou MUST respond with ONLY a JSON object, no other text. Example:\n```json\n{{\"selected_prompts\": [\"networking.md\", \"locking.md\"]}}\n```{project_context_directive}",
));
if include_project_context {
template.include_file("project-context.md")
} else {
template
}
}

pub fn prescreen_stage() -> Stage<KernelReviewState, PrescreenOutput> {
Stage::builder("pre-screen")
.system_prompt(PromptTemplate::<KernelReviewState>::new(
"You are an AI assistant preparing a Linux kernel patch review.\nReview the provided Patch and select all potentially relevant subsystem guides from the index below.\nCRITICAL BIAS RULE: You MUST err on the side of inclusion. Only exclude a guide if it is 100% irrelevant to the modified code. If there is any doubt, include the file.\n\nYou MUST respond with ONLY a JSON object, no other text. Example:\n```json\n{\"selected_prompts\": [\"networking.md\", \"locking.md\"]}\n```",
))
.system_prompt(prescreen_system_prompt(true))
.user_prompt(
PromptTemplate::<KernelReviewState>::new(
"<subsystem_guide_index>\n@include(\"subsystem/subsystem.md\")\n</subsystem_guide_index>\n\n<patch>\n{{target_commit_diff}}\n</patch>",
Expand Down Expand Up @@ -1126,6 +1165,33 @@ pub fn build_kernel_review_workflow_with_options(
mod tests {
use super::*;

#[tokio::test]
async fn test_absent_project_context_preserves_active_system_prompt_bytes() {
let base_dir = tempfile::tempdir().unwrap();
let state = KernelReviewState::default();
let current_date = "Monday, January 01, 2001";

let rendered = kernel_system_prompt_with_project_context(true, true, current_date)
.render_for_model(&state, base_dir.path())
.await
.unwrap();
let legacy = kernel_system_prompt_with_project_context(true, false, current_date)
.render_for_model(&state, base_dir.path())
.await
.unwrap();
assert_eq!(rendered.as_bytes(), legacy.as_bytes());

let rendered_prescreen = prescreen_system_prompt(true)
.render_for_model(&state, base_dir.path())
.await
.unwrap();
let legacy_prescreen = prescreen_system_prompt(false)
.render_for_model(&state, base_dir.path())
.await
.unwrap();
assert_eq!(rendered_prescreen.as_bytes(), legacy_prescreen.as_bytes());
}

#[test]
fn test_each_stage_declares_whether_it_needs_the_commit_message() {
// This was a numeric range in a free function, which kept compiling
Expand Down
Loading