fix windev

[jnsh-rf]

Type of Change

• New feature
• Bug fix
• Documentation update
• UI/UX improvement

Description

[GabiNun2]

this won't fix #4646 and even if it did there is already a open pr to fix it and changing winutil dev to accept args is good but it does it in a bloated way and can be improved

[jnsh-rf]

Not really, because if current version is run without Admin privs, it didn't run pre-release, only the latest stable release. With this changes (beside making params work) if you run irm https://christitus.com/windev | iex as is, it effectively run

Invoke-RestMethod -Uri https://github.com/ChrisTitusTech/winutil/releases/download/$latestTag/winutil.ps1 | Invoke-Expression

and passes tag so it actually run the pre-release

[FatBastard0]

The great and glorious AI at Google He/her/dog's telling me to do this

$latestTag = (Invoke-RestMethod https://api.github.com/repos/ChrisTitusTech/winutil/tags).Name | Select-Object -First 1
$scriptString = Invoke-RestMethod -Uri "https://github.com/ChrisTitusTech/winutil/releases/download/$latestTag/winutil.ps1"
& ([scriptblock]::Create($scriptString)) -Preset Standard

[jnsh-rf]

in this PR -Preset Standard is put inside of @PSBoundParameters and passed into winutil.ps1, and if there was a typo, it will be discarded and open GUI as normal

so, the code AI gave you is valid, my code is just more generic

[jnsh-rf]

Not really, because if current version is run without Admin privs, it didn't run pre-release, only the latest stable release. With this changes (beside making params work) if you run irm https://christitus.com/windev | iex as is, it effectively run

Invoke-RestMethod -Uri https://github.com/ChrisTitusTech/winutil/releases/download/$latestTag/winutil.ps1 | Invoke-Expression

and passes tag so it actually run the pre-release

In non-Admin terminal window, irm https://christitus.com/windev | iex will download latest pre-release (still not elevated), goes into the elevation block, download and run stable release (now elevated). This does not happen in Admin terminal window.

[GabiNun2]

Not really, because if current version is run without Admin privs, it didn't run pre-release, only the latest stable release. With this changes (beside making params work) if you run irm https://christitus.com/windev | iex as is, it effectively run

Invoke-RestMethod -Uri https://github.com/ChrisTitusTech/winutil/releases/download/$latestTag/winutil.ps1 | Invoke-Expression

and passes tag so it actually run the pre-release

what?, again this pr won't fix the issue

[jnsh-rf]

There are 2 bugs. First one masks the second. The logic that comes from start.ps1 have a bug that happens in non-Admin window ONLY when running irm https://christitus.com/windev | iex.
First one, regression cause by ValidateSet (exposed a bug/design oversight). irm https://christitus.com/windev | iex redirects to windev.ps1 in HEAD of main, that one takes the winutil.ps1, loads it to memory as a string (irm) that is piped into function that executes it (iex). Because it is executed as is (without params provided, form irm ... | iex does not expose convenient way to pass them, so none tried/tested that), it is equivalent to running .\Compile -Run or .\Compile.ps1 and .\winutil.ps1 without parameters in local clone. Direct .\winutil.ps1 with malformed params would launch GUI because Invoke-Expression treats params by position, so if one is not correct it would ignore it (see .\winutil.ps1 -Preset ThatShouldFail opening GUI when there is no ValidateSet, and failing with winutil.ps1: Cannot validate argument on parameter 'Preset'. The argument "ThatShouldFail" does not belong to the set "Standard;Minimal;Advanced" specified by the ValidateSet attribute. Supply an argument that is in the set and then try the command again. when there is). iex sees, that it is incorrect and ignores it, ValidateSet checks if it is correct, if not, rather than giving nothing, it leaves " " (space) in position where -Preset PRESET should be, thus iex cannot parse it because it got inforemed that param is correct, but it sees incorrect (empty param), so it does not know what to do and fails (see stack trace). That what can be seen running irm https://christitus.com/windev | iex in #4646. Adding param() block ensures that space is passed without eval, [scriptblock]::Create($scriptString) safely passed for evaluation and @PSBoundParameters passes args by value not position (as @args would do) so they did not drop (space is skipped).
Second bug (in start.ps1), if there is no param validation (pre #4539) - applicable only to running irm https://christitus.com/windev | iex from window that does not have Admin privs. It does not happen in window with Admin, or local file.
Terminal with Admin would download latest tagged version (get latest tag -> download release with that tag, either release or pre-release; second oversight (which this PR does not address)) - what if pre-release was removed, but corresponding tag was not (tag 26.05.19 exists btw). Because it has prior Admin privs (global scope) the block for non-Admin is not evaluated, so it just works (see version in About). Local version in Admin window skip that block as well. The non-Admin local goes into block, reads params, then check if its local file

$script = if ($PSCommandPath)` and beacuse it is `"& { & `'$($PSCommandPath)`' $($argList -join ' ') }"

adds params for execution, then executes with admin privs:

    if ($processCmd -eq "wt.exe") {
        Start-Process $processCmd -ArgumentList "$powershellCmd -ExecutionPolicy Bypass -NoProfile -Command `"$script`"" -Verb RunAs
    } else {
        Start-Process $processCmd -ArgumentList "-ExecutionPolicy Bypass -NoProfile -Command `"$script`"" -Verb RunAs
    }

If you try to run stable irm https://christitus.com/win | iex, with Admin block is skipped, without Admin goes, it is not local so goes to else path

    $script = if ($PSCommandPath) {
        "& { & `'$($PSCommandPath)`' $($argList -join ' ') }"
    } else {
        "&([ScriptBlock]::Create((irm https://github.com/ChrisTitusTech/winutil/releases/latest/download/winutil.ps1))) $($argList -join ' ')"
    }

else path re-download stable release so from user perspective everything works (you download the file twice - yet another oversight). Then it elevates. You see correct version in About.
But if you try to run irm https://christitus.com/windev | iex in non-Admin, it goes to windev.ps1 in HEAD of main, load file directly into memory, evaluate via iex it reaches the elevation (Admin) check, goes into block, as it is not a local file it goes into else path

    $script = if ($PSCommandPath) {
        "& { & `'$($PSCommandPath)`' $($argList -join ' ') }"
    } else {
        "&([ScriptBlock]::Create((irm https://github.com/ChrisTitusTech/winutil/releases/latest/download/winutil.ps1))) $($argList -join ' ')"
    }

downloads latest stable (effectively ignoring latest tag), and runs it

    if ($processCmd -eq "wt.exe") {
        Start-Process $processCmd -ArgumentList "$powershellCmd -ExecutionPolicy Bypass -NoProfile -Command `"$script`"" -Verb RunAs
    } else {
        Start-Process $processCmd -ArgumentList "-ExecutionPolicy Bypass -NoProfile -Command `"$script`"" -Verb RunAs
    }

You can see it via version in About page. See screenshots provided.
So, it works on local, on stable (with or without prior Admin privs) or pre-release with prior Admin. But not on non-Admin unstable, because all that logic is technically valid from PowerShell perspective and it all run directly in memory so you cannot check it.
The only way to see that wrong behaviour, is to either test it on git state prior to #4359 or with ValidateSet removed WITH windev.ps1 from this PR (test code is provided). Trying to run irm https://christitus.com/windev | iex or as Gabi tried via gist will hit the logic described above so it will just fail (on the current state of main).
Provided test code, when run in non-Admin window (in my testing it was inside test.ps1; screenshots provided) inside of local winutil clone, no files modified, it finds latest tag, passes it to process' env for storage (to not be dropped), launches local .\winutil.ps1 (-Raw is required as irm passes a raw string, and direct Get-Content does stuff line by line; Get-Content with -Raw gives the same output as irm), executes it as it, it hits the Admin check, goes into it, it is local so it becomes one to execute, executes with elevation - bug does not show. Equivalent to running .\winutil.ps1 with or without prior Admin.
If used outside of clone in non-Admin window, it does not find local file, it downloads latest release, loads into memory and executes. Fails Admin check, and as it is not local and thus path to it, you get

 $script = "&([ScriptBlock]::Create((irm https://github.com/ChrisTitusTech/winutil/releases/latest/download/winutil.ps1))) $($argList -join ' ')"` 

that get later executed with Admin privs. See version in About or in screenshot. This effectively emulates working windev.ps1 in main run via irm https://christitus.com/windev | iex.
If run you run .\Compile.ps1 with this changes and then .\winutil.ps1 in non-Admin, it saves latest tag to process env storage, when it fails Admin check, it correctly reads latest tag, as latest tag exist it goes into $latestTag path, and what will get elevated with Admin privs is this:

 $script = "&([ScriptBlock]::Create((Invoke-RestMethod -Uri https://github.com/ChrisTitusTech/winutil/releases/download/$latestTag/winutil.ps1))) $($argList -join ' ')"

[GabiNun2]

@jnsh-rf
brother in christ i am not reading all of that

[jnsh-rf]

Then dont bitch about not understanding a problem that happens under very specific condition, that normal testing can't catch, and you dint bother to understand. And the bug you caused by not testing if windev would work with your changes.

[GabiNun2]

Then dont bitch about not understanding a problem that happens under very specific condition, that normal testing can't catch, and you dint bother to understand. And the bug you caused by not testing if windev would work with your changes.

#4657

[jnsh-rf]

If the simplified version in that PR works, why don't you suggest changes here or send PR to my branch?

[GabiNun2]

If the simplified version in that PR works, why don't you suggest changes here?

because you are doing a bunch of unneccary things and are trying to fix the issue that #4651 already fixes and which btw your pr dosen't even fix the issue unlike #4651

also just to show why this pr won't fix the issue

[GabiNun2]

If the simplified version in that PR works, why don't you suggest changes here or send PR to my branch?

also i can't make prs to forks (unless you know a way beause last time someone tried to do so for me it didn't work for them)

[jnsh-rf]

you are doing a bunch of unneccary things

send patches, suggest changes or do PR against my branch

trying to fix the issue that #4651 already fixes

by removing something useful

your pr dosen't even fix the issue unlike #4651

[GabiNun2]

i think you should close this pr it's just bloat

[jnsh-rf]

I'll wait for Chris to decide.

[GabiNun2]

I'll wait for Chris to decide.

chris will close this because you are doing things that are unrelated to the pr and saying you fixed an issue when you didn't and after all of that your just adding bloat when another pr adds the same feature without creating a env varible for no reason lol

[jnsh-rf]

Why does it not work? Where does it fail? Where can it be improved? And most important - do you actually read what I wrote? If you want to me to know that something is wrong, point to it, explain and show better solution. Don't shout "its bad because its bad", take my work / investigation, and sell as yours. Unless you have something of value to add, don't bother responding.

[GabiNun2]

Why does it not work? Where does it fail? Where can it be improved? And most important - do you actually read what I wrote? If you want to me to know that something is wrong, point to it, explain and show better solution. Don't shout "its bad because its bad", take my work / investigation, and sell as yours. Unless you have something of value to add, don't bother responding.

okay first problem
in the pr description you wrote Fix issue #4646 which is not true, the problem was with the ValidateSet which this pr dosen't remove if you want more infromation on why ValidateSet was the problem look at #4651 which btw was created before this pr which means you are duping prs

second problem
you are adding bloat to the utility by first duping the problem of #4646 by adding an extra ValidateSet

than you are adding a envrioment varible to the system to detact if winutil was launched with windev which is uneneccary you could just do a $windev = $true than mention $windev in the $script = if of the relaunch like how pr #4657 does so

[jnsh-rf]

if you want more infromation on why ValidateSet was the problem look at #4651

This PR isn't a duplicate. While #4651 targets the parameter error inside the core utility, this one fixes the broken windev so can actually pass params safely during pre-release debugging (which would prevent that whole incident to begin with)

my code does not pipe it via iex until it is required, it passes the code till its the moment to evaluate it (which i already described)

this PR is based on main, preserving ValidateSet is intended, btw @args is prone to the same breakage that iex

you are adding bloat to the utility by first duping the problem of #4646 by adding an extra ValidateSet

thats why code looks like this so it safely passes arguments; if wrong it wont break everything at some random point in execution chain

$latestTag = (Invoke-RestMethod https://api.github.com/repos/ChrisTitusTech/winutil/tags).Name | Select-Object -First 1
$scriptString = Invoke-RestMethod -Uri https://github.com/ChrisTitusTech/winutil/releases/download/$latestTag/winutil.ps1
$scriptBlock = [scriptblock]::Create($scriptString)
& $scriptBlock @PSBoundParameters

one of benefits of this approach is windev-only params, like specific tags

than you are adding a envrioment varible to the system to detact if winutil was launched with windev which is uneneccary you could just do a $windev = $true than mention $windev in the $script = if of the relaunch like how pr #4657 does so

simple local variable like $windev = $trueworks fine if you are already running as an Administrator. Second bug happen when windev is called from non-Admin window which your code does not adress

$env:WINUTIL_DEV_TAG = $latestTag is needed, as name of the version tag itself must cross the administrator gap

Because WinUtil requires root privileges, the script immediately runs its admin check. Since the user is not an administrator, it prepares to launch a brand-new, elevated PowerShell window. When that new administrative window opens, it starts fresh RAM state with no memory of the variables or code that were running in the previous non-admin window. That's why it is hard to replicate.

Without the exact version string (like 26.06.04), the elevated script cannot build the download URL. It would either have to hit the GitHub API a second time inside the admin window (adding delay), or it would completely break. We would need to add more BLOAT.

[GabiNun2]

you clearly don't seem to understand so let me make it clear for you WinUtill will not launch using the irm | iex command while having ValidateSet it will simplify not happen as you can see in the description of #4651

also this is incorrect

it will be able to relaunch the window because it will have a hardcoded version like 26.06.04 from the $latestTag varible so it does work, do you per change know how i know it works? yes thats right i tested it unlike you because if you were to test this you will see that it dosen't work

[vyas-devgna]

Hey, the effort here is clear and the bug hunting is genuinely appreciated, but @GabiNun2 is right and it's worth understanding why before moving forward. The ValidateSet you added to windev.ps1 causes the exact same crash that started #4646 because users pipe windev.ps1 through iex with the standard command, and ValidateSet breaks under iex no matter where it sits, so the issue is still there just moved one level up. The env variable is also doing nothing useful because $latestTag is already in scope when the relaunch command string gets built, meaning the tag is naturally baked into that string before the new window ever opens, which is exactly what #4657 demonstrates with a much simpler approach. On the description, it is genuinely one of the longest PR descriptions in this repo for a change that touches three files, and while the investigation behind it is solid, writing five paragraphs of long and short versions, stack traces, and test harnesses when the core idea could have been explained in a few sentences makes it really hard for maintainers and contributors to engage with it, which is probably part of why this conversation went sideways. @GabiNun2 has been around this codebase long enough to know when something is adding unnecessary complexity and when a simpler path already exists, and in this case he is pointing at #4651 and #4657 which together cover everything this PR is trying to do without the contradictions. The best move here is to close this, let those two go through, and if you want to contribute the @PSBoundParameters improvement over @Args drop it as a small suggestion on #4657. Keep the next PR tight, explain only what changed and why, and when someone like @GabiNun2 points at a simpler solution it is worth slowing down and actually reading it before pushing back. Also on using AI for this kind of thing, it is a great tool for investigating bugs and drafting PRs but you have to actually verify what it gives you manually before putting it up, AI will confidently give you a solution that looks right but has the exact contradictions people pointed out here, make it dig deeper, question its own output, and always test before trusting it. The PR also breaks a few contributions guidelines directly. It should be focused on a single fix but the title alone mentions two separate things, both the bug and new feature boxes are checked, and the guidelines are clear that those should be separate PRs. The Compile.ps1 capitalisation change is an unrelated file edit which the guidelines say to avoid. On testing, the guidelines say to run .\Compile.ps1 -Run and verify nothing breaks before submitting, but a custom test script was used instead and @GabiNun2 showed the standard user command still crashes when tested properly. The description also has a caution note telling reviewers to read it fully which is a sign it went way beyond what the guidelines ask for, which is just explain what changed and why.

[vyas-devgna]

if you wanna use AI to implement a fix to any bug you found or want to solve or a new feature idea you wan to implement, going further you can use this prep prompt before staring and it will make the AI work better , using AI is not wrong but not using it right is what people fallback on

You are a senior open source contributor and code reviewer with a reputation for shipping minimal, correct, well-reasoned changes. When given any programming task involving a repository, you do not write a single line of code until you have fully read the contribution guidelines, scanned all related open and closed issues, reviewed relevant PR descriptions, code diffs, and every comment thread touching the area in question, and understood what has already been tried, what was rejected, and why. You build a complete picture of the codebase's conventions, the maintainer's preferences, and the existing work in flight before forming any opinion. You never rush to a solution to satisfy the user and you never trust your first instinct without verifying it against the actual execution flow and repo context. If a simpler solution already exists or is in progress you say so clearly instead of proposing something new. When you do propose something you explain exactly what changed, why it is the minimal correct fix, and what you verified to be sure nothing else breaks. You communicate everything in plain simple language, well structured and easy to follow, without walls of text, without speculative future justifications, and without complexity that was never asked for. If you are not certain about something you say so and investigate further rather than generating a confident answer that falls apart under scrutiny.

[GabiNun2]

@vyas-devgna i couldn't have said it any better thank you

[GabiNun2]

also i did close #4657 because it was to niche because the only way someone will get this problem is if the user isn't running and admin using the dev branch

since there is less than 20k users running windev and most of them are developers aka know what they are doing we shouldn't have this feature also if they are not running as admin it's not like it will fail it will just go to the main release so the user can still use winutil

which has only been 1 person that we know of doing that out of millions so this will just be unneccary code

[vyas-devgna]

also i did close #4657 because it was to niche because the only way someone will get this problem is if the user isn't running and admin using the dev branch

since there is less than 20k users running windev and most of them are developers aka know what they are doing we shouldn't have this feature also if they are not running as admin it's not like it will fail it will just go to the main release so the user can still use winutil

which has only been 1 person that we know of doing that out of millions so this will just be unneccary code

that's fair