339 lines
12 KiB
Markdown
339 lines
12 KiB
Markdown
# Request URL Resolution Design
|
|
|
|
**Goal:** Replace the temporary line-loss hardcoded request URL logic in `src/service/server.rs` with a single bootstrap-target resolution path that prefers current page context first, then deterministic submit results, then skill-owned metadata, and only finally falls back to `about:blank`.
|
|
|
|
**Status:** Approved design direction for the next slice.
|
|
|
|
---
|
|
|
|
## Problem
|
|
|
|
The current callback-host bootstrap path still derives the first helper-page request URL in `src/service/server.rs`:
|
|
|
|
- `initial_request_url_for_submit_task(...)` prefers `request.page_url`
|
|
- then `derive_request_url_from_instruction(...)`
|
|
- then falls back to `about:blank`
|
|
|
|
This is currently patched with a temporary line-loss branch:
|
|
|
|
- if instruction contains `线损` or `lineloss`
|
|
- return `http://20.76.57.61:18080`
|
|
|
|
That temporary branch is the wrong ownership boundary:
|
|
|
|
1. service code is guessing scene intent from raw instruction text
|
|
2. deterministic submit already has a structured execution plan with `target_url`
|
|
3. future direct-submit skills may also need a bootstrap URL, but should not require new Rust hardcoded branches every time
|
|
|
|
The result is duplicated routing knowledge and brittle request URL derivation.
|
|
|
|
---
|
|
|
|
## Decision Summary
|
|
|
|
1. Introduce one sgClaw-owned bootstrap target resolver for service submit bootstrap.
|
|
2. Resolution order is:
|
|
- explicit `request.page_url`
|
|
- deterministic submit execution plan
|
|
- skill metadata fallback
|
|
- `about:blank`
|
|
3. Deterministic submit is the primary source of truth for deterministic scenes such as line loss.
|
|
4. Skill metadata provides the compatibility fallback for direct browser-script skills that do not have a deterministic plan.
|
|
5. Remove the current line-loss text-match hardcode from `src/service/server.rs` once the resolver is in place.
|
|
|
|
---
|
|
|
|
## Recommended Architecture
|
|
|
|
### 1. Add a dedicated bootstrap-target result type
|
|
|
|
Introduce a small sgClaw-side result type dedicated to callback-host bootstrap only.
|
|
|
|
Recommended shape:
|
|
|
|
```rust
|
|
pub struct SubmitBootstrapTarget {
|
|
pub request_url: String,
|
|
pub expected_domain: Option<String>,
|
|
pub source: BootstrapTargetSource,
|
|
}
|
|
|
|
pub enum BootstrapTargetSource {
|
|
PageContext,
|
|
DeterministicPlan,
|
|
SkillConfig,
|
|
Fallback,
|
|
}
|
|
```
|
|
|
|
This type should remain intentionally small.
|
|
|
|
It is **not** a generic execution-plan object. Its only job is to answer:
|
|
- what URL should the helper page bootstrap against on first submit?
|
|
- where did that value come from?
|
|
|
|
Keeping this object narrow avoids coupling callback-host bootstrap to unrelated execution details.
|
|
|
|
---
|
|
|
|
### 2. Move URL derivation into one resolver
|
|
|
|
Replace the current `initial_request_url_for_submit_task(...)` branching with a single resolver, conceptually:
|
|
|
|
1. If `SubmitTaskRequest.page_url` exists and is non-empty, use it.
|
|
2. Else attempt deterministic parsing through `decide_deterministic_submit(...)`.
|
|
- If it returns `Execute(plan)`, use `plan.target_url`.
|
|
3. Else inspect configured direct-submit skill metadata.
|
|
- If metadata exposes a bootstrap URL, use it.
|
|
4. Else return `about:blank`.
|
|
|
|
This keeps service bootstrap logic declarative and removes scene-specific guessing from `server.rs`.
|
|
|
|
---
|
|
|
|
### 3. Deterministic submit becomes the primary truth for line loss
|
|
|
|
`src/compat/deterministic_submit.rs` already contains structured line-loss routing data:
|
|
|
|
- `DeterministicExecutionPlan.expected_domain`
|
|
- `DeterministicExecutionPlan.target_url`
|
|
|
|
For line-loss requests, service bootstrap should use `plan.target_url` rather than reconstructing or hardcoding a URL in `server.rs`.
|
|
|
|
That means the current temporary branch:
|
|
|
|
```rust
|
|
if instruction.contains("线损") || instruction.contains("lineloss") {
|
|
return Some("http://20.76.57.61:18080".to_string());
|
|
}
|
|
```
|
|
|
|
should disappear entirely after the resolver is introduced.
|
|
|
|
This is the cleanest fix because the deterministic parser already owns the scene contract.
|
|
|
|
---
|
|
|
|
### 4. Skill metadata is the fallback, not the primary owner
|
|
|
|
For non-deterministic direct browser-script skills, service may still need a bootstrap URL even when there is no page context.
|
|
|
|
The fallback should come from skill-owned metadata with minimal fields:
|
|
|
|
- `bootstrap_url`
|
|
- `expected_domain`
|
|
|
|
Recommended semantics:
|
|
|
|
- `bootstrap_url`: the page URL service should use when opening the helper/bootstrap context
|
|
- `expected_domain`: the hostname direct runtime can use when page context is absent
|
|
|
|
This metadata should only be consulted **after** page context and deterministic parsing fail.
|
|
|
|
That preserves the user-selected policy:
|
|
- deterministic plan first
|
|
- skill metadata second
|
|
|
|
It also avoids forcing deterministic scenes to duplicate already-structured routing data in skill config.
|
|
|
|
---
|
|
|
|
## Ownership Boundary
|
|
|
|
### sgClaw owns bootstrap resolution policy
|
|
|
|
sgClaw should own the policy and precedence order for request URL resolution.
|
|
|
|
That includes:
|
|
- checking current page context first
|
|
- deciding when deterministic parsing is authoritative
|
|
- deciding when skill metadata is an allowed fallback
|
|
- falling back to `about:blank` when nothing else resolves
|
|
|
|
This policy belongs in sgClaw because it is part of submit-path orchestration, not part of an individual skill script.
|
|
|
|
### deterministic submit owns deterministic scene targets
|
|
|
|
If a scene already resolves to `DeterministicExecutionPlan`, then that plan owns:
|
|
- the authoritative `target_url`
|
|
- the authoritative `expected_domain`
|
|
|
|
Service should consume that plan rather than re-deriving equivalent information from raw text.
|
|
|
|
### skill metadata owns direct-skill bootstrap hints
|
|
|
|
When there is no deterministic plan, the skill package may own the minimal hints needed for bootstrap compatibility:
|
|
- `bootstrap_url`
|
|
- `expected_domain`
|
|
|
|
The skill should not own resolution precedence. It only provides data for the fallback tier.
|
|
|
|
---
|
|
|
|
## File-Level Design
|
|
|
|
### `src/service/server.rs`
|
|
|
|
Change responsibilities here from “derive request URL by ad hoc branch logic” to “ask the resolver for a bootstrap target”.
|
|
|
|
Expected changes:
|
|
- replace `initial_request_url_for_submit_task(...)` logic with a call into a resolver
|
|
- delete `derive_request_url_from_instruction(...)` or reduce it to thin legacy glue during migration
|
|
- remove the line-loss text-match hardcode entirely
|
|
- keep callback-host startup logic unchanged apart from where `bootstrap_url` comes from
|
|
|
|
### `src/compat/deterministic_submit.rs`
|
|
|
|
No routing policy should move into service from this module.
|
|
|
|
Expected role in the new design:
|
|
- continue producing `DeterministicExecutionPlan`
|
|
- expose enough information for service bootstrap resolution to reuse `plan.target_url`
|
|
- remain the source of truth for deterministic line-loss target selection
|
|
|
|
### `src/compat/direct_skill_runtime.rs`
|
|
|
|
This module currently resolves skill tool execution and derives `expected_domain` from task context.
|
|
|
|
For this slice, it does **not** need a full behavior rewrite.
|
|
|
|
Expected role:
|
|
- add or expose a helper for reading direct skill metadata if service needs it
|
|
- keep runtime execution behavior stable unless required by the new metadata seam
|
|
|
|
A later slice may allow runtime execution to use skill-owned `expected_domain` fallback too, but that is not required to land this service TODO.
|
|
|
|
### `src/config/settings.rs`
|
|
|
|
If sgClaw configuration needs to point to direct-skill metadata or enable fallback behavior explicitly, add the minimum structure here.
|
|
|
|
However, this slice should avoid creating a second parallel source of target URLs inside sgClaw config if the same information can be read from skill metadata.
|
|
|
|
The key rule is:
|
|
- do not replace one hardcode with a different hardcoded config map inside `settings.rs`
|
|
|
|
### Skill metadata loading seam
|
|
|
|
The design assumes a small read path that can answer:
|
|
- for the configured direct-submit skill, is there a `bootstrap_url`?
|
|
- is there an `expected_domain`?
|
|
|
|
The exact storage location can follow the existing staged-skill packaging model, but the new metadata should remain minimal and execution-adjacent rather than introducing a new wide dispatch schema.
|
|
|
|
---
|
|
|
|
## Data Flow
|
|
|
|
### Current desired flow
|
|
|
|
1. service receives `ClientMessage::SubmitTask`
|
|
2. service converts it to `SubmitTaskRequest`
|
|
3. service resolves `SubmitBootstrapTarget`
|
|
4. service passes `SubmitBootstrapTarget.request_url` into `LiveBrowserCallbackHost::start_with_browser_ws_url(...)`
|
|
5. callback-host bootstraps helper page using that URL
|
|
6. remaining task execution continues unchanged
|
|
|
|
### Resolution behavior examples
|
|
|
|
#### Case A: page context exists
|
|
- request includes `page_url=https://www.zhihu.com`
|
|
- resolver returns `PageContext`
|
|
- service uses that URL directly
|
|
|
|
#### Case B: line-loss deterministic request, no page context
|
|
- request has no `page_url`
|
|
- deterministic parser returns `Execute(plan)`
|
|
- resolver returns `DeterministicPlan` with `request_url=plan.target_url`
|
|
- service uses line-loss target URL from the plan
|
|
|
|
#### Case C: direct-submit skill with configured bootstrap URL, no page context, not deterministic
|
|
- request has no `page_url`
|
|
- deterministic parser returns `NotDeterministic`
|
|
- configured direct skill metadata exposes `bootstrap_url`
|
|
- resolver returns `SkillConfig`
|
|
|
|
#### Case D: nothing resolves
|
|
- no page context
|
|
- no deterministic plan
|
|
- no skill metadata bootstrap URL
|
|
- resolver returns `Fallback` with `about:blank`
|
|
|
|
---
|
|
|
|
## Testing Strategy
|
|
|
|
### Resolver-focused tests
|
|
|
|
Add focused tests covering precedence:
|
|
|
|
1. `page_url` wins over everything else
|
|
2. deterministic line-loss `Execute(plan)` wins when `page_url` is absent
|
|
3. skill metadata fallback is used only when no deterministic plan exists
|
|
4. `about:blank` remains the terminal fallback
|
|
|
|
### Regression coverage for the removed TODO
|
|
|
|
Add a regression proving that the service no longer depends on:
|
|
- `instruction.contains("线损")`
|
|
- `instruction.contains("lineloss")`
|
|
|
|
The line-loss bootstrap URL should now come from the deterministic plan only.
|
|
|
|
### Direct skill fallback tests
|
|
|
|
Add tests for:
|
|
- configured skill metadata with valid `bootstrap_url`
|
|
- missing `bootstrap_url`
|
|
- malformed `bootstrap_url`
|
|
- mismatch between metadata and current page context precedence
|
|
|
|
Malformed `bootstrap_url` metadata should be treated as unusable fallback data rather than a hard error for service bootstrap resolution:
|
|
- if page context exists, page context still wins
|
|
- if deterministic plan exists, deterministic plan still wins
|
|
- if malformed metadata is the only candidate, resolver should ignore it and fall through to `about:blank`
|
|
|
|
### Existing callback-host tests remain stable
|
|
|
|
Do not redesign callback-host behavior in this slice.
|
|
|
|
The callback-host tests should only need enough updates to reflect the new bootstrap URL source, not a new helper lifecycle contract.
|
|
|
|
---
|
|
|
|
## Migration Plan Shape
|
|
|
|
Recommended implementation order:
|
|
|
|
1. Introduce the bootstrap-target resolver and narrow result type.
|
|
2. Wire deterministic line-loss resolution into it using `DeterministicExecutionPlan.target_url`.
|
|
3. Remove the temporary line-loss hardcode from `server.rs`.
|
|
4. Add skill metadata fallback for configured direct-submit skills.
|
|
5. Expand tests to lock precedence and fallback behavior.
|
|
|
|
This order lands the TODO removal early without forcing the full fallback design to be implemented blindly first.
|
|
|
|
---
|
|
|
|
## Explicit Non-Goals
|
|
|
|
This slice does **not**:
|
|
- redesign callback-host lifecycle
|
|
- redesign deterministic scene parsing
|
|
- redesign direct-submit routing ownership
|
|
- introduce a broad scene registry for request URL derivation
|
|
- change browser command protocol
|
|
- rewrite direct skill execution behavior beyond what is needed for metadata lookup
|
|
- replace all current uses of page context with skill metadata
|
|
|
|
---
|
|
|
|
## Design Rule
|
|
|
|
For service bootstrap request URL resolution:
|
|
|
|
- current page context stays first
|
|
- deterministic execution plans are the authoritative source for deterministic scenes
|
|
- skill metadata provides a narrow fallback for non-deterministic direct skills
|
|
- `about:blank` remains the final fallback
|
|
- `src/service/server.rs` must not contain scene-specific text-match hardcodes such as the current line-loss TODO
|