Skip to content

Commit 0819067

Browse files
committed
fix: tighten TestNoMagicValues, migrate 7 numeric constants to config
Remove blanket isConstDef/isVarDef exemption from TestNoMagicValues — numeric const definitions outside config/ are now caught. All 7 violations resolved: - JSON-RPC error codes (ErrCodeParse, ErrCodeNotFound, ErrCodeInvalidArg, ErrCodeInternal) moved from mcp/proto/schema.go to config/mcp/schema/. Updated 14 consumer sites + 1 test file. - DefaultPriority (50) moved to config/steering/. Updated steering/frontmatter.go and cli/steering/cmd/add/cmd.go. - PollIntervalSec (5) moved to config/mcp/server/. - Removed dead isVarDef helper from magic_values_test.go. Spec: specs/ast-audit-contributor-guide.md Signed-off-by: Jose Alekhinne <jose@ctx.ist>
1 parent 839bcbc commit 0819067

20 files changed

Lines changed: 71 additions & 77 deletions

File tree

internal/audit/magic_values_test.go

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,14 @@ func TestNoMagicValues(t *testing.T) {
9999
return true
100100
}
101101

102-
// Skip file-level const/var definition sites only.
103-
// Local consts inside function bodies are NOT exempt —
104-
// they are just renamed magic numbers.
105-
if isConstDef(file, lit) || isVarDef(file, lit) {
106-
return true
107-
}
102+
// Const/var definitions in exempt packages
103+
// are already skipped (line 86). Outside
104+
// those packages, numeric constants are
105+
// magic values that belong in config/.
106+
//
107+
// DO NOT re-add a blanket isConstDef
108+
// exemption. It masks constants defined
109+
// in the wrong package.
108110

109111
if exemptIntLiterals[lit.Value] {
110112
return true
@@ -157,28 +159,6 @@ func isExemptPackage(pkgPath string) bool {
157159
return false
158160
}
159161

160-
// isVarDef reports whether lit appears inside a var declaration.
161-
func isVarDef(file *ast.File, lit *ast.BasicLit) bool {
162-
for _, decl := range file.Decls {
163-
gd, ok := decl.(*ast.GenDecl)
164-
if !ok || gd.Tok != token.VAR {
165-
continue
166-
}
167-
for _, spec := range gd.Specs {
168-
vs, ok := spec.(*ast.ValueSpec)
169-
if !ok {
170-
continue
171-
}
172-
for _, val := range vs.Values {
173-
if containsNode(val, lit) {
174-
return true
175-
}
176-
}
177-
}
178-
}
179-
return false
180-
}
181-
182162
// isStrconvArg reports whether lit is an argument to a strconv
183163
// function (radix or bitsize parameter).
184164
func isStrconvArg(file *ast.File, lit *ast.BasicLit) bool {

internal/cli/steering/cmd/add/cmd.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@ import (
2424
writeSteering "github.com/ActiveMemory/ctx/internal/write/steering"
2525
)
2626

27-
// defaultPriority is the default priority for new steering files.
28-
const defaultPriority = 50
29-
3027
// Cmd returns the "ctx steering add" subcommand.
3128
//
3229
// Returns:
@@ -79,7 +76,7 @@ func Run(c *cobra.Command, name string) error {
7976
sf := &steering.SteeringFile{
8077
Name: name,
8178
Inclusion: cfgSteering.InclusionManual,
82-
Priority: defaultPriority,
79+
Priority: cfgSteering.DefaultPriority,
8380
}
8481

8582
data := steering.Print(sf)

internal/config/README.md

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44

55
This directory contains ~60 sub-packages, each holding constants,
66
compiled regexes, type definitions, or text keys for a single
7-
domain. This looks unusual. It's intentional.
7+
domain. This looks unusual. **It's intentional**.
88

99
### The problem it solves
1010

1111
A monolithic `config` package creates a false dependency: importing
1212
`config` to use `config.TokenBudget` also imports every regex
1313
pattern, every MCP constant, every entry type, and every CLI flag
14-
name. In Go, the package is the dependency unit importing one
14+
name. In Go, the package is the dependency unit: importing one
1515
symbol imports the whole package. A change to any constant in the
1616
package marks every consumer as stale for recompilation and makes
1717
the blast radius of any change the entire codebase.
@@ -44,7 +44,7 @@ import "github.com/ActiveMemory/ctx/internal/config" // everything
4444

4545
- Surgical dependency tracking (change `config/mcp/tool` and only
4646
MCP packages recompile)
47-
- Zero import cycles (all sub-packages are leaves zero internal
47+
- Zero import cycles (all sub-packages are leaves: zero internal
4848
dependencies)
4949
- Clear ownership (each file belongs to one domain)
5050
- Safe to modify (changing a constant in `config/agent` cannot
@@ -138,12 +138,12 @@ method receivers, interface participation, or business logic. A
138138
type with `func (t IssueType) Severity() int` has outgrown
139139
`config/` and belongs in `entity/`.
140140

141-
| Stage | Home | Example |
142-
|-------|------|---------|
143-
| Pure value enum | `config/<domain>/` | `type IssueType string` with const values |
144-
| Cross-package value enum | `config/<domain>/` | Same — `config/` is already importable everywhere |
145-
| Type with methods | `entity/` | `func (t IssueType) Severity() int` |
146-
| Type implementing interfaces | `entity/` | `var _ fmt.Stringer = IssueType("")` |
141+
| Stage | Home | Example |
142+
|------------------------------|--------------------|---------------------------------------------------|
143+
| Pure value enum | `config/<domain>/` | `type IssueType string` with const values |
144+
| Cross-package value enum | `config/<domain>/` | Same — `config/` is already importable everywhere |
145+
| Type with methods | `entity/` | `func (t IssueType) Severity() int` |
146+
| Type implementing interfaces | `entity/` | `var _ fmt.Stringer = IssueType("")` |
147147

148148
The migration path is natural: start in `config/`, promote to
149149
`entity/` when behavior appears. `TestCrossPackageTypes` catches

internal/config/mcp/schema/schema.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,18 @@ package schema
99
// ProtocolVersion is the MCP protocol version string.
1010
const ProtocolVersion = "2024-11-05"
1111

12+
// Standard JSON-RPC error codes.
13+
const (
14+
// ErrCodeParse indicates malformed JSON.
15+
ErrCodeParse = -32700
16+
// ErrCodeNotFound indicates method not found.
17+
ErrCodeNotFound = -32601
18+
// ErrCodeInvalidArg indicates invalid parameters.
19+
ErrCodeInvalidArg = -32602
20+
// ErrCodeInternal indicates an internal error.
21+
ErrCodeInternal = -32603
22+
)
23+
1224
// JSON Schema type constants.
1325
const (
1426
// Object is the JSON Schema type for objects.

internal/config/mcp/server/server.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ const (
2020
SubcommandServe = "serve"
2121
)
2222

23+
// PollIntervalSec is the default interval in seconds for
24+
// resource change polling.
25+
const PollIntervalSec = 5
26+
2327
// Args returns the CLI arguments to launch the ctx MCP server.
2428
func Args() []string {
2529
return []string{"mcp", SubcommandServe}

internal/config/steering/steering.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,7 @@ const (
2626
// LabelAllTools is the display label when a steering
2727
// or trigger item applies to all tools.
2828
const LabelAllTools = "all"
29+
30+
// DefaultPriority is the default injection priority for
31+
// steering files when omitted from frontmatter.
32+
const DefaultPriority = 50

internal/mcp/proto/schema.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,6 @@ type RPCError struct {
6464
Data interface{} `json:"data,omitempty"`
6565
}
6666

67-
// Standard JSON-RPC error codes.
68-
const (
69-
// ErrCodeParse indicates malformed JSON was received.
70-
ErrCodeParse = -32700
71-
// ErrCodeNotFound indicates the method was not found.
72-
ErrCodeNotFound = -32601
73-
// ErrCodeInvalidArg indicates invalid method parameters.
74-
ErrCodeInvalidArg = -32602
75-
// ErrCodeInternal indicates an internal JSON-RPC error.
76-
ErrCodeInternal = -32603
77-
)
78-
7967
// --- Initialization types ---
8068

8169
// InitializeParams contains client information sent during initialization.

internal/mcp/server/parse/parse.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/ActiveMemory/ctx/internal/assets/read/desc"
1313
"github.com/ActiveMemory/ctx/internal/config/embed/text"
14+
cfgSchema "github.com/ActiveMemory/ctx/internal/config/mcp/schema"
1415
"github.com/ActiveMemory/ctx/internal/config/mcp/server"
1516
"github.com/ActiveMemory/ctx/internal/mcp/proto"
1617
)
@@ -32,7 +33,7 @@ func Request(data []byte) (*proto.Request, *proto.Response) {
3233
return nil, &proto.Response{
3334
JSONRPC: server.JSONRPCVersion,
3435
Error: &proto.RPCError{
35-
Code: proto.ErrCodeParse,
36+
Code: cfgSchema.ErrCodeParse,
3637
Message: desc.Text(text.DescKeyMCPErrParse),
3738
},
3839
}

internal/mcp/server/poll/poll.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@ import (
1717
"github.com/ActiveMemory/ctx/internal/mcp/server/catalog"
1818
)
1919

20-
// defaultPollInterval is the default interval for resource change
21-
// polling.
22-
const defaultPollInterval = 5 * time.Second
20+
// defaultPollInterval is the default interval for
21+
// resource change polling.
22+
var defaultPollInterval = time.Duration(
23+
server.PollIntervalSec,
24+
) * time.Second
2325

2426
// NewPoller creates a poller for the given context directory.
2527
//

internal/mcp/server/resource/dispatch.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/ActiveMemory/ctx/internal/assets/read/desc"
1414
"github.com/ActiveMemory/ctx/internal/config/embed/text"
15+
cfgSchema "github.com/ActiveMemory/ctx/internal/config/mcp/schema"
1516
"github.com/ActiveMemory/ctx/internal/context/load"
1617
"github.com/ActiveMemory/ctx/internal/mcp/proto"
1718
"github.com/ActiveMemory/ctx/internal/mcp/server/catalog"
@@ -50,14 +51,14 @@ func DispatchRead(
5051
req.Params, &params,
5152
); unmarshalErr != nil {
5253
return out.ErrResponse(
53-
req.ID, proto.ErrCodeInvalidArg,
54+
req.ID, cfgSchema.ErrCodeInvalidArg,
5455
desc.Text(text.DescKeyMCPErrInvalidParams),
5556
)
5657
}
5758

5859
ctx, loadErr := load.Do(contextDir)
5960
if loadErr != nil {
60-
return out.ErrResponse(req.ID, proto.ErrCodeInternal,
61+
return out.ErrResponse(req.ID, cfgSchema.ErrCodeInternal,
6162
fmt.Sprintf(
6263
desc.Text(text.DescKeyMCPLoadContext),
6364
loadErr,
@@ -74,7 +75,7 @@ func DispatchRead(
7475
return readAgentPacket(req.ID, ctx, tokenBudget)
7576
}
7677

77-
return out.ErrResponse(req.ID, proto.ErrCodeInvalidArg,
78+
return out.ErrResponse(req.ID, cfgSchema.ErrCodeInvalidArg,
7879
fmt.Sprintf(
7980
desc.Text(text.DescKeyMCPErrUnknownResource),
8081
params.URI,

0 commit comments

Comments
 (0)