Skip to content

Verify which script a Pine save actually wrote to - #514

Open
azuzubairezeasor-cmd wants to merge 2 commits into
tradesdontlie:mainfrom
azuzubairezeasor-cmd:fix/pine-save-target
Open

azuzubairezeasor-cmd wants to merge 2 commits into
tradesdontlie:mainfrom
azuzubairezeasor-cmd:fix/pine-save-target

Conversation

@azuzubairezeasor-cmd

Copy link
Copy Markdown

Fixes #513.

The problem

TradingView keeps the Pine Editor bound to exactly one saved script. pine_open and pine_new only call monaco.setValue() — that replaces the editor text but does not move the binding. A save issued afterwards therefore writes to whichever script was bound before, silently, and is reported as success.

openScript made this harder to notice by returning script_id — the id it had just fetched from — which reads like confirmation that the editor is now bound to that script.

I hit this in real use: opened script A, injected source, saved, and watched script B get overwritten. Nothing in the tool results indicated anything was wrong.

What this does

Verifies the save target. The binding isn't readable from the page, so save() and smartCompile() now snapshot the saved-script list before and after and report which script actually changed. If it isn't the one the caller opened, it throws — naming the overwritten script and its new version, so it can be restored from version history. A silent overwrite becomes a loud, recoverable error.

Both paths needed it: smartCompile clicks the editor's own Save button, so it writes to the same bound script pine_save does.

force: true skips the check for callers that deliberately want to write to whatever is bound.

Fixes name resolution. The match loop tested scriptName and scriptTitle with equal precedence in list order:

if (sn === target || st === target) { match = scripts[i]; break; }

A script whose title collided with the query could win over the one actually named it. Concretely, pine_open({name: "My script"}) returned a script named Azu's Gold Sweeps v2, because its title happened to be "My script" and it sorted earlier. Precedence is now exact name → exact title → substring on each, and the result reports matched_on.

Combined with the save bug, that pair is how a user overwrites a script they never named.

Stops pine_new implying it created something. It doesn't — it blanks the current script's buffer. The action is now editor_buffer_reset with a warning, and it marks the pending save so the verification catches the overwrite rather than letting it through.

Notes

  • Resolution moved out of the injected page script into Node so it's unit testable. resolveScriptMatch and diffScriptVersions are pure and exported.
  • 10 new tests, including a regression test for the exact title collision above. node --test tests/pine_analyze.test.js tests/pine_save_target.test.js → 26/26 pass.
  • The verification is post-hoc: it detects the wrong write rather than preventing it, because the target can't be known beforehand. Preventing it properly means driving TradingView's own Open / Save as flow so the binding actually moves — worth doing, but a larger change than this.
  • Unrelated, but worth flagging: npm test on main runs tests/e2e.test.js against a live TradingView with no opt-in guard. Running the suite drove my real chart. fix/drawing-di-resolve appears to address this already.

🤖 Generated with Claude Code

azuzubairezeasor-cmd and others added 2 commits September 8, 2026 23:40
TradingView keeps the Pine Editor bound to one saved script. pine_open and
pine_new only call monaco.setValue(), which replaces the editor text but does
not move that binding, so a subsequent save lands on whichever script was
bound before — silently, and reported as success. openScript even returned the
script_id it fetched from, which reads as confirmation of a binding that never
happened.

The binding is not readable from the page, so this snapshots the saved-script
list around every save and reports which script actually changed. A mismatch
against the script the caller opened now throws, naming the script that was
overwritten and its new version so it can be restored from version history.
That converts a silent overwrite into a loud, recoverable error. Pass
force:true to write to whatever the editor is bound to.

Also fixes name resolution in openScript. The match loop tested scriptName and
scriptTitle with equal precedence in list order, so a script whose *title*
collided with the query could win over the one actually *named* that —
pine_open({name: "My script"}) opened a script named "Azu's Gold Sweeps v2"
because its title was "My script". Precedence is now exact name, exact title,
then substring on each, and the result reports which field matched.

newScript keeps its behaviour but stops implying it created anything: the
action is now editor_buffer_reset and it carries a warning, and it marks the
pending save so verifySaveTarget catches the overwrite rather than allowing it.

Resolution moved from the injected page script into Node so it is unit
testable; adds tests covering the title-collision regression and the diff.

Fixes tradesdontlie#513

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Pine Editor's title button reports the script the editor is actually
bound to, independent of the text buffer. That is the binding pine_open
cannot move, and reading it turns the previous post-hoc check into a
pre-flight guard: save() and smartCompile() now throw before writing
anything if the editor is bound to a different script than the caller
opened, naming the script that would have been clobbered.

The saved-script diff stays as a second net for when the control cannot
be read.

openScript now reads the binding back after injecting and reports
editor_bound_to and save_target_moved, instead of returning a script_id
that implies a rebind that never happened. When the editor already
happened to be bound to the requested script there is nothing to guard
against, so no warning is emitted.

pine_new + save is now refused outright rather than silently overwriting
whatever was bound. force:true still writes to the bound script.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@azuzubairezeasor-cmd

Copy link
Copy Markdown
Author

Pushed be5cac0 — this upgrades the fix from detecting a wrong write to preventing one.

What changed

While digging into whether the binding could be moved programmatically, I found it can at least be read: the Pine Editor's title button ([class*="nameButton"]) reports the script the editor is actually bound to, independent of what's in the text buffer.

That's the missing piece. save() and smartCompile() now do a pre-flight check and throw before dispatching anything if the editor is bound to a different script than the caller opened — naming the script that would have been overwritten. The saved-script diff stays as a second net for when the control can't be read.

openScript now reads the binding back after injecting and returns editor_bound_to and save_target_moved, rather than a bare script_id that implies a rebind that never happened. If the editor already happened to be bound to the requested script, there's nothing to guard against and no warning is emitted.

pine_new + save is refused outright now instead of silently overwriting the bound script.

On the real rebind

I tried to make pine_open drive TradingView's own flow so the binding genuinely moves, and got most of the way:

  • Script menu is [class*="nameButton"]; its items are Create new, Make a copy…, Rename…, Open script… (⌘O)
  • The dialog is [data-name="open-user-script-dialog"], with a search input and rows exposing [data-name="open-script-dialog-item-name"]
  • Each row's React fiber carries onClick plus a scriptItem prop with the correct {id, name}

Filtering the list works. Activating a row does not. I tried a synthetic .click(), a full pointerdown/mousedown/pointerup/mouseup/click sequence, trusted CDP Input.dispatchMouseEvent (single and double) at coordinates verified with elementFromPoint, and invoking the fiber's onClick directly with a synthesised SyntheticEvent — that last one gets as far as failing on an internal openAction lookup. The editor had unsaved buffer changes throughout, which may be the blocker, but I couldn't confirm it.

So the rebind stays manual. Anyone who knows how that dialog commits a selection could finish it — the selectors above are the hard part, and they're all stable data-name attributes rather than hashed classes.

🤖 Generated with Claude Code

@azuzubairezeasor-cmd

Copy link
Copy Markdown
Author

Correction to my previous comment: row activation in the Open-script dialog does work. I said it didn't. It does — it's just asynchronous with a long delay, and I checked too soon.

What I saw: [data-name="open-script-dialog-item-name"].click() appeared to do nothing across several immediate re-reads, so I concluded the handler wasn't firing. Some minutes later the binding had in fact moved — [class*="nameButton"] read new code, the script I had clicked. The click landed; my polling window was far too short.

That changes the outlook for a proper rebind. The flow is:

  1. click [class*="nameButton"] to open the script menu
  2. click the item whose text starts with Open script
  3. wait for [data-name="open-user-script-dialog"]
  4. set the search input's value via the native setter and dispatch an input event
  5. click the matching [data-name="open-script-dialog-item-name"]
  6. poll [class*="nameButton"] until it reads the target name — this is the step I got wrong; it needs a generous timeout, not a fixed short wait

Step 6 is also the completion signal, so the whole thing is verifiable rather than fire-and-forget. Make a copy… and Create new sit in the same menu and would give genuine save-as / new-script support.

I have not implemented this — the guard in this PR still assumes the binding cannot be moved, which remains correct and safe behaviour. But it means a real fix is reachable and the earlier pessimism in this thread was mine, not the platform's.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pine_open / pine_new don't rebind the editor — saves silently overwrite the wrong script

1 participant