fix: clone script by path instead of hash

[pyranota]

No description provided.

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	7caafe3
Status:	✅  Deploy successful!
Preview URL:	https://5d6ac62a.windmill.pages.dev
Branch Preview URL:	https://clone-script-by-path.windmill.pages.dev

View logs

[claude]

Claude finished @pyranota's task in 1m 52s —— View job

---

PR Review: Clone Script by Path Instead of Hash

• Gather context and understand the changes
• Review code quality and best practices
• Check for potential bugs or issues
• Analyze performance and security implications
• Provide detailed feedback

---

Summary

This PR changes the clone_script and fetch_script_for_update functions to look up scripts by path instead of hash. This is a sensible change that improves reliability when cloning scripts during dependency resolution jobs.

What Changed

1. fetch_script_for_update (scripts.rs:913-967):

   • Changed the parameter from hash: ScriptHash to path: &str
   • Modified the SQL query to select by path with ORDER BY created_at DESC LIMIT 1
   • This ensures the latest non-archived version at that path is fetched

2. clone_script (scripts.rs:975-1080):

   • Now accepts path: &str instead of base_hash: ScriptHash
   • Uses s.hash (the hash from the fetched script) for all subsequent operations
   • The parent_hash is correctly set from the fetched script's hash

3. clone_runnable (jobs.rs:2765-2770):

   • Updated to pass j.runnable_path() instead of base_hash

Review Feedback

Overall: The change looks correct and well-implemented. Looking up by path instead of hash is more robust because:

• It always gets the latest version at a path
• It avoids potential issues with stale hash references

Minor suggestions:

1. I left an inline comment suggesting the error message include the path for better debugging.

2. The formatting change on line 3004 in jobs.rs is unrelated to this fix (whitespace-only change) - this is fine but worth noting it's just a style cleanup.

Testing Instructions

To verify these changes, you need to test the script dependency resolution flow. This is triggered when a script imports another script using relative imports and the imported script's dependencies need to be resolved. The flow involves:

Navigate to a workspace and create a TypeScript/Deno script that imports from another script in the workspace using relative imports (e.g., import { something } from "./another_script"). Save the script to trigger a dependency resolution job. The fix ensures that when cloning the script during dependency resolution, the system correctly fetches the latest non-archived version by path rather than potentially failing due to hash mismatches.

---

[pyranota]

/updatesqlx

[windmill-internal-app]

Starting sqlx update...

View workflow run