fix: make property-assessor safer for whatsapp runs
This commit is contained in:
@@ -56,10 +56,14 @@ Rules:
|
||||
- Avoid fragile interactive gallery flows if they are likely to require approval or bounce the user into Control UI.
|
||||
- If a richer photo pass would require approval, do not silently force that path first. Continue with the best approval-free workflow available and clearly lower confidence if needed.
|
||||
- Only escalate to approval-heavy browser interaction when there is no reasonable alternative and the extra fidelity materially changes the assessment.
|
||||
- In WhatsApp or similar messaging runs, keep the core assessment on native chat-safe tools: `web_search`, `web_fetch`, and bounded `web-automation` browsing/extraction.
|
||||
- In those messaging runs, do **not** make `scripts/property-assessor assess`, `scripts/property-assessor locate-public-records`, `node zillow-discover.js`, `node har-discover.js`, `node zillow-photos.js`, `node har-photos.js`, `curl`, or `wget` the default core-analysis path.
|
||||
- From messaging runs, the only subprocess-style step you should attempt by default is the final `scripts/property-assessor render-report` call after the verdict, fair-value range, and report body are complete.
|
||||
- If the final PDF render fails, return the complete decision-grade report in chat and say the render/send step failed. Do not restart the whole assessment.
|
||||
- Do **not** run `npm install`, `npm ci`, or other dependency-setup commands during a normal chat assessment flow when the local skill already has `node_modules` present.
|
||||
- On this machine, treat `property-assessor` dependencies as already installed. Use the helper entrypoints directly unless the wrapper itself explicitly reports missing local dependencies.
|
||||
- Do **not** use ad hoc shell snippets, heredocs, or inline interpreter eval for public-record or CAD lookup from chat. Avoid forms like `python3 - <<'PY'`, `python -c`, `node -e`, `node --input-type=module -e`, or raw `bash -lc '...'` probes.
|
||||
- For public-record enrichment, use `scripts/property-assessor locate-public-records`, `scripts/property-assessor assess`, or `web_fetch`. If a one-off fetch helper is truly needed, add a file-based helper under the skill tree first and run that file-based entrypoint instead of inline code.
|
||||
- For public-record enrichment in chat, prefer `web_fetch` plus official assessor/CAD pages. If a one-off helper is truly needed, add a file-based helper under the skill tree first and use it from a non-messaging surface instead of inline code.
|
||||
|
||||
## Source order
|
||||
|
||||
@@ -75,6 +79,8 @@ Use `web_search` sparingly to discover alternate URLs, then return to `web-autom
|
||||
|
||||
## Helper runtime
|
||||
|
||||
These helper commands are primarily for local/manual runs, non-chat surfaces, and the final PDF render step. They are **not** the default WhatsApp core-analysis path.
|
||||
|
||||
`property-assessor` now includes TypeScript helper commands for:
|
||||
- address-first preliminary assessment assembly
|
||||
- public-record jurisdiction lookup
|
||||
@@ -98,7 +104,7 @@ scripts/property-assessor locate-public-records --address "<street-address>"
|
||||
scripts/property-assessor render-report --input "<report-payload-json>" --output "<output-pdf>"
|
||||
```
|
||||
|
||||
`assess` is the address-first entrypoint. It should:
|
||||
`assess` is the address-first entrypoint for helper-driven runs. It should:
|
||||
- require the assessment purpose
|
||||
- treat the assessment purpose as missing unless it is present in the current request or explicitly confirmed as unchanged from earlier context
|
||||
- resolve official public-record jurisdiction automatically from the address
|
||||
@@ -131,18 +137,18 @@ Default approach:
|
||||
|
||||
Approval-safe rule:
|
||||
- do not perform CAD/public-record discovery with inline shell or interpreter snippets
|
||||
- use the built-in TypeScript helper path first
|
||||
- if the official site needs a plain page fetch, prefer `web_fetch`
|
||||
- if rendered interaction is unavoidable, use a file-based `web-automation` helper rather than ad hoc shell text
|
||||
- in chat/messaging runs, prefer `web_fetch` plus official CAD/assessor pages first
|
||||
- use the built-in TypeScript helper path on local/manual surfaces or for the final PDF render step
|
||||
- if rendered interaction is unavoidable, use bounded `web-automation` behavior rather than ad hoc shell text
|
||||
|
||||
Use the helper CLI first:
|
||||
Use the helper CLI first on local/manual surfaces:
|
||||
|
||||
```bash
|
||||
cd ~/.openclaw/workspace/skills/property-assessor
|
||||
scripts/property-assessor locate-public-records --address "<street-address>"
|
||||
```
|
||||
|
||||
When you want the helper to assemble the preliminary assessment payload in one step, use:
|
||||
When you want the helper to assemble the preliminary assessment payload in one step from a non-messaging surface, use:
|
||||
|
||||
```bash
|
||||
cd ~/.openclaw/workspace/skills/property-assessor
|
||||
|
||||
37
skills/property-assessor/src/async-timeout.ts
Normal file
37
skills/property-assessor/src/async-timeout.ts
Normal file
@@ -0,0 +1,37 @@
|
||||
export class TimeoutError extends Error {
|
||||
readonly timeoutMs: number;
|
||||
|
||||
constructor(operationName: string, timeoutMs: number) {
|
||||
super(`${operationName} timed out after ${timeoutMs}ms`);
|
||||
this.name = "TimeoutError";
|
||||
this.timeoutMs = timeoutMs;
|
||||
}
|
||||
}
|
||||
|
||||
export async function withTimeout<T>(
|
||||
operation: () => Promise<T>,
|
||||
{
|
||||
operationName,
|
||||
timeoutMs
|
||||
}: {
|
||||
operationName: string;
|
||||
timeoutMs: number;
|
||||
}
|
||||
): Promise<T> {
|
||||
let timer: NodeJS.Timeout | undefined;
|
||||
|
||||
try {
|
||||
return await Promise.race([
|
||||
operation(),
|
||||
new Promise<T>((_, reject) => {
|
||||
timer = setTimeout(() => {
|
||||
reject(new TimeoutError(operationName, timeoutMs));
|
||||
}, timeoutMs);
|
||||
})
|
||||
]);
|
||||
} finally {
|
||||
if (timer) {
|
||||
clearTimeout(timer);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,5 +1,6 @@
|
||||
import { discoverHarListing } from "../../web-automation/scripts/har-discover.js";
|
||||
import { discoverZillowListing } from "../../web-automation/scripts/zillow-discover.js";
|
||||
import { TimeoutError, withTimeout } from "./async-timeout.js";
|
||||
|
||||
export interface ListingDiscoveryResult {
|
||||
attempts: string[];
|
||||
@@ -7,29 +8,65 @@ export interface ListingDiscoveryResult {
|
||||
harUrl: string | null;
|
||||
}
|
||||
|
||||
export async function discoverListingSources(address: string): Promise<ListingDiscoveryResult> {
|
||||
interface ListingDiscoveryDeps {
|
||||
timeoutMs?: number;
|
||||
discoverZillowListingFn?: typeof discoverZillowListing;
|
||||
discoverHarListingFn?: typeof discoverHarListing;
|
||||
}
|
||||
|
||||
const DEFAULT_DISCOVERY_TIMEOUT_MS = Number(
|
||||
process.env.PROPERTY_ASSESSOR_DISCOVERY_TIMEOUT_MS || 20_000
|
||||
);
|
||||
|
||||
export async function discoverListingSources(
|
||||
address: string,
|
||||
deps: ListingDiscoveryDeps = {}
|
||||
): Promise<ListingDiscoveryResult> {
|
||||
const attempts: string[] = [];
|
||||
let zillowUrl: string | null = null;
|
||||
let harUrl: string | null = null;
|
||||
const timeoutMs = deps.timeoutMs ?? DEFAULT_DISCOVERY_TIMEOUT_MS;
|
||||
const discoverZillowListingFn = deps.discoverZillowListingFn || discoverZillowListing;
|
||||
const discoverHarListingFn = deps.discoverHarListingFn || discoverHarListing;
|
||||
|
||||
try {
|
||||
const result = await discoverZillowListing(address);
|
||||
const result = await withTimeout(
|
||||
() => discoverZillowListingFn(address),
|
||||
{
|
||||
operationName: "Zillow discovery",
|
||||
timeoutMs
|
||||
}
|
||||
);
|
||||
zillowUrl = result.listingUrl;
|
||||
attempts.push(...result.attempts);
|
||||
} catch (error) {
|
||||
attempts.push(
|
||||
`Zillow discovery failed: ${error instanceof Error ? error.message : String(error)}`
|
||||
);
|
||||
if (error instanceof TimeoutError) {
|
||||
attempts.push(`Zillow discovery timed out after ${timeoutMs}ms.`);
|
||||
} else {
|
||||
attempts.push(
|
||||
`Zillow discovery failed: ${error instanceof Error ? error.message : String(error)}`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
const result = await discoverHarListing(address);
|
||||
const result = await withTimeout(
|
||||
() => discoverHarListingFn(address),
|
||||
{
|
||||
operationName: "HAR discovery",
|
||||
timeoutMs
|
||||
}
|
||||
);
|
||||
harUrl = result.listingUrl;
|
||||
attempts.push(...result.attempts);
|
||||
} catch (error) {
|
||||
attempts.push(
|
||||
`HAR discovery failed: ${error instanceof Error ? error.message : String(error)}`
|
||||
);
|
||||
if (error instanceof TimeoutError) {
|
||||
attempts.push(`HAR discovery timed out after ${timeoutMs}ms.`);
|
||||
} else {
|
||||
attempts.push(
|
||||
`HAR discovery failed: ${error instanceof Error ? error.message : String(error)}`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { extractHarPhotos } from "../../web-automation/scripts/har-photos.js";
|
||||
import { extractZillowPhotos } from "../../web-automation/scripts/zillow-photos.js";
|
||||
import { withTimeout } from "./async-timeout.js";
|
||||
|
||||
export type PhotoSource = "zillow" | "har";
|
||||
|
||||
@@ -19,12 +20,33 @@ export interface PhotoReviewResolution {
|
||||
discoveredListingUrls: Array<{ label: string; url: string }>;
|
||||
}
|
||||
|
||||
interface PhotoReviewDeps {
|
||||
timeoutMs?: number;
|
||||
extractZillowPhotosFn?: typeof extractZillowPhotos;
|
||||
extractHarPhotosFn?: typeof extractHarPhotos;
|
||||
}
|
||||
|
||||
const DEFAULT_PHOTO_EXTRACTION_TIMEOUT_MS = Number(
|
||||
process.env.PROPERTY_ASSESSOR_PHOTO_TIMEOUT_MS || 25_000
|
||||
);
|
||||
|
||||
export async function extractPhotoData(
|
||||
source: PhotoSource,
|
||||
url: string
|
||||
url: string,
|
||||
deps: PhotoReviewDeps = {}
|
||||
): Promise<PhotoExtractionResult> {
|
||||
const timeoutMs = deps.timeoutMs ?? DEFAULT_PHOTO_EXTRACTION_TIMEOUT_MS;
|
||||
const extractZillowPhotosFn = deps.extractZillowPhotosFn || extractZillowPhotos;
|
||||
const extractHarPhotosFn = deps.extractHarPhotosFn || extractHarPhotos;
|
||||
|
||||
if (source === "zillow") {
|
||||
const payload = await extractZillowPhotos(url);
|
||||
const payload = await withTimeout(
|
||||
() => extractZillowPhotosFn(url),
|
||||
{
|
||||
operationName: "Zillow photo extraction",
|
||||
timeoutMs
|
||||
}
|
||||
);
|
||||
return {
|
||||
source,
|
||||
requestedUrl: String(payload.requestedUrl || url),
|
||||
@@ -37,7 +59,13 @@ export async function extractPhotoData(
|
||||
};
|
||||
}
|
||||
|
||||
const payload = await extractHarPhotos(url);
|
||||
const payload = await withTimeout(
|
||||
() => extractHarPhotosFn(url),
|
||||
{
|
||||
operationName: "HAR photo extraction",
|
||||
timeoutMs
|
||||
}
|
||||
);
|
||||
return {
|
||||
source,
|
||||
requestedUrl: String(payload.requestedUrl || url),
|
||||
|
||||
32
skills/property-assessor/tests/timeout-guards.test.ts
Normal file
32
skills/property-assessor/tests/timeout-guards.test.ts
Normal file
@@ -0,0 +1,32 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { discoverListingSources } from "../src/listing-discovery.js";
|
||||
import { extractPhotoData } from "../src/photo-review.js";
|
||||
|
||||
test("discoverListingSources times out stalled Zillow and HAR discovery calls", async () => {
|
||||
const result = await discoverListingSources(
|
||||
"1011 Ennis Joslin Rd APT 235, Corpus Christi, TX 78412",
|
||||
{
|
||||
timeoutMs: 20,
|
||||
discoverZillowListingFn: async () => await new Promise(() => {}),
|
||||
discoverHarListingFn: async () => await new Promise(() => {})
|
||||
}
|
||||
);
|
||||
|
||||
assert.equal(result.zillowUrl, null);
|
||||
assert.equal(result.harUrl, null);
|
||||
assert.match(result.attempts.join(" "), /zillow discovery timed out/i);
|
||||
assert.match(result.attempts.join(" "), /har discovery timed out/i);
|
||||
});
|
||||
|
||||
test("extractPhotoData times out a stalled photo extraction instead of hanging forever", async () => {
|
||||
await assert.rejects(
|
||||
async () =>
|
||||
extractPhotoData("zillow", "https://www.zillow.com/example", {
|
||||
timeoutMs: 20,
|
||||
extractZillowPhotosFn: async () => await new Promise(() => {})
|
||||
}),
|
||||
/timed out/i
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user