P3 [CRF-2] validateAgentRuntimeHours accepts allocation=0 with a positive hard_limit, minting a feature that reports Enabled: false yet advertises a non-zero enforcement ceiling. (Knov P3, Mafuuu P3)

With allocation=0 this reduces to hard < 0, so any hard >= 0 is accepted, including hard=1000.

The soft check is symmetric (soft < 0 || soft >= allocation forbids any soft when allocation is 0), but hard is not: hard < allocation is vacuous at zero. The loop then builds and stores the feature with Enabled=false and HardLimit set. No reader consumes HardLimit today, so there is no runtime consequence now (hence P3), but this PR is where the contract is established, and the C1 admission gate is the consumer that would read a ceiling off a feature that is off. Either forbid soft and hard when allocation == 0 (symmetric with the missing-allocation case), or document that a hard ceiling on a zero allocation is intentional and define which of Enabled/Limit/HardLimit the future gate honors. ZeroAllocationWithZeroHard asserts allocation=0, hard=0 is valid, so if that stays intended, reject only hard > 0 at zero allocation. No test exercises the allocation=0, hard>0 combination.

🤖

P3 [CRF-3] The rollout-safety invariant "none of the three claim names is a feature name" is load-bearing but unpinned by any test. (Chopper)

nothing fails if a future change registers one of the three claim strings as a feature name, or renames a constant to collide. The moment that happens, old deployments start interpreting a claim they were supposed to ignore, and the documented rollout guarantee breaks silently in the field

UnknownClaimsCompatibility uses invented names (future_feature_allocation) rather than the real claim constants, so it proves "unknown claims are ignored" but never asserts the real claim names are in fact unknown to FeatureNamesMap. One assertion closes it: require.NotContains(t, codersdk.FeatureNamesMap, codersdk.FeatureName(license.ClaimAgentRuntimeHoursAllocation)) for all three constants. This is the red test the next engineer touching feature registration would need.

🤖

P3 [CRF-4] The knowledge that agent_runtime_hours is composed of three claims is now encoded in three places instead of one decoder: validateAgentRuntimeHours, the remap/skip block here (300-317), and the inline soft/hard attachment in the UsesUsagePeriod case (345-354). (Ryosuke P3, Robin Note)

a claim added to the validator but not the decoder is silently ignored, the inverse rejects a license the decoder would have accepted

Consequence is maintenance drag and a silent-drift vector, not a runtime bug, and it does mirror the accepted managed-agent precedent, so this is a rough edge to weigh rather than a stop. Ryosuke's suggested collapse: lift claim-to-Feature decoding into a dedicated decodeAgentRuntimeHours(claims.Features) (codersdk.Feature, bool) that owns validation and soft/hard attachment together, called once from the loop. Robin notes a third such feature is the point at which this pattern is worth a table (claim-name to feature-name plus a soft/hard flag) instead of a fourth inline branch.

🤖

P3 [CRF-1] The new exported AgentRuntimeHours test builder has zero callers and is itself untested. (Netero)

Its sibling ManagedAgentLimit (same file, line 237) has three production-test callers ... Every test added in this PR bypasses the builder and constructs the map directly

Because no test exercises it, a wrong mapping in the helper (for example pointing at the soft-limit claim) would ship undetected. Either delete it or use it in the new tests so it is covered. Assuming the usage-wiring follow-ups will call it is a deferral without a caller in this diff.

🤖

P4 [CRF-5] The new warning-suppression branch runs but no test asserts its effect; the one test that reaches it skips the feature instead of checking the warning is gone. (Bisky)

Delete the production skip and every test still passes; the branch is executed for coverage but its behavior is unproven.

SingleLicenseNotEntitled enables every feature via all, so FeatureAgentRuntimeHours is Enabled and NotEntitled, exactly the state this skip suppresses, but the test adds the feature to its continue list rather than asserting the warning is absent. If the skip regresses, admins get a spurious "Agent Runtime Hours is enabled but your license is not entitled" warning. Assert require.NotContains on that warning string in a test where all enablements apply and no agent-runtime license is granted. Makes the PR bigger, not wrong, so P4.

🤖

Nit [CRF-6] Doc comment starts with AgentRuntimeHours, but the declared identifier is FeatureAgentRuntimeHours. (Gon, Leorio)

Go doc convention and revive's exported-comment rule want the comment to begin with the exact name. It reads // AgentRuntimeHours is a usage period feature; make it // FeatureAgentRuntimeHours is a usage period feature. This is the only const in the block that carries a doc comment, so the mismatch stands out and go doc renders it detached from the symbol.

🤖

Nit [CRF-7] HardLimit has no doc comment of its own, so the generated API schema documents soft_limit but leaves hard_limit blank. (Meruem Nit, Leorio Note)

The shared doc block sits on SoftLimit (line 384), and swagger attaches a field description only to the field its comment immediately precedes, so docs/reference/api/schemas.md renders hard_limit with an empty description column. Add a one-line comment directly above HardLimit (for example, "HardLimit is the enforcement ceiling; see SoftLimit.") and regenerate with make gen.

🤖

Nit [CRF-8] The four error vars drop Hours, breaking the naming pattern every other symbol for this feature follows. (Gon)

The feature is FeatureAgentRuntimeHours, the claims are ClaimAgentRuntimeHours*, the validator is validateAgentRuntimeHours, but the errors are ErrMissingAgentRuntimeAllocation, ErrInvalidAgentRuntimeAllocation, ErrInvalidAgentRuntimeSoftLimit, ErrInvalidAgentRuntimeHardLimit. Rename to ErrMissingAgentRuntimeHoursAllocation etc. so the set reads consistently and greps together.

🤖

Nit [CRF-9] ErrMissingAgentRuntimeAllocation's message "license has agent runtime limit claims but is missing the ... claim" is vaguer than the code it guards. (Gon)

The guard fires when a soft or hard limit is present without an allocation. "agent runtime limit claims" is imprecise; say "license has agent runtime hours soft/hard limit claims but is missing the %s claim" so the operator knows which claims triggered it. (Leorio confirms the sibling messages are otherwise the right bar: they name the offending claim and the exact constraint.)

🤖