419 lines
16 KiB
Markdown
419 lines
16 KiB
Markdown
# Request URL Resolution Implementation Plan
|
||
|
||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||
|
||
**Goal:** Replace the temporary line-loss request URL hardcode in `src/service/server.rs` with a unified bootstrap-target resolver that prefers current page context, then deterministic submit plans, then skill metadata, and finally `about:blank`.
|
||
|
||
**Architecture:** Add a small service-owned resolver that returns a narrow `SubmitBootstrapTarget` result and centralizes precedence rules. Reuse `DeterministicExecutionPlan.target_url` as the authoritative source for deterministic line-loss scenes, then add minimal skill metadata fallback for configured direct browser-script skills, while keeping callback-host behavior unchanged.
|
||
|
||
**Tech Stack:** Rust, serde/serde_json, tungstenite, zeroclaw skill loader, staged `SKILL.toml` manifests, cargo test
|
||
|
||
---
|
||
|
||
### Task 1: Add resolver-focused red tests for precedence
|
||
|
||
**Files:**
|
||
- Modify: `src/service/server.rs:422-467`
|
||
- Test: `src/service/server.rs` (crate-local resolver tests)
|
||
- Test: `tests/service_ws_session_test.rs`
|
||
|
||
- [ ] **Step 1: Write the failing page-context precedence test**
|
||
|
||
In a crate-local unit test inside `src/service/server.rs`, add a focused resolver test that exercises the request-url resolver with:
|
||
- non-empty `page_url = "https://already-open.example.com/page"`
|
||
- an instruction that would otherwise match deterministic line-loss logic
|
||
- configured direct skill metadata present
|
||
|
||
Assert the resolved bootstrap target uses the explicit non-empty `page_url` and reports `PageContext` source.
|
||
|
||
- [ ] **Step 2: Run the test to verify it fails**
|
||
|
||
Run: `cargo test page_context_bootstrap_target_wins_over_deterministic_and_skill_fallback --lib -- --nocapture`
|
||
Expected: FAIL because no unified resolver/source enum exists yet.
|
||
|
||
- [ ] **Step 3: Write the failing deterministic-precedence test**
|
||
|
||
In `src/service/server.rs` crate-local tests, add a focused test for a deterministic line-loss instruction with no `page_url`.
|
||
|
||
Use the same instruction shape already accepted by `decide_deterministic_submit(...)`, and assert:
|
||
- resolver source is `DeterministicPlan`
|
||
- resolved `request_url` equals `DeterministicExecutionPlan.target_url`
|
||
- no raw `instruction.contains("线损")` fallback is needed
|
||
|
||
- [ ] **Step 4: Run the test to verify it fails**
|
||
|
||
Run: `cargo test deterministic_bootstrap_target_uses_plan_target_url --lib -- --nocapture`
|
||
Expected: FAIL because service still uses `derive_request_url_from_instruction(...)`.
|
||
|
||
- [ ] **Step 5: Write the failing skill-fallback test**
|
||
|
||
In `src/service/server.rs` crate-local tests, add a focused test for:
|
||
- no `page_url`
|
||
- instruction not deterministic
|
||
- configured direct-submit skill metadata provides `bootstrap_url`
|
||
|
||
Assert resolver source is `SkillConfig` and `request_url` matches metadata.
|
||
|
||
- [ ] **Step 6: Run the test to verify it fails**
|
||
|
||
Run: `cargo test skill_metadata_bootstrap_url_is_used_when_no_page_context_or_plan_exists --lib -- --nocapture`
|
||
Expected: FAIL because skill metadata is not read today.
|
||
|
||
- [ ] **Step 7: Write the failing malformed-metadata fallback test**
|
||
|
||
In `src/service/server.rs` crate-local tests, add a focused test for malformed `bootstrap_url` metadata, with no page context and no deterministic plan.
|
||
|
||
Assert the resolver:
|
||
- ignores malformed metadata
|
||
- returns `Fallback`
|
||
- resolves to `about:blank`
|
||
|
||
- [ ] **Step 8: Run the test to verify it fails**
|
||
|
||
Run: `cargo test malformed_skill_bootstrap_url_falls_back_to_about_blank --lib -- --nocapture`
|
||
Expected: FAIL because malformed metadata is not handled by a resolver yet.
|
||
|
||
---
|
||
|
||
### Task 2: Introduce the bootstrap-target resolver in service code
|
||
|
||
**Files:**
|
||
- Modify: `src/service/server.rs:280-467`
|
||
- Modify: `src/service/mod.rs:17-22`
|
||
- Test: `src/service/server.rs` (crate-local resolver tests)
|
||
|
||
- [ ] **Step 1: Add the narrow resolver types in service code**
|
||
|
||
In `src/service/server.rs`, add:
|
||
|
||
```rust
|
||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||
pub(crate) struct SubmitBootstrapTarget {
|
||
pub request_url: String,
|
||
pub expected_domain: Option<String>,
|
||
pub source: BootstrapTargetSource,
|
||
}
|
||
|
||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||
pub(crate) enum BootstrapTargetSource {
|
||
PageContext,
|
||
DeterministicPlan,
|
||
SkillConfig,
|
||
Fallback,
|
||
}
|
||
```
|
||
|
||
Keep them scoped to service code. Do not create a generic cross-runtime planning object.
|
||
|
||
- [ ] **Step 2: Add a minimal resolver entry point**
|
||
|
||
Implement a service-owned function in `src/service/server.rs`, conceptually:
|
||
|
||
```rust
|
||
pub(crate) fn resolve_submit_bootstrap_target(
|
||
request: &crate::agent::SubmitTaskRequest,
|
||
workspace_root: &Path,
|
||
settings: &SgClawSettings,
|
||
) -> SubmitBootstrapTarget
|
||
```
|
||
|
||
Initial behavior for this step:
|
||
- return `PageContext` only when `request.page_url` exists and is non-empty after trimming
|
||
- add a crate-local regression that empty/whitespace `page_url` does not short-circuit later precedence tiers
|
||
- otherwise fall through to existing behavior temporarily so the new tests can compile incrementally
|
||
|
||
- [ ] **Step 3: Update service startup to call the resolver**
|
||
|
||
At the callback-host startup call site in `serve_client(...)`, replace:
|
||
|
||
```rust
|
||
let bootstrap_url = initial_request_url_for_submit_task(&request);
|
||
```
|
||
|
||
with resolver usage:
|
||
|
||
```rust
|
||
let bootstrap_target = resolve_submit_bootstrap_target(&request, context.workspace_root(), &settings);
|
||
let bootstrap_url = bootstrap_target.request_url;
|
||
```
|
||
|
||
Use the current settings-loading seam already used elsewhere in service code. Keep callback-host startup behavior otherwise unchanged.
|
||
|
||
- [ ] **Step 4: Keep resolver visibility crate-local**
|
||
|
||
Do not make the resolver types broadly public for integration tests. Keep the resolver and `BootstrapTargetSource` crate-local, and keep source-level assertions in `src/service/server.rs` unit tests.
|
||
|
||
Only re-export/remove existing `initial_request_url_for_submit_task(...)` seams through `src/service/mod.rs` if production callers still require that wiring.
|
||
|
||
- [ ] **Step 5: Run the first precedence test to verify it passes**
|
||
|
||
Run: `cargo test page_context_bootstrap_target_wins_over_deterministic_and_skill_fallback --lib -- --nocapture`
|
||
Expected: PASS.
|
||
|
||
- [ ] **Step 6: Commit**
|
||
|
||
```bash
|
||
git add src/service/server.rs src/service/mod.rs
|
||
git commit -m "refactor(service): add submit bootstrap target resolver scaffold"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 3: Make deterministic submit the authoritative source for line-loss bootstrap URLs
|
||
|
||
**Files:**
|
||
- Modify: `src/service/server.rs:422-467`
|
||
- Modify: `src/compat/deterministic_submit.rs:13-101`
|
||
- Test: `src/service/server.rs` (crate-local resolver tests)
|
||
- Test: `tests/service_ws_session_test.rs`
|
||
|
||
- [ ] **Step 1: Write a small service-side seam for deterministic resolution**
|
||
|
||
In `src/service/server.rs`, update the resolver so that when `page_url` is absent it calls:
|
||
|
||
```rust
|
||
crate::compat::deterministic_submit::decide_deterministic_submit(
|
||
&request.instruction,
|
||
request.page_url.as_deref(),
|
||
request.page_title.as_deref(),
|
||
)
|
||
```
|
||
|
||
Only `DeterministicSubmitDecision::Execute(plan)` should produce a deterministic bootstrap target.
|
||
|
||
Treat `NotDeterministic` and `Prompt { .. }` as “no deterministic bootstrap target” for service startup.
|
||
|
||
- [ ] **Step 2: Use `plan.target_url` directly**
|
||
|
||
Map `DeterministicSubmitDecision::Execute(plan)` to:
|
||
- `request_url = plan.target_url.clone()`
|
||
- `expected_domain = Some(plan.expected_domain.clone())`
|
||
- `source = BootstrapTargetSource::DeterministicPlan`
|
||
|
||
Do not reconstruct the URL in `server.rs`.
|
||
|
||
- [ ] **Step 3: Remove the temporary line-loss hardcode**
|
||
|
||
Delete this branch from `derive_request_url_from_instruction(...)` or remove the function entirely if it is no longer needed:
|
||
|
||
```rust
|
||
if instruction.contains("线损") || instruction.contains("lineloss") {
|
||
return Some("http://20.76.57.61:18080".to_string());
|
||
}
|
||
```
|
||
|
||
Keep any still-needed legacy Zhihu fallback only if the resolver still requires it after deterministic integration.
|
||
|
||
- [ ] **Step 4: Add/adjust a deterministic regression test**
|
||
|
||
In `src/service/server.rs` crate-local tests, add a focused assertion that line-loss bootstrap URL now comes from `DeterministicExecutionPlan.target_url`, not raw text matching.
|
||
|
||
A good assertion shape is:
|
||
- call resolver with deterministic line-loss instruction
|
||
- assert `request_url == "http://20.76.57.61:18080/gsllys/tqLinelossStatis/tqQualifyRateMonitor"`
|
||
- assert `source == DeterministicPlan`
|
||
|
||
- [ ] **Step 5: Run deterministic tests to verify they pass**
|
||
|
||
Run: `cargo test deterministic_bootstrap_target_uses_plan_target_url --lib -- --nocapture`
|
||
Expected: PASS.
|
||
|
||
- [ ] **Step 6: Run service websocket coverage for the same precedence**
|
||
|
||
Run: `cargo test callback_host --test service_ws_session_test -- --nocapture`
|
||
Expected: PASS with no line-loss hardcode dependency.
|
||
|
||
- [ ] **Step 7: Commit**
|
||
|
||
```bash
|
||
git add src/service/server.rs src/compat/deterministic_submit.rs tests/service_ws_session_test.rs
|
||
git commit -m "refactor(service): derive line-loss bootstrap URL from deterministic plan"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 4: Add skill-metadata fallback for configured direct-submit skills
|
||
|
||
**Files:**
|
||
- Modify: `src/compat/direct_skill_runtime.rs:114-153`
|
||
- Modify: `src/service/server.rs:422-467`
|
||
- Optionally modify: `src/config/settings.rs` only if a tiny metadata pointer is required
|
||
- Modify: `D:/data/ideaSpace/rust/sgClaw/claw/claw/skills/skill_staging/skills/fault-details-report/SKILL.toml`
|
||
- Optionally modify: `D:/data/ideaSpace/rust/sgClaw/claw/claw/skills/skill_staging/skills/95598-weekly-monitor-report/SKILL.toml`
|
||
- Test: `src/service/server.rs` (crate-local resolver tests)
|
||
- Test: `tests/service_ws_session_test.rs`
|
||
|
||
- [ ] **Step 1: Define the minimal skill metadata shape**
|
||
|
||
Extend staged `SKILL.toml` parsing expectations to support a narrow metadata seam for browser-script direct skills.
|
||
|
||
The plan target fields are:
|
||
- `bootstrap_url`
|
||
- `expected_domain`
|
||
|
||
Keep the metadata minimal. Do not add a broad dispatch registry or scene-policy schema.
|
||
|
||
Recommended TOML shape in the skill manifest:
|
||
|
||
```toml
|
||
[tools.metadata]
|
||
bootstrap_url = "https://example.com/path"
|
||
expected_domain = "example.com"
|
||
```
|
||
|
||
If the actual skill loader only supports per-tool custom fields in another location, use that established seam instead. Do not invent a parallel config file.
|
||
|
||
- [ ] **Step 2: Add a helper that reads fallback metadata for the configured direct skill**
|
||
|
||
In `src/compat/direct_skill_runtime.rs`, add a helper like:
|
||
|
||
```rust
|
||
pub(crate) fn resolve_direct_submit_bootstrap_metadata(
|
||
configured_tool: &str,
|
||
workspace_root: &Path,
|
||
settings: &SgClawSettings,
|
||
) -> Result<Option<DirectSubmitBootstrapMetadata>, PipeError>
|
||
```
|
||
|
||
Recommended shape:
|
||
|
||
```rust
|
||
pub(crate) struct DirectSubmitBootstrapMetadata {
|
||
pub bootstrap_url: String,
|
||
pub expected_domain: Option<String>,
|
||
}
|
||
```
|
||
|
||
Reuse the existing `resolve_browser_script_skill(...)` lookup path so the service resolver does not duplicate staged-skill discovery logic.
|
||
|
||
- [ ] **Step 3: Validate metadata conservatively**
|
||
|
||
When reading fallback metadata:
|
||
- accept only non-empty `bootstrap_url`
|
||
- require it to parse as a valid absolute URL
|
||
- normalize or preserve `expected_domain` only if non-empty
|
||
- on malformed metadata, return `Ok(None)` for resolver purposes instead of failing service startup
|
||
|
||
This keeps malformed fallback data from breaking submits and matches the approved spec.
|
||
|
||
- [ ] **Step 4: Wire skill metadata into the service resolver**
|
||
|
||
Update `resolve_submit_bootstrap_target(...)` to:
|
||
- check skill metadata only after page context and deterministic parsing fail
|
||
- use `SkillConfig` as the source when metadata resolves
|
||
- fall through to `about:blank` when metadata is missing or malformed
|
||
|
||
- [ ] **Step 5: Add a staged-skill fixture update**
|
||
|
||
Update at least one configured direct skill fixture, likely `fault-details-report`, to include valid fallback metadata.
|
||
|
||
Use concrete values appropriate for that skill’s target page; do not reuse the line-loss URL.
|
||
|
||
- [ ] **Step 6: Run the skill-fallback test to verify it passes**
|
||
|
||
Run: `cargo test skill_metadata_bootstrap_url_is_used_when_no_page_context_or_plan_exists --lib -- --nocapture`
|
||
Expected: PASS.
|
||
|
||
- [ ] **Step 7: Run the malformed-metadata test to verify it passes**
|
||
|
||
Run: `cargo test malformed_skill_bootstrap_url_falls_back_to_about_blank --lib -- --nocapture`
|
||
Expected: PASS.
|
||
|
||
- [ ] **Step 8: Commit**
|
||
|
||
```bash
|
||
git add src/compat/direct_skill_runtime.rs src/service/server.rs D:/data/ideaSpace/rust/sgClaw/claw/claw/skills/skill_staging/skills/fault-details-report/SKILL.toml tests/service_ws_session_test.rs
|
||
git commit -m "feat(service): add direct skill bootstrap URL fallback metadata"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 5: Remove obsolete request-url glue and lock the final precedence contract
|
||
|
||
**Files:**
|
||
- Modify: `src/service/server.rs:422-467`
|
||
- Modify: `src/service/mod.rs:20-22`
|
||
- Test: `src/service/server.rs` (crate-local resolver tests)
|
||
- Test: `tests/service_ws_session_test.rs`
|
||
|
||
- [ ] **Step 1: Delete obsolete helper logic**
|
||
|
||
If `derive_request_url_from_instruction(...)` is no longer needed after resolver landing, delete it completely.
|
||
|
||
If a tiny legacy Zhihu-only seam still remains, keep it private behind the resolver and remove the old public shape from `service::browser_ws_client` if no longer needed.
|
||
|
||
- [ ] **Step 2: Lock the precedence contract with one final matrix test**
|
||
|
||
In `src/service/server.rs` crate-local tests, add one table-driven or clearly segmented test that verifies all four final outcomes:
|
||
- non-empty page context wins
|
||
- deterministic plan wins when page context is absent or empty
|
||
- skill metadata wins when page context and deterministic plan are absent
|
||
- fallback becomes `about:blank` when nothing resolves
|
||
|
||
- [ ] **Step 3: Run the focused resolver suite**
|
||
|
||
Run: `cargo test bootstrap_target --lib -- --nocapture`
|
||
Expected: PASS.
|
||
|
||
- [ ] **Step 4: Run service websocket regression coverage**
|
||
|
||
Run: `cargo test callback_host --test service_ws_session_test -- --nocapture`
|
||
Expected: PASS.
|
||
|
||
- [ ] **Step 5: Commit**
|
||
|
||
```bash
|
||
git add src/service/server.rs src/service/mod.rs tests/service_ws_session_test.rs
|
||
git commit -m "refactor(service): finalize bootstrap target precedence"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 6: Full verification and implementation handoff check
|
||
|
||
**Files:** None (verification only)
|
||
|
||
- [ ] **Step 1: Run focused deterministic and direct-skill tests**
|
||
|
||
Run: `cargo test deterministic_submit -- --nocapture`
|
||
Expected: PASS.
|
||
|
||
Run: `cargo test direct_submit -- --nocapture`
|
||
Expected: PASS.
|
||
|
||
- [ ] **Step 2: Run service submit regression coverage**
|
||
|
||
Run: `cargo test --test service_task_flow_test -- --nocapture`
|
||
Expected: PASS.
|
||
|
||
Run: `cargo test --test service_ws_session_test -- --nocapture`
|
||
Expected: PASS.
|
||
|
||
- [ ] **Step 3: Run targeted config/settings coverage if touched**
|
||
|
||
Run: `cargo test service_protocol_update_config_test -- --nocapture`
|
||
Expected: PASS.
|
||
|
||
- [ ] **Step 4: Build the project**
|
||
|
||
Run: `cargo build --bin sg_claw`
|
||
Expected: PASS.
|
||
|
||
- [ ] **Step 5: Manual behavior checklist**
|
||
|
||
Verify manually:
|
||
1. Existing page-attached submits still bootstrap against the current page URL.
|
||
2. Deterministic line-loss submit without page context boots helper against the line-loss target page from `DeterministicExecutionPlan.target_url`.
|
||
3. Non-deterministic configured direct skill without page context uses skill metadata bootstrap URL if present.
|
||
4. Missing or malformed skill metadata does not crash startup and falls back to `about:blank`.
|
||
5. No service code remains that hardcodes line-loss request URL by checking raw instruction text.
|
||
|
||
- [ ] **Step 6: Final commit (only if verification revealed required follow-up fixes)**
|
||
|
||
```bash
|
||
git add -A
|
||
git commit -m "test: lock request URL resolution precedence"
|
||
```
|
||
|
||
Only create this commit if verification required an additional code or test fix.
|