fix(flags): stop reporting success for a flag that cannot take effect on this host - #651
Open
filip131311 wants to merge 2 commits into
Open
fix(flags): stop reporting success for a flag that cannot take effect on this host#651filip131311 wants to merge 2 commits into
filip131311 wants to merge 2 commits into
Conversation
… here `argent enable argent-lens` printed "Enabled flag" on Linux and changed nothing. The Lens tools are registered only on macOS — deliberately, since the preview window drives Terminal and the simulator stream through macOS-only paths — but the flag registry carried no platform information, so neither `enable` nor `argent flags` could say so. The flag's own description reads "off by default while the feature is in development", which invites exactly the attempt. A flag can now declare the platforms its feature exists on. Enabling one elsewhere still stores it and still exits 0 — the setting is portable, may be committed, and is often authored for a teammate or a CI host that does support it — but a note on stderr says plainly that nothing changes here. It claims nothing about other machines: with a global scope, or a client linked to a remote tool-server, this host cannot know which machine will read the setting. `argent flags` marks the constraint beside the state word that was doing the misleading, and the help text carries it too. `flags --json` reports the platforms rather than a "supported here" boolean, which would be wrong as soon as the output were read anywhere else. Reads stay platform-blind, so a flag committed by a teammate on macOS behaves identically here — a test pins that, since it is the reason not to make `isFlagEnabled` platform-aware. The registration gate keeps its own explicit platform check rather than deriving one from the registry: a missing entry would then register the tools everywhere, which is precisely backwards for a gate that exists to keep them off. A test pins the two together instead. The Lens skill told the model to enable the flag and retry when the tools came back not-found — on Linux that never terminates. It now states the platform in the line a router reads, and gives the terminal condition. Fixes #642
…atform The skill-description gate requires a capital + verb opening; leading with the platform failed it. The constraint moves into the same sentence instead, so a router still sees it before invoking the skill.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #642.
Reproduced on a real Linux VM (Ubuntu aarch64, Node 20, published
0.18.0), not simulated:After, on the same VM:
The gap
The darwin registration gate is deliberate and well-commented. What was missing is that
FLAG_REGISTRYcarried no platform information, so the CLI could not know. A flag can now declareplatforms, andargent flags/enablerender from it.Note
argent lensalready told the truth on Linux ("macOS-only — it drives Terminal/iTerm viaosascript"). That asymmetry is the whole bug.
Decisions a reviewer should weigh
Warn and store, exit 0 — not refuse.
enableauthors portable state rather than performing anaction:
flags.jsongets committed, and a Linux teammate must be able to author project-scope configfor macOS colleagues. Refusing would also turn a silent no-op into a hard failure for any shared
bootstrap script running under
set -e— a behaviour regression well beyond a message fix. Thecounter-argument (exit 2 is unambiguous and machine-readable) is real; I weighted the regression
higher for a truthfulness fix.
The note claims nothing about other machines. An earlier draft ended "…and will apply on macOS."
That is false in the default global scope —
~/.argent/flags.jsonon this Linux box will never beread by a mac — and wrong again under
argent link, where registration and flag reads happen on theremote host. The note now states only what is true of the host in front of you.
Reads stay platform-blind, permanently.
isFlagEnabledsits in the per-request exposure path andits package is documented as having no console I/O; more importantly, a flag committed by a macOS
teammate must behave identically here. There is a named test pinning this so a later "improvement"
gets caught with its reason attached.
The registration gate is NOT derived from the registry, though that was tempting for
single-source-of-truth. Deriving fails in the wrong direction:
getFlagDefinitionreturns undefinedonce the flag graduates and its entry is removed — and this flag's own description says "while the
feature is in development", so removal is the expected end state — which would register the Lens
tools on every platform, exactly what the gate exists to prevent. The two statements are pinned
together by a test that asserts registration matches the declared platforms across darwin/linux/win32.
flags --jsonreportsplatforms, notsupportedOnThisPlatform. A per-host boolean encodesprocess state as flag data and is wrong the moment the output is read elsewhere or under
link.Additive field; the one in-repo consumer is a test, updated here.
Also fixed: an infinite loop in the Lens skill
skills/argent-lens/SKILL.mdtold the model: if the tools come back not-found, the flag is off —enable it and retry. On Linux enabling never creates them, and there was no terminal condition. It
now states the platform in the frontmatter
description(the line a router reads before invokingthe skill, so a Linux agent isn't handed it in the first place) and gives the exit condition: do not
retry, tell the user.
Verification
Linux VM: exit 0 with the Enabled line on stdout and the note on stderr;
flags.jsonwritten;flagsbadge;--jsonplatforms: ["darwin"]vsnullfor an unconstrained flag;tools | wc -lstill 75, proving registration was untouched;
disablesilent; an unconstrained flag gets no note.macOS:
enablesilent on stderr, exit 0, both Lens tools still registered, badge renders[macOS only]with no "unavailable" suffix.configuration-core 93 passed, argent-cli 285 passed, tool-server 3087 passed.
Unchanged and still passing:
registry-feature-flag-gate.test.ts,variant-flag-gate.test.tsand theHTTP exposure tests — exposure semantics do not change.
🤖 Generated with Claude Code