Skip to content

Commit 5f2ad4c

Browse files
tikazyqclaude
andauthored
feat(http): migrate read path to adapter layer (#261) (#289)
* feat(http): migrate read path to adapter layer (#261) Spec #258 isolated markdown internals (SpecLoader, SpecInfo, SpecWriter, SpecArchiver, …) behind a module boundary and left leanspec-http intentionally broken as a compiler-guided migration map. This change implements the read-path portion of that migration per spec #261: - types/specs.rs: SpecSummary/SpecDetail now build from &SpecDoc using schema semantic hints (status, priority, tags, assignee) rather than the old SpecInfo frontmatter struct. - handlers/specs/helpers.rs: get_loader_and_project replaced with get_adapter_and_project returning Box<dyn Adapter>, plus shared helpers for AdapterError → HTTP mapping and a markdown-adapter guard. - handlers/specs/read.rs: list / get / search / raw / sub-spec read handlers go through adapter.list() and adapter.get(). Hierarchy and required_by are computed from SpecDoc.links instead of the markdown loader's relationship index. - handlers/specs/compute.rs: stats group by semantic-hint fields on SpecDoc; dependencies and per-spec validation now return HTTP 422 with a clear error when the active adapter is not markdown. - watcher.rs / state.rs: the file watcher holds a typed Arc<MarkdownAdapter> per project for cache invalidation and only activates for projects whose adapter resolves to markdown. - handlers/adapter.rs: adds GET /api/projects/{id}/schema returning the active adapter's SpecSchema as JSON so the UI can render dynamic field sets. Write handlers (create / metadata / raw / checklist / batch) are migrated minimally to the adapter API so the crate compiles — they keep using leanspec_core::adapters::markdown::{SpecInfo, SpecStatus, SpecFrontmatter} via the public markdown module path for transition checks, umbrella completion verification, and template rendering. Full fetch-transform-push migration of those handlers lives in #262. Tests: cargo test -p leanspec-http (102 passing), cargo clippy -p leanspec-http -- -D warnings, cargo fmt -- --check all green. Closes #261 * fix(http): address review feedback on adapter migration Copilot review on #289 flagged seven issues; the substantive ones are fixed here: * Path traversal in `resolve_markdown_spec_path`: spec ids containing `..`, path separators, or absolute paths now return None instead of joining outside the specs root. Same `invalid_spec_id` guard rejects malicious names in `create_project_spec` so they can't write outside `specs_dir`. * `file_path` regression on markdown list/search responses: SpecDoc.url is None for the markdown adapter, so list summaries had empty `file_path`. A new `populate_file_path` helper falls back to the on-disk README path for markdown projects so the field round-trips like before. * Cycle detection in `build_hierarchy`: parent links forming a cycle (A→B, B→A) would recurse forever. The walker now carries a visited set and short-circuits when a spec is re-entered, so a malformed dataset can no longer stack-overflow the server. * Relative `settings.directory` resolution: a markdown adapter config with `directory: specs` was being resolved against the server CWD rather than the project root. New `adapter_resolution.rs` module normalises relative paths against `project_root` before instantiating the adapter, so the `/specs`, `/adapter`, and `/schema` endpoints all agree on the right spec tree. * Duplicated adapter config candidate list: `state.rs` had its own copy of the YAML candidate paths. Both watcher and handlers now share `adapter_resolution::ADAPTER_CONFIG_CANDIDATES`/`find_adapter_config`, so watcher activation can't drift from request-time resolution. * Fuzzy spec id match in `resolve_markdown_spec_path`: the old `contains` heuristic would resolve `01` onto `001-foo` or `foo` onto `bar-foo-baz`. Tightened to numeric-prefix or slug-suffix exact match against the directory's `<NNN>-<slug>` shape. Added unit tests cover: `invalid_spec_id` rejecting traversal, fuzzy matching boundaries, `build_hierarchy` terminating on a parent cycle, and the new adapter-resolution behaviour (fallback, relative→absolute, absolute preserved, non-markdown left alone). --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6fef741 commit 5f2ad4c

13 files changed

Lines changed: 1841 additions & 890 deletions

File tree

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
//! Shared helpers for resolving the active adapter for a project.
2+
//!
3+
//! Centralised so handlers (`/specs`, `/adapter`, `/schema`) and the file
4+
//! watcher in `state.rs` all use the exact same lookup order and config
5+
//! normalisation. Keeping this in one place stops watcher activation from
6+
//! drifting away from request-time adapter resolution.
7+
8+
use std::path::{Path, PathBuf};
9+
10+
use leanspec_core::adapters::{Adapter, AdapterConfig, AdapterError, AdapterRegistry};
11+
12+
/// Candidate file paths (relative to the project root) for adapter config,
13+
/// in priority order. The legacy `provider:` files are honoured so existing
14+
/// projects keep working.
15+
pub const ADAPTER_CONFIG_CANDIDATES: &[&str] = &[
16+
"leanspec.adapter.yaml",
17+
".lean-spec/adapter.yaml",
18+
"leanspec.provider.yaml",
19+
".lean-spec/provider.yaml",
20+
];
21+
22+
/// Returns the first adapter config file that exists under `project_root`.
23+
pub fn find_adapter_config(project_root: &Path) -> Option<PathBuf> {
24+
ADAPTER_CONFIG_CANDIDATES
25+
.iter()
26+
.map(|c| project_root.join(c))
27+
.find(|p| p.exists())
28+
}
29+
30+
/// Load the adapter config for a project. Falls back to a markdown adapter
31+
/// rooted at `specs_dir` when no config file is present.
32+
///
33+
/// When the loaded config is a markdown adapter with a relative
34+
/// `settings.directory`, the directory is resolved against `project_root` so
35+
/// the adapter reads from the right tree regardless of the server's CWD.
36+
pub fn load_adapter_config(
37+
project_root: &Path,
38+
specs_dir: &Path,
39+
) -> Result<AdapterConfig, AdapterError> {
40+
let mut config = if let Some(path) = find_adapter_config(project_root) {
41+
AdapterRegistry::load_config(&path)?
42+
} else {
43+
AdapterConfig {
44+
adapter: "markdown".into(),
45+
settings: serde_json::json!({ "directory": specs_dir.to_string_lossy().as_ref() }),
46+
}
47+
};
48+
49+
normalise_markdown_directory(&mut config, project_root, specs_dir);
50+
Ok(config)
51+
}
52+
53+
/// Resolve and instantiate the active adapter for a project.
54+
pub fn resolve_adapter(
55+
project_root: &Path,
56+
specs_dir: &Path,
57+
) -> Result<Box<dyn Adapter>, AdapterError> {
58+
let config = load_adapter_config(project_root, specs_dir)?;
59+
AdapterRegistry::create(&config)
60+
}
61+
62+
/// When the active adapter is markdown, rewrite a relative `settings.directory`
63+
/// (e.g. `"specs"`) so it's anchored at the project root. Falls back to
64+
/// `specs_dir` when the config omits the field entirely.
65+
fn normalise_markdown_directory(config: &mut AdapterConfig, project_root: &Path, specs_dir: &Path) {
66+
if config.adapter != "markdown" {
67+
return;
68+
}
69+
70+
let current = config
71+
.settings
72+
.get("directory")
73+
.and_then(|v| v.as_str())
74+
.map(PathBuf::from);
75+
76+
let resolved = match current {
77+
Some(dir) if dir.is_absolute() => dir,
78+
Some(dir) => project_root.join(dir),
79+
None => specs_dir.to_path_buf(),
80+
};
81+
82+
if let Some(obj) = config.settings.as_object_mut() {
83+
obj.insert(
84+
"directory".into(),
85+
serde_json::Value::String(resolved.to_string_lossy().into_owned()),
86+
);
87+
} else {
88+
config.settings = serde_json::json!({ "directory": resolved.to_string_lossy().as_ref() });
89+
}
90+
}
91+
92+
#[cfg(test)]
93+
mod tests {
94+
use super::*;
95+
use std::fs;
96+
use tempfile::TempDir;
97+
98+
#[test]
99+
fn fallback_uses_specs_dir() {
100+
let tmp = TempDir::new().unwrap();
101+
let specs = tmp.path().join("specs");
102+
fs::create_dir_all(&specs).unwrap();
103+
104+
let config = load_adapter_config(tmp.path(), &specs).unwrap();
105+
assert_eq!(config.adapter, "markdown");
106+
assert_eq!(
107+
config.settings.get("directory").and_then(|v| v.as_str()),
108+
Some(specs.to_string_lossy().as_ref())
109+
);
110+
}
111+
112+
#[test]
113+
fn relative_directory_resolved_against_project_root() {
114+
let tmp = TempDir::new().unwrap();
115+
let project = tmp.path();
116+
let yaml = project.join("leanspec.adapter.yaml");
117+
fs::write(&yaml, "adapter: markdown\ndirectory: docs/specs\n").unwrap();
118+
119+
let specs = project.join("specs");
120+
let config = load_adapter_config(project, &specs).unwrap();
121+
let resolved = config
122+
.settings
123+
.get("directory")
124+
.and_then(|v| v.as_str())
125+
.unwrap();
126+
assert_eq!(resolved, project.join("docs/specs").to_string_lossy());
127+
}
128+
129+
#[test]
130+
fn absolute_directory_preserved() {
131+
let tmp = TempDir::new().unwrap();
132+
let project = tmp.path();
133+
let yaml = project.join("leanspec.adapter.yaml");
134+
let abs = project.join("absolute-specs");
135+
fs::write(
136+
&yaml,
137+
format!("adapter: markdown\ndirectory: {}\n", abs.display()),
138+
)
139+
.unwrap();
140+
141+
let specs = project.join("specs");
142+
let config = load_adapter_config(project, &specs).unwrap();
143+
assert_eq!(
144+
config.settings.get("directory").and_then(|v| v.as_str()),
145+
Some(abs.to_string_lossy().as_ref())
146+
);
147+
}
148+
149+
#[test]
150+
fn non_markdown_adapter_left_alone() {
151+
let tmp = TempDir::new().unwrap();
152+
let project = tmp.path();
153+
let yaml = project.join("leanspec.adapter.yaml");
154+
fs::write(
155+
&yaml,
156+
"adapter: github\nowner: acme\nrepo: backend\ntoken_env: TEST_TOKEN\n",
157+
)
158+
.unwrap();
159+
160+
let specs = project.join("specs");
161+
let config = load_adapter_config(project, &specs).unwrap();
162+
assert_eq!(config.adapter, "github");
163+
// No `directory` key was added.
164+
assert!(config.settings.get("directory").is_none());
165+
}
166+
}
Lines changed: 21 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,22 @@
1-
//! Adapter introspection endpoint.
1+
//! Adapter introspection endpoints.
22
//!
3-
//! Returns the active [`leanspec_core::Adapter`]'s capabilities for a given
4-
//! project so clients (UI, agents) can build adapter-aware workflows without
5-
//! assuming markdown-specific conventions.
3+
//! Surfaces the active [`leanspec_core::Adapter`]'s capabilities and schema
4+
//! for a given project so clients (UI, agents) can build adapter-aware
5+
//! workflows without assuming markdown-specific conventions.
66
77
#![allow(clippy::result_large_err)]
88

9-
use std::path::Path as FsPath;
10-
119
use axum::extract::{Path, State};
1210
use axum::http::StatusCode;
1311
use axum::Json;
14-
use leanspec_core::adapters::{AdapterCapabilities, AdapterConfig, AdapterRegistry};
12+
use leanspec_core::adapters::AdapterCapabilities;
13+
use leanspec_core::SpecSchema;
1514

15+
use crate::adapter_resolution::resolve_adapter;
1616
use crate::error::ApiError;
1717
use crate::state::AppState;
1818
use crate::utils::resolve_project;
1919

20-
/// Candidate file paths (relative to the project root) for adapter config,
21-
/// in priority order. The legacy `provider:` files are honoured so existing
22-
/// projects keep working.
23-
const ADAPTER_CONFIG_CANDIDATES: &[&str] = &[
24-
"leanspec.adapter.yaml",
25-
".lean-spec/adapter.yaml",
26-
"leanspec.provider.yaml",
27-
".lean-spec/provider.yaml",
28-
];
29-
30-
fn resolve_adapter_for_project(
31-
project_root: &FsPath,
32-
specs_dir: &FsPath,
33-
) -> Result<Box<dyn leanspec_core::Adapter>, (StatusCode, Json<ApiError>)> {
34-
for candidate in ADAPTER_CONFIG_CANDIDATES {
35-
let path = project_root.join(candidate);
36-
if path.exists() {
37-
let config = AdapterRegistry::load_config(&path).map_err(api_error)?;
38-
return AdapterRegistry::create(&config).map_err(api_error);
39-
}
40-
}
41-
42-
// No adapter config — fall back to the markdown adapter pointed at the
43-
// project's declared specs directory.
44-
let config = AdapterConfig {
45-
adapter: "markdown".into(),
46-
settings: serde_json::json!({ "directory": specs_dir.to_string_lossy().as_ref() }),
47-
};
48-
AdapterRegistry::create(&config).map_err(api_error)
49-
}
50-
5120
fn api_error(err: leanspec_core::AdapterError) -> (StatusCode, Json<ApiError>) {
5221
(
5322
StatusCode::INTERNAL_SERVER_ERROR,
@@ -61,6 +30,19 @@ pub async fn get_project_adapter_capabilities(
6130
Path(project_id): Path<String>,
6231
) -> Result<Json<AdapterCapabilities>, (StatusCode, Json<ApiError>)> {
6332
let project = resolve_project(&state, &project_id).await?;
64-
let adapter = resolve_adapter_for_project(&project.path, &project.specs_dir)?;
33+
let adapter = resolve_adapter(&project.path, &project.specs_dir).map_err(api_error)?;
6534
Ok(Json(adapter.capabilities().clone()))
6635
}
36+
37+
/// GET /api/projects/{id}/schema
38+
///
39+
/// Returns the active adapter's `SpecSchema` so the UI can render dynamic
40+
/// field sets without hard-coding adapter-specific conventions.
41+
pub async fn get_project_schema(
42+
State(state): State<AppState>,
43+
Path(project_id): Path<String>,
44+
) -> Result<Json<SpecSchema>, (StatusCode, Json<ApiError>)> {
45+
let project = resolve_project(&state, &project_id).await?;
46+
let adapter = resolve_adapter(&project.path, &project.specs_dir).map_err(api_error)?;
47+
Ok(Json(adapter.schema().clone()))
48+
}

0 commit comments

Comments
 (0)